(This is my first bug report on drupal.org, feel free to point out things that I could have done better)
Problem/Motivation
It seems that CSS aggregation changes the order of CSS rules if "@import" rules are used.
I've created a simple theme to reproduce the issue:
test_theme.info.yml
name: Test theme type: theme description: Test theme core: 8.x libraries: - test_theme/main
test_theme.libraries.yml
main:
css:
theme:
static/main.css: {}static/main.css
@import url(first.css); @import url(second-desktop.css) screen and (min-width: 1024px); @import url(second-mobile.css) screen and (max-width: 1023px);
static/first.css
.first { margin: 0; }The contents of "second-desktop.css" and "second-mobile.css" are irrelevant.
If I use this theme with CSS aggregation turned on then the whole CSS for the theme is served as a single file with the following content:
@import url(/themes/custom/test_theme/static/second-desktop.css) screen and (min-width:1024px);@import url(/themes/custom/test_theme/static/second-mobile.css) screen and (max-width:1023px);.first{margin:0;}As you can see, the content from "first.css" is now contained in the file instead of being imported. However, it is included after the "second-*.css" files. While this is correct according to the CSS standard (which mandates that "@import" rules must come before any other rules) it changes the meaning of the CSS, since I now cannot override rules from "first.css" in the "second-*.css" files.
If I turn of CSS aggregation then the theme works as expected.
Proposed resolution
"@import" rules should only be inlined during CSS aggregation if all "@import" rules after them can also be inlined. If some are guarded by media queries as in my example then only "@import" rules after the last guarded "@import" rule should be inlined.
Comments
Comment #2
star-szrThanks for the report, I think asset library system is a more accurate component but could be wrong :)
Comment #3
wim leersGreat find! I'm 99% certain this bug then also exists in Drupal 7.
The proposed resolution says:
I was gonna say , but then I remembered that all imports must be at the top, per http://www.w3.org/TR/REC-CSS2/cascade.html#at-import.
I honestly think it's only by accident that we don't resolve the
@importrules that have a media query. I bet the regular expressions we use just don't find those.Your proposed solution would work. But wouldn't it be better to also resolve those with a media query and then just wrap the CSS in a
@mediarule? The browser always fetches all CSS, even if it doesn't currently meet that particular media query. If we don't do what I propose, then the effectiveness of CSS aggregation is severely diminished.Comment #4
torf commentedWim Leers said:
That does indeed sound like a better solution.
Comment #15
catchPretty sure this was fixed in #2936067: CSS aggregation fails on many variations of @import.