Problem/Motivation

ore in D7/D8 does the following when adding CSS stylesheets:

    // Always add a tiny value to the weight, to conserve the insertion order.
    $options['weight'] += count($css) / 1000;
// ...
    $css[$data] = $options;

while the documentation does state that earlier attached files will win in the order if nothing else ...

This even applies when using only #attached! (and in Drupal 8)

Consider:

foo.css

a.css
b.css
c.css
a.css

The algorithm will do:

foo.css: 0
a.css: 0.001
b.css: 0.002
c.css: 0.003
a.css: 0.003

=> ORDER

b.css:0.002
a.css:0.003
c.css:0.003

I don't assume someone relies on that order, but it still is a bug and violates the specification how this should work. Only because we have different aggregation groups, this does not break horribly.

Proposed resolution

- Check if the weight exists, re-use the old weight if so.

- Another possibility is to increase the weight always instead.

Remaining tasks

- Upload patch
- Discuss

User interface changes

- None

API changes

- CSS order is earliest file wins within a group with same weight

Files: 
CommentFileSizeAuthor
#2 css_js_order_with_the-2380655-2.patch1.65 KBFabianx
FAILED: [[SimpleTest]]: [MySQL] 41,126 pass(es), 2 fail(s), and 0 exception(s). View

Comments

Wim Leers’s picture

Hah :) We know that the weight system is horrible DX-wise and horrendously unreliable, so I'm not surprised. Most people are surprised we got away with it for so long.

I'd say we don't want to fix this in D8 in favor of before/after relationships, but I'm not 100% sure that can still happen in D8, but I think we could still do it in 8.1 because it is used so rarely and it really is a horrible, horrible DX.

Fabianx’s picture

Version: 8.0.x-dev » 7.x-dev
FileSize
1.65 KB
FAILED: [[SimpleTest]]: [MySQL] 41,126 pass(es), 2 fail(s), and 0 exception(s). View

Attaching a possible patch for 7.x

Fabianx’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: css_js_order_with_the-2380655-2.patch, failed testing.

David_Rothstein’s picture

Version: 7.x-dev » 8.1.x-dev

Looking at https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Asset%21A... and https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Asset%21A... it seems likely to me a version of this bug still exists in Drupal 8...

In Drupal 7, I think #2712993: Can't override the same CSS files multiple times just fixed it for drupal_add_css(), but a fix for drupal_add_js() would still need to be backported here.

Drupal 7 has a test in testRenderOverride() which failed as a result of this (only on PHP 7)... based on a brief look it seems to me like those tests may have gone missing in Drupal 8 though. So we could use new tests here too.

Fabianx’s picture

I actually like the fix in #2712993: Can't override the same CSS files multiple times more than my one here.

I was pretty surprised to find a patch here by me for this! :)

Again this won't ever fail in D8 as files are never replaced and _drupal_add_css / _drupal_add_js is deprecated, but it should be fixed anyway.

David_Rothstein’s picture

Well, the Drupal 8 code actually seems to contemplate the possibility that files can be replaced, e.g. from the above link:

          // Local and external files must keep their name as the associative
          // key so the same JavaScript file is not added twice.
          $javascript[$options['data']] = $options;

Perhaps it's not possible in practice though?.... Seems like it would require the same JavaScript file to be referenced by more than one library or something like that.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

xjm’s picture

Title: CSS/JS order with the same weight within a group is arbitrary » Make CSS/JS order with the same weight deterministic
Category: Bug report » Task
Priority: Major » Normal

@alexpott, @Cottser, @joelpittet, @lauriii, and I discussed this issue and agreed that while this is not great, there is not an actual known bug from it, so we agreed to handle it as a normal task to make the ordering deterministic.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.