Problem/Motivation
Drupal 12 ships jQuery 4 and removes the jQuery Dialog jQuery-event shim, so several of Entity Browser's JavaScript behaviours break on Drupal 12. Verified on a real Drupal 12 (jQuery 4) install, there are three distinct problems:
- jQuery 4 removed the deprecated event methods —
.bind()/.unbind()and the shorthand event helpers (.click(),.focus(),.resize(fn), …). - The Dialog jQuery events lost their extra arguments — core removed
dialog-deprecation.jsin D12, sodialog:aftercreate/dialog:beforeclosehandlers no longer receive the(event, dialog, $element, settings)args.js/entity_browser.modal.jsthen throwsCannot read properties of undefined (reading 'find')when opening the modal (seen in the next-major CI job on #3613186). - jQuery 4 removed
jQuery.proxy()— still used incommand_queue.jsandview.js; it throws$.proxy is not a functionduring behavior attach, which cascades (e.g. the views widget fails withCannot read properties of undefined (reading 'iframe')).
Proposed resolution
1. Mechanical event-method swap across js/ — no behaviour change, these are exact aliases on the current jQuery 3.x:
.bind(ev, fn)=>.on(ev, fn).unbind(ev)=>.off(ev)- shorthand triggers
.click()/.focus()=>.trigger('click')/.trigger('focus') - shorthand handlers
.resize(fn)/.click(fn)=>.on('resize'|'click', fn)
2. Migrate the dialog event handlers to the D12 event API — read the element off the event (var $element = $(event.target);) instead of the removed handler argument. Equivalent on 10.x/11.x (the BC shim passes $($event.target) anyway), works on 12.x without the shim.
3. Replace jQuery.proxy(fn, ctx) with native fn.bind(ctx) in command_queue.js and view.js (also what eslint's no-jquery/no-proxy asks for).
Remaining tasks
- Review + merge the JavaScript sweep.
- Verify against the next-major (jQuery 4) CI job once #3613186 lands and this rebases (that issue enables next-major testing).
Issue fork entity_browser-3613381
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:
Comments
Comment #5
velmir_taky commentedThanks @yusuf_khan — looks like we pushed the same sweep at once (!152 vs !151), sorry for the overlap!
Heads up though: the mechanical
.bind => .onswap is necessary but not sufficient. Themodal.jscrashCannot read properties of undefined (reading 'find')isn't a jQuery 4 removal — it's the Dialog event API migration. D12 dropsdialog-deprecation.js, sodialog:aftercreateno longer passes the extra(event, dialog, $element, settings)args =>$elementisundefined=>$element.find(...)throws (modal.js:99).Fix: read it off the event =>
var $element = $(event.target);. Equivalent on 10.x/11.x (the shim passes$($event.target)anyway), works on 12.x without the shim.Verified on real D12 (jQuery 4),
PluginsTest::testModalDisplay:TypeError … reading 'find' at dialog:aftercreate (modal.js:99), modal fails to open;!151 has the sweep plus this dialog fix (
7e6c393f), so I'd suggest we land !151 and close !152 — happy to credit you. Or I can move the commit onto !152, whatever's easier.Comment #6
yusuf_khan commented@velmir_taky Thanks for the heads-up and for catching that. I completely agree—the .bind() → .on() sweep addresses the jQuery 4 deprecations, but the dialog:aftercreate API change is a separate Drupal 12 compatibility issue.
Thanks for identifying the root cause and verifying the fix on a real Drupal 12 installation. Since !151 already includes both the mechanical replacements and the dialog event fix, I'm happy to close !152 and avoid duplicate work. No worries about the overlap—it happens!
Comment #7
velmir_taky commentedComment #8
velmir_taky commentedRan the full FunctionalJavascript suite against a real Drupal 12 (jQuery 4) install to make sure nothing else was hiding beyond the modal — and there was one more:
jQuery.proxy()/$.proxy()was removed in jQuery 4, and we still used it in two places:js/entity_browser.command_queue.js—setTimeout($.proxy(...));js/entity_browser.view.js—.each(jQuery.proxy(view_instance.attachExposedFormAjax, view_instance))(this one runs during the view widget behavior attach).On D12 that threw
$.proxy is not a functionduring attach, which cascaded and left the widget half-initialized — e.g.EntityBrowserViewsWidgetTestthen failed withCannot read properties of undefined (reading 'iframe'). Replaced both with nativeFunction.prototype.bind()(also what eslint'sno-jquery/no-proxyasks for). After that the views widget test is green on D12.So this MR now carries three JS fixes: the mechanical sweep, the dialog-event migration, and this
$.proxyremoval. An exhaustive scan ofjs/for jQuery-4-removed APIs is clean now.For the record, on a real D12 the entity_browser JS paths I could run pass (modal display, views widget, reference widget config, etc.). The remaining reds in my local run are all either missing test cross-dependencies (token/inline_entity_form/entity_embed/paragraphs/…) or local Selenium timing flakiness that reproduces identically on 11.x — not from this change.
Comment #11
berdirNot very familiar with this, but only 5 next major tests are now failing with this, all of them IEF or entity query integrations and current and previous major are green, so lets get this in.
With just those tests failing, I think I'd be OK with also adding back the £D12 compatibility, but it should also unblock other contrib now by explicitly requiring the dev version for now.