Make WordPress Core

#56830 closed defect (bug) (fixed)

jQuery Migrate deprecation in wpdialog

Reported by: tobiasbg's profile TobiasBg Owned by: audrasjb's profile audrasjb
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)

56830.diff (401 bytes) - added by elifvish 14 months ago.
wordpress.png (55.8 KB) - added by shubham1gupta 11 months ago.
56830.2.diff (403 bytes) - added by shubham1gupta 11 months ago.
56830.3.diff (411 bytes) - added by shubham1gupta 11 months ago.

Download all attachments as: .zip

Change History (21)

@elifvish
14 months ago

#1 @elifvish
14 months ago

  • Keywords has-patch added; needs-patch removed

#2 @audrasjb
11 months ago

  • Keywords commit added
  • Owner set to audrasjb
  • Status changed from new to accepted

The patch works fine on my side. Self assigning for commit consideration.

#3 @audrasjb
11 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 55052:

Code Modernization: Fix a jQuery Migrate deprecation in wpdialog.

This changeset replaces a focus() shorthand in WP Core's customized jQuery UI widget wpdialog to avoid a jQuery Migrate deprecation notice in the browser's console.

Props TobiasBg, elifvish.
Fixes #56830.

#4 @TobiasBg
11 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 @audrasjb
11 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 @TobiasBg
11 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: @shubham1gupta
11 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 @TobiasBg
11 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 @shubham1gupta
11 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

  1. Installed TablePress, created a new table
  2. Pressed Ctrl + P, a dialog comes up. I checked the browser console attached screenshot above.

#10 @TobiasBg
11 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: @TobiasBg
11 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 @audrasjb
11 months ago

Replying to TobiasBg:

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).

Ah, totally makes sense, thanks for investigating 👍

#13 @shubham1gupta
11 months ago

@TobiasBg Thank you for the insights!

#14 follow-up: @TobiasBg
11 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 @shubham1gupta
11 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 with this.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!

#16 @audrasjb
11 months ago

  • Keywords has-patch added; good-first-bug removed

Alright, thanks for the reproduction steps and various investigations @TobiasBg.
I can confirm this.element.trigger('focus'); fixes the issue.

Gonna commit this in a few mins.

#17 @audrasjb
11 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 55134:

Code Modernization: Fix a JS error in wpdialog.

This changeset replaces this.element._trigger('focus'); with this.element.trigger('focus'); in wpdialog to fix a JS error introduced in [55052].

Indeed, 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 be used, as _trigger is only defined on the jQuery UI widget.

Follow-up to [55052].

Props TobiasBg, audrasjb, shubham1gupta.
Fixes #56830.

Note: See TracTickets for help on using tickets.