Problem/Motivation

This module defines totally separate libraries from Drupal Core, which, at least as of the latest 10.x version, simply includes jquery files in the definitions of a few dependent libraries such as drupal.dialog.

This means that if core libraries like drupal.dialog are attached at the same time as this contrib's own libraries, the same jQuery UI assets may be included twice from different paths. (Core's version, and the contrib's version.)

This is definitely problematic in the context of AJAX: it is possible only one version will be attached to the initial page render, with the other only being added during a subsequent AJAX call. In Drupal 10.1.0, additional CSS assets are appended to the bottom of <head>.

Steps to reproduce

Install Focal Point 2.0.0 exactly on a Drupal 10.1 site using the Standard profile site, or otherwise having the media module installed and an image-based media type configured.

Configure the media library form display for the Image media type to use the Focal Point widget.

Configure a media reference field on a node type. Configure the field's form display to use the Media Library widget.

Edit a node of that type, and use the media field to open a media library dialog. Upload a new file. The modal overlay will display on top of the modal, because the jquery_ui module's version of core.css is appended near the bottom of head.

Proposed resolution

One possibility that comes to mind: add code to the libraries_info_alter hook in this module to crawl core libraries that still use jQuery UI, such as drupal.dialog. Replace the core paths with the assets from this module. (I have not tried this out, however, and I am not actually certain if it would prevent the above issue with Focal Point. Depends on whether Drupal's asset aggregation system detects duplicate paths used by multiple libraries.)

I am not certain if this would be the right approach. For one thing it assumes that the behavior of Drupal 10.1.0, appending new CSS to the bottom of in AJAX, is not considered a regression/bug of its own.

This would also open up the possibility of this module substituting a version of jQuery UI different from that shipped with the user's version of core, which could be problematic.

Remaining tasks

Determine a path forward.

Issue fork jquery_ui-3371462

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

bvoynick created an issue. See original summary.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Confirming the issue from #3371179: Overlay blocking image upload modal in Drupal 10.1, taking a look at the changes, I'm not sure if it was focal_point's fault or is wrong logic in the jquery_ui module? Can the jquery_ui maintainers tell easily perhaps?

In general, I can say that it's of course unexpected that with jquery_ui installed, the CSS file priority is different. That bug was really hard to find. The priority / loading order should stay the same, as described in the proposed resolution, but focal_point declared dependencies in a given order that may also have caused this specific bug.

See Drupal 10.1 change records for details how the aggregation system was changed, which may have caused this bug: https://www.drupal.org/project/drupal/releases/10.1.0

bvoynick’s picture

bvoynick
he/him
Primary language English
commented
Issue summary: View changes
bvoynick’s picture

bvoynick
he/him
Primary language English
commented
Issue summary: View changes
catch’s picture

catch
he/him
Primary language English
commented

Using one set of files sounds good, but I don't think we detect duplicate files as such. I might be better to force the jquery_ui libraries as dependencies of the core libraries, then remove the core versions of the files from those library definitions - then you know that one file is coming from one library definition.

This could potentially be related to #3222107: Library order asset weights do not work properly when a large number of javascript files is loaded between two jQuery UI libraries, or it could be a side effect of the asset generation changes themselves.

Can you check what happens with aggregation disabled? If so that would narrow it down to #3222107: Library order asset weights do not work properly when a large number of javascript files is loaded between two jQuery UI libraries a bit more closely.

bvoynick’s picture

bvoynick
he/him
Primary language English
commented

It does still happen with aggregation disabled. I think it's down to there being two distinct assets in the two distinct libraries.

Removing core's asset definitions and substituting dependencies on the jquery_ui module's libraries sounds like a good path forward, thanks.

bvoynick opened merge request !8

bvoynick’s picture

bvoynick
he/him
Primary language English
commented
Status: Active » Needs review

Opened MR 8 with an implementation.

New jquery_ui_library_info_alter() logic replaces core library jQuery UI files with dependency declaration(s) where possible. Where that is not possible, due to the site not having the sub-module for a particular JQUI library installed, the file definition in the core library is instead replaced using the file & definition from the jquery_ui library data.

At first I tried just replacing the files that are actually present in declared libraries with dependencies - and leaving any other JQUI assets in core libraries alone. However, I found this caused ordering problems. The jquery_ui module uses a -11 weight across the board for its JS files, and the recommended approach of dependencies within a network of library definitions to establish ordering. Core's drupal.autocomplete and drupal.dialog libraries - as well as all the jquery_ui* libraries included in 9.x - take a different approach, declaring everything in one library, and using lesser weights like -11.9 and -11.8 to order files within the one library. On a test site without certain submodules like jquery_ui_autocomplete enabled, that was resulting in dependent files that were still declared on the core library - like autocomplete-min.js - being included before core library dependencies like widget-min.js that are always going to be provided by the jquery_ui/core library. Which results in javascript errors.

So I switched to re-working all assets found in core libraries, by one method or another, based on a comprehensive mapping of jquery_ui.libraries.data.json.

recrit’s picture

recrit commented
Status: Needs review » Needs work

Changing this back to needs work since there is nothing to compare in the MR.

recrit’s picture

recrit commented

See #3420890: Remove duplicate jQuery UI JavaScript and CSS files that has a fix for all core libraries by replacing the individual files instead of adding a library dependency which can cause resolved library weighting issues.