One of the ways CSS determines what property value to use when selectors have the same specificity is their source order, with later instances overriding earlier ones; this also applies to CSS files referenced via <link> elements, where their order in the HTML document/DOM also determines which property value is used when selectors have the same specificity, with later files' values being used over earlier ones. Unfortunately, Turbo breaks this in some edge cases by simply appending the <link> elements like so:
document.head.appendChild(element)
This is present in both the current stable version (7.3.0) and the most recent commit at the time of writing.
This is most easily demonstrated on the Gin admin theme in dark mode with widgets such as dropbuttons and vertical tabs; doing a full page load on a page that doesn't have them and then navigating to a page that does results in the Claro CSS <link> elements to be appended to the <head> after the Gin CSS that would normally override the Claro colours with dark mode colours, so the dark mode Gin overrides are replaced with the Claro rules of the same specificty, instead of the other way around.
Note that this primarily manifests with CSS aggregation off as aggregation preserves the correct order/weights when combining CSS files, and the observed occurrences seem to get aggregated into the same aggregate files, thus fixing this issue in a roundabout way.
Dropbuttons with incorrect specificity:
Dropbuttons with correct specificity:
Vertical tabs with incorrect specificity:
Vertical tabs with correct specificity:
We should first open an issue on the Turbo GitHub with a really simple reproducible case to get that ball rolling. Example of how we can implement a minimal reproduction in this Turbo pull request by Matt Yorkley.
If that doesn't get resolved soon enough and it turns out it's more severe for us, we can try to implement a workaround to re-order the inserted <link> elements.
In the front-end, this can be accomplished via a MutationObserver or (better yet) via one of the Turbo events as CSS files seem to delay triggering of the turbo:render event while they load.
In the back-end, we would have to pass this information to the front-end, and preferably in an efficient way that doesn't require a lot of work for both PHP and the browser. We can probably just output the CSS file paths in the sorted order via the asset.resolver service (see AssetResolver::getCssAssets() on api.drupal.org and in GitLab).
See previous heading.
Stuff less borked.
None?
None.
Comments
Comment #2
ambient.impactExpanded proposed resolution with links to how we might work around this in Drupal.
Comment #3
ambient.impactComment #4
ambient.impactSome rewording of "Proposed resolution"
Comment #5
ambient.impactForgot to finish a sentence under "Steps to reproduce" whoops.
Comment #6
ambient.impactAdded link to example of how we can implement a minimal reproduction in this Turbo pull request by Matt Yorkley.
Comment #7
ambient.impactAdded screenshots demonstrating the issue.
Comment #8
ambient.impactLinking screenshots in issue summary.
Comment #10
ambient.impactThe workaround I've committed is okay for now, but still results in brief flashes of incorrect specificity, so I'm going to set this as postponed for the time being.
Comment #12
alex.skrypnyk@ambient.impact
Thank you for your hard work on this module! It is really great!
Unfortunately, your last committed change causes browsers to recalculate styles, triggering FOUC, which essentially made this module unusable in production.
The current implementation appends (takes out of the DOM and adds to the bottom) all of the styles for each of the stylesheet that changed the order resulting in a lot of unnecessary DOM mutations, which triggers FOUC.
Can this please be reverted or implemented differently to only moving elements that are actually out of order (often just 1-2, or none at all). I'm attaching a patch with a fix to `#sort()` function that would only move the stylesheets with update ed order.
Comment #13
ambient.impact@alex.skrypnyk Thank you for the patch and good catch. I've created an issue fork in #3500115: Turbo: Still encountering FOUC during RefreshLess loads and adapted your patch to that. I'm going to test the changes out on a couple of sites and will merge when I'm done with that.