Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.When using hook_css_alter, sometimes extra aggregates are produced. This is because drupal_group_css() expects the items to be sorted such that matching groups are adjacent in the list. However, drupal_sort_css_js ignores 'media' and 'browser' keys, so for example if a print stylesheet appears in between two media=all stylesheets then you get extra aggregates.
Steps to reproduce:
1) Drop this alter hook somewhere:
function mymodule_css_alter(&$css) {
foreach ($css as $name => $style) {
$css[$name]['group'] = CSS_DEFAULT;
$css[$name]['every_page'] = FALSE;
}
}
2) Enable aggregation
Expected behavior: only one media=all stylesheet will be output, since the alter hook has forced them into the same group.
Actual behavior: 3 media=all stylesheets are output (not counting conditional IE styles)
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | drupal_sort_css_js.patch | 2.31 KB | grendzy |
| drupal_sort_css_js.patch | 2.13 KB | grendzy | |
Comments
Comment #1
effulgentsia CreditAttribution: effulgentsia commentedI don't think we want this. Ideally, we don't want to mess with CSS rule order. If a media="print" stylesheet is weighted between 2 media="all" stylesheets, then I think it's better to allow 3 aggregate files to be produced than to disrespect the weights. In #769226: Optimize JS/CSS aggregation for front-end performance and DX, we introduced a 'every_page' key that has greater priority than weight, but this was done because there was a compelling performance reason to do it, and it appeared to make some sense. However, even that has proven to confuse people: #977844: Remove the 'every_page' option for CSS/JS assets: it is confusing, even damaging.
Comment #2
effulgentsia CreditAttribution: effulgentsia commentedTagging so that people watching the markup queue see this, since any proposed changes to CSS rule order affects them.
Comment #3
grendzy CreditAttribution: grendzy commentedHm, I see what you mean about respecting weights. I dug deeper to see why core would assign such strange weights (such that print / IE styles would be sandwiched in between global styles), and it's because of this in drupal_add_css:
Here are the groups that are produced by the default install (after applying the above css_alter). You can see that Bartik's print and IE stylesheets break the global styles into 3 groups, and the weights are essentially arbitrary:
Preserving insertion order is quite handy, but I'm having a hard time imagining a developer wanting insertion order to have a higher precedence than media type or browser. What do you think about sorting by floor(weight), then browser/media, then weight again? That way explicit integer weights could still "break out" of a group, but implicit fractional weights could not.
Comment #4
grendzy CreditAttribution: grendzy commentedHere's an updated patch. The main change is sorting by integer weight first, which prevents implicit weights from splitting a group, but still allows explicit weights to break out of a group. Insertion order is still preserved within groups.
I also made the sort callback more compact by just using subtraction to compare numeric values - rather than separate conditions for less than / greater than.
Comment #5
alanburke CreditAttribution: alanburke commentedsubscribe
Comment #6
tomgf CreditAttribution: tomgf commentedsubscribe