claro.jquery.ui css assets may be added the page multiple times

🤔 is something preventing AjaxResponseAttachmentsProcessor::buildAttachmentsCommands() from filtering out assets from libraries that were already loaded on the page?

@gapple yeah I was debugging a lot through there, it's hard to figure out which dependency (of a dependency of a dependency) is adding the libraries in the first place but I was trying to follow the theme.css file which comes from the claro.jquery.ui library.

That library is added via libraries-extend for core/drupal.autocomplete, core/drupal.dialog and core/drupal.tabbingmanager

core/drupal.autocomplete is in drupalSettings.ajaxPageState on page load, and it's in the already attached libraries in that processor.

I have no idea what library is adding the CSS file again on the AJAX request, but it smells like something related to libraries-extend.

A fix and a test to avoid future regression.

Back to debugging this as it’s causing a lot of issues with our 10.1 upgrade.

Looking into AssetResolver::getCssAssets I can see 2 libraries adding core/themes/claro/css/components/jquery.ui/theme.css (btw it’s not specific to this file this is just the one wrecking the buttons) on initial page load:

• core/drupal.dialog
• core/drupal.tabbingmanager

Only 1 is added because the $css array is keyed by the asset url.

On the AJAX request, core/drupal.autocomplete is then added which also adds that CSS file.

getLibrariesToLoad only diffs the libraries, it doesn’t care about what CSS files have been added.

IMO this is a pretty major regression for 10.1. Again, this already happened in prior versions but because of the changes in 📌 Improve Drupal\Core\Ajax\AddCssCommand to accept an array of CSS assets Fixed these files are no longer prepended meaning they can override existing styles.

Other things that are affected by this (for us at least):

1. Using layout_builder_modal, the close button on the modal dialog has broken hover styles.
2. Also getting some whacky overlay issues with layout_builder_modal + a block with a WYSIWYG field in it when opening the Linkit dialog. The overlay doesn't disappear on save, meaning the page is not interactable.

I think I see - Claro's use of libraries-extend is merging the assets from claro/claro.jquery.ui to multiple libraries rather than adding it as a dependency of those libraries. (Looks like that happened here: #3113400-50: Deprecate more jQuery UI library definitions → ).

While the change to add_css's behaviour seems to be exposing this issue, I don't think it's particularly the fault of add_css - and I can see different problems if the behaviour was changed to prepend css assets to the head element (e.g. Theme CSS being inserted via AJAX response before module CSS already on the page, so that selectors with equal specificity aren't applied in the right order).

I'm hoping the MR will address this particular issue of duplicate assets by allowing proper library resolution.

I think how/where assets from AJAX responses are added in relation to existing assets is a bigger, messier issue, that's going to have issues in certain cases with both prepending or appending new assets to the document head.

Amazing @gapple! That has fixed the first issue for me. I think the layout_builder_modal thing may be similar but is probably a bit more nuanced (the module renders the modal in the admin theme, the CSS file that's breaking the styles is core/assets/vendor/jquery.ui/themes/base/theme.css so not Claro)

I don't think we need to worry about prepended/appending, we need to stop duplicate files being added altogether. Rather than diffing libraries, it needs to diff the actual files attached (which, yes, will be very messy).

Great! I think that's enough for RTBC - I don't think a test would be necessary or helpful for this specific bug.

----

duplicate files would only be an issue if a file is added to multiple libraries, which I don't think should be happening if the library system is being used correctly - common files should be placed in a common dependency.

Agreed this would be great to get in.

I just tracked down another issue with duplicate CSS files, this time assets/vendor/jquery.ui/themes/base/core.css

which I don't think should be happening if the library system is being used correctly

That's the issue, it's not. Look at drupal.autocomplete and drupal.dialog - lots of dupes there. Which is exactly what caused this 2nd issue (core.css duplicated and overriding z-index styles).

Slack monologue https://drupal.slack.com/archives/C1BMUQ9U6/p1692681337738279

Agreed a test wouldn't be useful here, we'd just be testing the Claro library definitions match a certain pattern. Looks like we should have done some kind of library.name.deprecated trick with the jquery_ui deprecations instead of my hack to directly include the files :/

Committed/pushed to 11.x and cherry-picked to 10.1.x, thanks!

Automatically closed - issue fixed for 2 weeks with no activity.

Closed 🐛 Background colour of UI widgets get overridden on Ajax load. Active as a duplicate

smustgrave → credited KelvinWong → .

I reopened 🐛 Background colour of UI widgets get overridden on Ajax load. Active because there is another underlying issue that this hasn't solved. We don't get the css loaded multiple times, but we do get inconsistent order of css files loaded, and that results in the same issue with missing button backgrounds.