On a site that upgrading from 'wysiwyg' 2.2 to 'wysiwyg' 2.4, it doesn't add a particular CSS file to the WYSIWYG. I put a whole bunch of debug statements into the _wysiwyg_pre_render_styles()
function to try and determine why that was.
In _wysiwyg_pre_render_styles()
it has this code:
if (isset($elements['#groups'][$child]['group']) && $elements['#groups'][$child]['group'] != CSS_THEME) {
continue;
}
This is assuming that the numerical indexes (ie. $child
could equal '38') in $elements
line up with the numerical indexes in $element['#groups']
. For whatever reason, that isn't true for this particular CSS file, and walking through the arrays, for many others as well.
In my testing, this is the $elements[$child]
:
[38] => Array
(
[#type] => html_tag
[#tag] => link
[#attributes] => Array
(
[type] => text/css
[rel] => stylesheet
[href] => http://site/profiles/distro/themes/distro_theme/assets/css/bootstrap-custom.css?or8e8i
[media] => all
)
[#browsers] => Array
(
[!IE] => 1
[IE] => 1
)
)
But this is $elements['#groups'][$child]
:
[38] => Array
(
[type] => external
[group] => 0
[every_page] =>
[media] => all
[preprocess] => 0
[browsers] => Array
(
[!IE] => 1
[IE] => 1
)
[items] => Array
(
[0] => Array
(
[type] => external
[group] => 0
[weight] => 0.056
[every_page] =>
[media] => all
[preprocess] => 0
[data] => //fonts.googleapis.com/css?family=Source+Serif+Pro:400,600,700
[browsers] => Array
(
[!IE] => 1
[IE] => 1
)
)
)
)
That's clearly not the same item! The one in $elements['#groups'][38]
has a 'group' of 0, but the item at $elements[38]
should have a 'group' of 100 (or CSS_THEME
) that would get it included (as opposed to skipped in this case).
In this case, it's off by one -- the item at $elements['#groups'][37]
is the correct one. However, when I walk through both arrays, it's not always off by one, the items are close sometimes, but mixed up in various ways.
As best as I can tell, the theme isn't doing anything unusual. The CSS stylesheet that we want to be added is added thus:
stylesheets[all][] = assets/css/bootstrap-custom.css
And the font entry is added in template.php in THEME_preprocess_html()
:
drupal_add_css('//fonts.googleapis.com/css?family=Source+Serif+Pro:400,600,700', array('type' => 'external'));
However, this theme is a 3rd generation sub-theme: GRANDPARENT -> PARENT -> THEME, which may be how it got all mixed up? And the 'bootstrap-custom.css' stylesheet itself is defined in the PARENT theme and overridden in the THEME itself, which could also be related to the cause?
Anyway, I guess what I'm getting to, is that there are apparently circumstances where those arrays don't line up, and so I don't think it's safe to assume that the indexes will match. I'm going to continue try and dig into a safe way to connect a style element with it's group.
Comment | File | Size | Author |
---|---|---|---|
#8 | wysiwyg-theme-css-v24-2884691-8.patch | 1.08 KB | dsnopek |
Comments
Comment #2
dsnopekHere's a patch that fixes this in my case. I don't know how brittle it is or if there are other cases that this doesn't work with, but it works in mine. :-)
Comment #3
dsnopekComment #4
cboyden CreditAttribution: cboyden commentedAfter applying the patch there are a lot more stylesheets linked in the WYSIWYG frame. I count 40 link tags in the HEAD section of the frame, compared to 10 when running 2.4 without the patch. This is on a clean install of a Panopoly site using the Responsive Bartik theme; stats may differ for other themes.
The additions include CSS files from the core system, field, node, and user modules, as well as from contrib modules jQuery Update, Date, Views, etc, and from Panopoly features.
Not sure if this is a problem?
Comment #5
TwoDThat's odd, would be interesting to find what's causing it. Are you using any modules which change how CSS files are aggregated? Perhaps it is re-ordering them in unexpected ways.
Comment #6
cboyden CreditAttribution: cboyden commented@TwoD this is on a test environment where aggregation is turned off.
Looking more closely at the stylesheets that are loaded, it looks like after applying the patch all the same stylesheets that are loaded in the HEAD section of the main page are being loaded in the WYSIWYG frame as well.
Comment #7
dsnopekGah! Ignore my patch in #2 -- it "solves" the problem, but only because I got the logic wrong and so it adds *all* CSS files rather than just the theme specific ones. I'll take a 2nd shot at this...
@TwoD: I don't think the site is doing anything abnormal, but it's certainly possible that it could be. But I'm also not convinced that there's any guaranty that the arrays would be in the same order, even with just Drupal core affecting the list of styles, although, I haven't made it through the full code path to get from
$elements['#items']
to$elements['#groups']
(that's done by drupal_group_css()) to the children on$elements
. But, hopefully, I'll eventually track it down!Comment #8
dsnopekOk! Here's a whole new patch. What it does is it removes all CSS that aren't CSS_THEME in
_wysiwyg_filter_editor_styles()
which runs before any processing, so, it's an easy thing to determine at that point. Then when we get to_wysiwyg_pre_render_styles()
, it just lets everything through, because we know it's already been filtered to just CSS_THEME. This shouldn't be nearly as brittle as expecting those array indices to line up!Comment #9
dsnopekComment #10
TwoDCool, thanks! Will test as soon as I can.
Comment #11
cboyden CreditAttribution: cboyden commentedDid a first round of testing, and this patch looks good so far.
Comment #12
dsnopekWe'll likely include this patch in Panopoly!
Comment #13
candelas CreditAttribution: candelas as a volunteer commentedIt works! thanks @dsnopek :) I was tired to write body#tinymce in my styles for node edit forms!!!!
Comment #15
TwoDThank you so much for patching and testing this!
Adjusted the patch slightly because the
drupal_alter()
change wasn't needed any more but otherwise it worked very well.