When there are more than 31 stylesheets on a page, in order to support IE9 CssCollectionRenderer outputs them within an @import statement instead of as <link> elements.
This affects Content-Security-Policy rules, requiring 'unsafe-inline' in order for the CSS to not be blocked by the browser.
Enabling CSS aggregation should reduce the number of stylesheets on the page below the threshold of 31 items such that they can be output as <link> elements, so production sites should not be affected if this particular support for IE9 was removed.
Remove the @import workaround so that CssCollectionRenderer always outputs stylesheets in <link> elements.
Create a contrib module (ie9 / legacyie) that provides the current CssCollectionRenderer from 8.3
In Internet Explorer 9 and earlier, there is a limit of 31 style sheets per page. Drupal dropped support for Internet Explorer 9 and 10 in 8.4.0, but Drupal 8.5 and 8.6 retained a workaround to allow 32 or more stylesheets to be included. This workaround has been removed in 8.7. Sites requiring Internet Explorer 9 support should enable CSS aggregation (preferred) or install the IE9 Compatibility contributed module.
| Comment | File | Size | Author |
|---|---|---|---|
| #22 | drupal-2897408-22-interdiff.patch | 3.46 KB | gapple |
| #22 | drupal-2897408-22-remove-css-ie-hack.patch | 32.77 KB | gapple |
Comments
Comment #2
catchI noticed we were still doing this when working on #1014086: Stampedes and cold cache performance issues with css/js aggregation, it'd be great to remove that workaround, and production sites should still be using aggregation so it doesn't actually break anything visitor-facing anyway.
Comment #3
cburschkaThis removes the hack, but doesn't update the corresponding unit test yet.
Comment #4
cburschkaAlso removed the explanation of the hack from the phpdoc.
Comment #5
cburschkaComment #9
effulgentsia commented+1 for doing this in 8.5. For 8.4, https://www.drupal.org/node/2897971 currently says:
Should we honor that for this issue? In which case, should we do something like rename the existing
CssCollectionRenderertoCssCollectionRendererLegacyorDeprecatedCssCollectionRendererand have some mechanism for sites to opt into it (or out of it)?Comment #10
effulgentsia commentedAlternatively, should we update that change record to narrow "has retained existing workarounds" to something like "has retained existing workarounds for production configurations"?
Comment #11
catchI think #10 is reasonable for this issue.
Comment #12
gappleI've created https://www.drupal.org/project/ie9, which decorates core's CssCollectionRenderer to handle output when there are 32 or more stylesheets.
Comment #14
xjmI agree on doing this for 8.5.x. I'm not sure it's worth backporting anything to 8.4.x for it.
Comment #15
xjmAlso, thanks @gapple for #12. This would be the first actually hard break for IE9 (vs. just passively not adding workarounds or testing against IE9 anymore). I know @droplet has been concerned about the impact of dropping IE9 support for users in China, so that project is a great compromise.
Comment #16
xjmComment #19
gappleUpdated patch
- Removes additional comments on the class regarding LINK versus STYLE tags that is no longer relevant
- Updated tests:
-- Change expected order of
#attributeskeys-- Test that 1, 31, and 32 stylesheets are always rendered with LINK elements; remove IE-specific tests for STYLE grouping.
Comment #20
gappleComment #22
gappleAnother test was dependent on the order of attributes
Comment #24
gappleI forgot to not name the interdiff file
.patch. Updating status since the actual patch was successful.Comment #25
joelpittetSo much cruft removal, I love it and thanks for the ie9 decorator! Let's ensure that is mentioned in the CR
Comment #26
catchThis is something to mention in 8.7.0 release notes. Still needs a change record.
Patch looks great so +1 to the RTBC.
Comment #27
larowlanAgain, +1 RTBC, but we need a change record here
Comment #28
gappleCreated change record: CSS Rendering no longer includes workaround for Internet Explorer 9
Comment #29
joelpittetBack to RTBC, thanks for adding a CR @gapple
Comment #30
catchCommitted c724390 and pushed to 8.7.x. Thanks!
Comment #33
kattekrab commentedAdding "needs release note" tag.
As this is listed in the "important update info" section of the alpha release notes for Drupal 8.7, we need to let people know what has changed.
Comment #34
xjmThanks @kattekrab! Adding a snippet based on the CR.
Comment #35
xjmComment #36
xjmComment #37
gapple