I'm not sure how this was ever working as per this article:
http://www.drupaler.co.uk/blog/path-theme-module/73

Wrong! You cannot, must not, EVER use path_to_theme() outside of either a theme function or template.php. If you do, it will not return what you expect it to return. Instead of the path to your theme, you'll get a seemingly arbitrary path to the last invoked module. Because modules are not supposed to have anything to do with themes, you have no business calling path_to_theme() in a module anyway.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

james.elliott’s picture

FileSize
5.51 KB

I've got a patch for this. path_to_theme() will return the last item that has run a theme function, which when called in a module is going to be a module. To get the path to the theme correctly I've changed it to call drupal_get_path('theme', variable_get('theme_default', 'none')) to return the path to the default theme.

james.elliott’s picture

Status: Active » Needs review

Oops, always forget to change status

TwoD’s picture

Well, it works as intended before D7. Wysiwyg used to run its processing in an '#after_build' callback, while it now runs in '#pre_render', which I guess changes its "context" from "in module" to "in theme". As per documentation path_to_theme() always returns the path to the active theme when called in a module context, and I think we want the same thing in D7. Looks like this context change consequence was overlooked when the D7 port of Wysiwyg was made.
.
Using the default theme may not have the desired effect. (For example, while editing in Seven, files would still be imported from Garland.) I didn't test what happens when a user is using a different theme than the active one, but I suppose it'll have the same effect.

If I'm editing using Seven, I would expect Seven's files to be used by the editor so I can easily determine where editor-specific styles are if I need to tweak them. I would on the other hand expect my non-editing theme to be used during previews of the same content, but Wysiwyg module is not involved at that point.

james.elliott’s picture

My thought was that when using the WYSIWYG I would want to see my content styled the way it would appear when viewed, so that I didn't have to switch between view and edit repeatedly to see the final effects of my styling. If it's more acceptable to use the current theme, such as Seven in the overlay, then using $theme_key instead of variable_get('theme_default', 'none') would return Seven's theming.

But then again, if I wanted my admin theme for the WYSIWYG I could change the Editor CSS value to "Use theme CSS" which would add the files from my admin theme.

TwoD’s picture

Yeah, $theme_key would probably be the way to go.

Technically, you'd have to switch between Edit and View for each one of your active themes to know what the styling would really look like. If your editing theme has a good styling compromise between the various active themes, you could be more sure the edited content would look alright in all of them. ;)

Yes, switching to "Use theme CSS" would give you the files from the active theme, but "Define CSS" could give you that, and the ability to remove/add files as you see fit and still have it work with all themes.

james.elliott’s picture

Maybe a solution would be to use %theme_name as the token where theme_name is the name of the theme that you want to use.

So I could enter %seven to get the path to Seven.

sun’s picture

Status: Needs review » Needs work

This basically means that we need to defer profile loading into #pre_render. Or alternatively, just initialize that damn theme? D7 initializes relatively early anyway, I think.

Either way, the configuration variable is of no use.

TwoD’s picture

Status: Needs work » Needs review
FileSize
6.53 KB

Won't $theme_key work together with drupal_get_path()? Drupal initializes the theme and sets $theme_key as part of the last bootstrap step so we should be ok there.

james.elliott’s picture

Won't $theme_key be set to the admin theme though? Or is that the desired behavior? My only concern would be that this would load Seven (or another admin theme) into the WYSIWYG instead of the site theme when the site theme has the styling that the content will have on save?

TwoD’s picture

Yes, if the admin theme is different from the main theme. That behavior is the same as in D6 and I think we should keep it the same at least while fixing/porting the code for D7.

Figuring out which theme is active where the contents will be rendered would require a lot more code similar to what's in drupal_theme_initialize() and menu_get_custom_theme(), if we can even predict where the contents will be rendered in the first place.

Maybe we could "simply" check which theme is active for the current user on a well known path like the front page or the user profile page and assume the same theme will be used to render the contents currently in the editor, but it'd still be a lot more new code that'd be better handled in a feature request.

sun’s picture

Priority: Critical » Normal

This will have to wait for 7.x-1.1. I'm not sure about the problem and proposed solution yet.

tobby’s picture

FileSize
7.32 KB

I have rolled a patch based on TwoD's changes in #8, but it uses the default theme system variable instead of $theme_key to ensure that it gets the site theme, and not the currently used admin theme. It might not be the perfect solution, but it's working fine for me (using a custom theme for the site, and rubik for my admin theme, I see the editor.css from my custom theme).

bryancasler’s picture

Just tried applying the patch in #12, but it didn't seem to have any effect. I'm using a custom sub theme as my site's main theme and Seven as my admin theme. I applied the patch, ensured "Use theme CSS" was selected for my editor. I then cleared my site cache, logged out, cleared my browser cache and logged back in. The WYSIWYG editor is still displaying the same styling.

I looked at the source of an admin page before and after, the same CSS files were being loaded. Also Sun's comment in #11 made me wonder if some other implementation had made it into the 2.x version. Thoughts?

joelstein’s picture

Status: Needs review » Reviewed & tested by the community

The patch in #12 worked for me.

Regarding the comment in #13, this issue is about using "Define CSS" and populating the "CSS path" field (not "Use theme CSS").

bryancasler’s picture

So I need to add all these as comma separated values?
http://i.min.us/iyK0C.jpg

TwoD’s picture

You will most likely not need to include all of those stylesheets in the editing area, only those with style rules that actually affect the node/comment content.

joelstein’s picture

What I usually do is reference one stylesheet (something like wysiwyg.css), and in there @import the stylesheets I need. Works pretty well.

FrequenceBanane’s picture

#12 patch is not fully compatible with the one of #901862: Editor does not load multiple CSS files for whizzywig because of the following parts of code (for everything else, replace $settings['content_css'] by $settings['externalCSS'] is ok.
From patch of #901862: Editor does not load multiple CSS files :

if ($config['css_setting'] == 'theme') {
-      $css = path_to_theme() . '/style.css';
-      if (file_exists($css)) {
-        $settings['externalCSS'] = base_path() . $css;
-      }
+      $settings['externalCSS'] = wysiwyg_get_css();
     }

From this (#835682: %t (path_to_theme() ) returns modules/field (or whatever module is creating the wysiwyg) instead of the active theme.) patch :

if ($config['css_setting'] == 'theme') {
-      $css = path_to_theme() . '/style.css';
+      $css = drupal_get_path('theme', $themekey) . '/style.css';
       if (file_exists($css)) {
         $settings['externalCSS'] = base_path() . $css;
       }
michaelfavia’s picture

Subscribe. Also affected.

gagarine’s picture

Status: Reviewed & tested by the community » Needs work

I do not agree than because you use seven as an admin theme you should use the CSS of seven for wysiwig. It's specially true when you use overlay.

I guess the easy way should be to add new options for each case.

#Editor CSS:
- Use theme CSS
- Use admin theme CSS
- Use active theme CSS
- Define CSS
- Editor CSS 

Use theme CSS - loads stylesheets from default site theme.
Use admin CSS - loads stylesheets from the admin theme.
Use active theme CSS - load the CSS from the current active theme
Define CSS - enter path for stylesheet files below.
Editor default CSS - uses default stylesheets from editor.
szantog’s picture

Status: Needs work » Needs review
FileSize
6.62 KB

I think, playing with global $custom_theme a little bit overcomplicated. There are a global $theme, what return the active theme's name. I've just change path_to_theme function to drupal_get_path in all place, where path_to_theme was used.

szantog’s picture

Using global $theme causes namespace conflict. Change every $theme variable to $editor_theme.

jerry’s picture

Subscribing.

komlenic’s picture

For me, the patch in #12 is returning is the path of the admin theme NOT that of the site theme (which appears to be the opposite of the described behavior). I have not investigated thoroughly why that may be.

In reference to #10: Does a feature request exist for something like what you describe? I could envision this working in a way that the theme context would be inferred from where the contents will be rendered (if that context is an existing node/etc.)... and otherwise could perhaps be the front page or user page. That would seem to accommodate other modules such as ThemeKey, etc.

TwoD’s picture

@komlenic, I'm not aware of such a feature request. It's currently very hard - if at all possible - to know where anything will be rendered, especially when considering modules like Views, Panels, Display Suite etc. Even if we added a hook or other mechanism to ask modules where they'll render the content being edited - or preferably, which theme they'll use - we'd still not know what to do if the response includes more than one theme.

komlenic’s picture

@TwoD: True, it is difficult (or impossible) to reliably determine the context and theme, especially in your example where the response might include more than one theme.

I have a particular use case where I can safely limit the theme "guessing" to nodes only, and those nodes always have only one theme set reliably by the Theme Key module: in such a case it is possible to reliably know the theme where the node will be rendered based on the taxonomy of the node. As such I have employed a custom module to dynamically set the editor css file, dependent on the taxonomy of the node being edited/added. This works as desired for the use case, but has far too many specific dependencies to be included in any sensible way into the official WYSIWYG module. (As you essentially point out.)

I include this here, mainly to perhaps give someone an idea of how this might be accomplished -- even though it does not seem possible that there can be an all-encompassing solution for the official module.

TwoD’s picture

To summarize; We've got two patches with two different results. The one in #22 uses the "editing" theme (the "admin" theme if one is enabled, otherwise the "frontend" theme), while the one in #12 always uses the default frontend theme.

Which to pick?

#1309040: Select which theme's CSS to use when choosing 'Use theme CSS' in settings and possibly #1017564: Allow user to dynamically change in-editor theme should also be considered here so we can [re-]build theme handling step by step instead of having to re-implement it once for each of these issues.

ralphb’s picture

For what it's worth: I require the %t substitute exactly to differentiate between rich-text editors instantiated within the admin overlay versus those instantiated within the regular web page. Solution #12 would thus be worthless (and could be replaced by a hard-coded path anyway), whereas solution #22 is exactly what is needed here.

Paul Lomax’s picture

Here's a patch that adds a new %d token, which provides the path to the default theme.

TwoD’s picture

Re #29, we must still get rid of the path_to_theme() calls since Wysiwyg now does its processing when the return value of that function is not guaranteed to be what we expect.

AgaPe’s picture

Wysiwyg 6.x-2.4 the same problem.
The patch from #29 didnt apply to me so here is a new one just for the ckeditor.

Silicon.Valet’s picture

Version: 7.x-2.x-dev » 7.x-2.1
Status: Needs review » Reviewed & tested by the community

Wysiwyg 7.x-2.x has the problem as well
patch works for me. Thank you.

marcoka’s picture

i patched with 31 on d7. works. pretty important for the tokens to work. more important if you use multisite and dont want to manually "change" the sites foldername every time over and over.

Herr Lehmann’s picture

The patch from #31 works but be aware that this patch only applies for CKEditor!

sun’s picture

Version: 7.x-2.1 » 7.x-2.x-dev
Status: Reviewed & tested by the community » Fixed
FileSize
7.04 KB

Thanks for reporting, reviewing, and testing! Committed attached patch to 7.x-2.x.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.