Opened 8 months ago
Closed 4 months ago
#56830 closed defect (bug) (fixed)
jQuery Migrate deprecation in wpdialog
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.2 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Administration | Keywords: | has-patch |
Focuses: | javascript | Cc: |
Description
(Follow up to #51812.)
WP Core's customized jQuery UI widget wpdialog
produces a jQuery Migrate deprecation notice in the browser's console, about the focus()
shorthand being decprecated.
/src/js/_enqueues/lib/dialog.js
's line
this.element.focus();
should be changed to
this.element._trigger('focus');
from what I can see.
Attachments (4)
Change History (21)
#4
@
4 months ago
- Resolution fixed deleted
- Status changed from closed to reopened
I'm now getting
Uncaught TypeError: this.element._trigger is not a function at $.<computed>.<computed>.open (wpdialog.js?ver=6.2-alpha-55114:20:17
errors.
It looks like the custom jQuery UI _trigger()
(note the _
) is not correct here. Instead, using jQuery's trigger()
works for both getting rid of the deprecation and doesn't throw that error.
trigger( 'focus' )
is actually also the functional equivalent to the shorthand focus()
. My initial suggestion as bit overambitious here...
#5
@
4 months ago
- Keywords has-patch commit removed
Thanks for reopening the ticket @TobiasBg. Could you please share detailed reproduction steps to get this error log?
#6
@
4 months ago
Sure! In my plugin TablePress, I'm creating a wpdialog. Once I open that, I see the error.
(Quickest way: Install the plugin, add a new table, and hit Cmd+P on the "Edit" screen.)
It seems that a pure call to
jQuery( '#selector_id' ).wpdialog( {} );
with some existing #selector_id
element is sufficient, without any options being set.
#7
follow-up:
↓ 8
@
4 months ago
Tried to reproduce the issue but couldn't. Also note that /src/js/_enqueues/lib/dialog.js's link already contains the
this.element._trigger('focus');
#8
in reply to:
↑ 7
@
4 months ago
Replying to shubham1gupta:
Tried to reproduce the issue but couldn't.
Can you clarify what part? The original deprecation notice or the (new) JS error?
Also note that /src/js/_enqueues/lib/dialog.js's link already contains the
this.element._trigger('focus');
Yes, that's since [55052]. I'm suggesting to change _trigger
to trigger
now (no underscore).
#9
@
4 months ago
Can you clarify what part? The original deprecation notice or the (new) JS error?
The New JS error, I tried using the following steps:
Uncaught TypeError: this.element._trigger is not a function at $.<computed>.<computed>.open (wpdialog.js?ver=6.2-alpha-55114:20:17
- Installed TablePress, created a new table
- Pressed Ctrl + P, a dialog comes up. I checked the browser console attached screenshot above.
#10
@
4 months ago
Thanks for the explanation! From what I can see in your screenshot (version string in the load-scripts.js file) you are testing with WordPress 6.1.1.
The error will not happen there, because commit [55052] will only be part of WordPress 6.2.
You would therefore have to test this with WordPress trunk.
#11
follow-up:
↓ 12
@
4 months ago
Some more findings:
this
inside the open()
function of wpdialog
refers to the jQuery UI widget, whereas this.element
is the jQuery object of the DOM element. Thus, native jQuery methods need to used, as _trigger
is only defined on the jQuery UI widgets (in core.js).
#12
in reply to:
↑ 11
@
4 months ago
Replying to TobiasBg:
Some more findings:
this
inside theopen()
function ofwpdialog
refers to the jQuery UI widget, whereasthis.element
is the jQuery object of the DOM element. Thus, native jQuery methods need to used, as_trigger
is only defined on the jQuery UI widgets (in core.js).
Ah, totally makes sense, thanks for investigating 👍
#14
follow-up:
↓ 15
@
4 months ago
Thanks for the patch. I'm not sure that getting rid of the element
here is the proper way though. This could have many implications and would require a lot of testing (especially as this is mainly needed to work around Webkit things).
I would suggest to go with this.element.trigger('focus');
as all we really want here for now is to silence a deprecation notice.
#15
in reply to:
↑ 14
@
4 months ago
Replying to TobiasBg:
Thanks for the patch. I'm not sure that getting rid of the
element
here is the proper way though. This could have many implications and would require a lot of testing (especially as this is mainly needed to work around Webkit things).
I would suggest to go withthis.element.trigger('focus');
as all we really want here for now is to silence a deprecation notice.
Ah! That makes so much sense, thank you!
The patch works fine on my side. Self assigning for
commit
consideration.