Follow-up: Replace deprecated jQuery methods used in core scripts

[indigoxela]

Description of the Task

This is a follow-up to #3106, which added jQuery 3.7.0 to core (with a fallback to 1.12.4).

Methods that have been removed from jQuery already got handled there, but many deprecated functions are still in use in core.
And this issue exists to address that.

Examples for such deprecated functions:

• bind() instead of on()
• click() instead of on('click'...
• $.isArray() instead of Array.isArray()
• $.isFunction instead of typeof
• $.trim() instead of the native string .trim()

These deprecated functions don't cause any problems currently, but getting rid of them is necessary to be ready for jQuery updates, especially the ones that will finally remove those methods.

Luckily, there's a helper tool to find such usages: https://github.com/jquery/jquery-migrate/

None of the changes will (and should) have any impact on functionality, nor should it change anything for contrib.

[indigoxela]

A PR is available, a lot (hopefully most) deprecated code got updated, 32 files so far.
I used jquery-migrate-3.4.1.js and clicked through the UI, to catch its console nagging. So (fingers crossed) everything I touched still works. Many, but not all changes are trivial. I didn't want to blindly rewrite code.

There are external scripts, that also cause nagging, but these (jquery.form.js, jquery.once.js... probably should get discussed in other issues (newer versions available).

There are still usages of "bind" and the like, but these are either trickier to understand, or I didn't figure out yet, when those get executed, anyway. That's why it's work in progress.

[argiepiano]

Thanks so much, @indigoxela! I've read through the changes (but have not tested). I've added a couple of comments.

[indigoxela]

I wonder, if it helps with testing, if I temporarily include the jquery-migrate-3.4.1.js script. Testers could then also see deprecation logging in their browser console.

What do people think? Is it worth it and useful, or not?

[indigoxela]

I think, I caught them all now 😏, so I guess, my PR's ready for testing and review.

For further reading: https://api.jquery.com/category/deprecated/deprecated-3.0/ (and newer deprecations).

[argiepiano]

Wow, tons of great work here, thanks! I've read through the changes and they look good.

I wonder, if it helps with testing, if I temporarily include the jquery-migrate-3.4.1.js script

That sounds like a good idea.

Like #3106 it may help merging these changes as soon as possible to facilitate testing.

[indigoxela]

@argiepiano I've (temporarily) added jquery-migrate-3.4.1.js for easier testing. And disabled js aggregation in the PR sandbox.

You will notice some console nagging, for example triggered by contextual.js, which is actually caused by jquery.once. Ajax in forms triggers deprecation nagging caused by jquery.form. I'd suggest to ignore that here and create new issues for external code. What do you think?

[indigoxela]

Two weeks later 😀 I decided to remove the debugging code again. Still testing and review is highly appreciated.

All automated checks are passing, BTW.

[indigoxela]

Some hints, how to test: no functionality changes, but as the PR changes lots of js files (although tiny changes), testers could have a look at UI items for fields (add, edit...), layouts, date widgets, node form (e.g. vertical tabs)...

BTW: I only touched code, for which I could figure out where it runs and what it does. And checked that everything still works as before.

[quicksketch]

I did a full read-through and only found one possible problem: backdrop/backdrop#4463 (review)

I tested all the common JS functionality when setting up a content type, creating content, creating/editing menus, configuring a layout, configuring a view, enabling a module, and color module theme support. No issues and no JS errors anywhere that I can see.

Great job @indigoxela!!

Code-review-wise, I'm comfortable with all these changes. I would appreciate another broad manual testing review (@klonos or @kiamlaluno maybe?).

[indigoxela]

@quicksketch many thanks for testing and reviewing. 🙏 I've committed your improvement. 👍

[quicksketch]

Changes look good. I re-tested vertical tab summaries and it still works as before. 👍

[kiamlaluno]

I am not sure it is related to these changes, but one of the tests fails with Failed to set field uninstall[locale] to 1. The test fails only for PHP 8.2; it could be a false positive or caused by a difference between PHP 8.2 and previous versions.

[kiamlaluno]

I played with the preview site. On Console, I only got two warnings.

• Layout was forced before the page was fully loaded. If stylesheets are not yet loaded this may cause a flash of unstyled content.
• Synchronous XMLHttpRequest on the main thread is deprecated because of its detrimental effects to the end user’s experience. For more help http://xhr.spec.whatwg.org/

Those warnings were shown even when I changed to jQuery 1.x. They are not related to the changes done in this MR.

[indigoxela]

@kiamlaluno many thanks for taking the time to test! 🙏

Re the failing locale test - completely unrelated.
Re synchronous XMLHttpRequest (and rendering unstyled content) - that's not new and unrelated to the current changes (Firefox?). But maybe worth further investigation in a different issue (which might already exist)?

[quicksketch]

I reran the failing test and it passed the second time; that uninstall test is one of the intermittent failures we get on all versions of PHP. Seems like this is good to go, and should probably be 1.x (1.26.0) only?

[indigoxela]

that uninstall test is one of the intermittent failures we get...

Yes, I see than one fail occasionally, not sure what the trigger is.

... and should probably be 1.x (1.26.0) only?

The changes are compatible with both jQuery versions, so it's up to core committers to decide, whether it's appropriate for 1.25.x or better 1.26.x. 😉

Possibly, if the addition of jQuery 3.7 is seen as an overall modernization task, 1.26 would be a good fit.

[quicksketch]

For now I've only merged backdrop/backdrop#4463 into 1.x to be a little on the conservative side. Thanks @indigoxela, @argiepiano, and @kiamlaluno for your work here!