jQuery Migrate deprecation in wpdialog

[TobiasBg]

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

[audrasjb]

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

[audrasjb]

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.

[TobiasBg]

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

[audrasjb]

Thanks for reopening the ticket @TobiasBg. Could you please share detailed reproduction steps to get this error log?

[TobiasBg]

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.

[shubham1gupta]

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');

[TobiasBg]

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

[shubham1gupta]

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.

[TobiasBg]

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.

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

[audrasjb]

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 👍

[shubham1gupta]

@TobiasBg Thank you for the insights!

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

[shubham1gupta]

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!

[audrasjb]

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.

[audrasjb]

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.