Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 11 months ago

#50564 closed task (blessed) (fixed)

Update jQuery step two

Reported by: azaozz's profile azaozz Owned by: azaozz's profile azaozz
Milestone: 5.6 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords: early has-patch needs-testing
Focuses: javascript Cc:

Description (last modified by desrosj)

Follow up from #37110.

Update to latest jQuery, latest jQuery Migrate, latest jQuery UI.

This ticket represents step 2 in the following roadmap for updating jQuery to 3.x in Core: https://make.wordpress.org/core/2020/06/29/updating-jquery-version-shipped-with-wordpress/

Attachments (2)

50564.diff (741.1 KB) - added by azaozz 4 years ago.
50564.2.diff (22.9 KB) - added by azaozz 4 years ago.

Download all attachments as: .zip

Change History (56)

#1 @desrosj
4 years ago

  • Description modified (diff)
  • Keywords needs-patch added

Adding the blog post on Make Core to the ticket description for full context.

#2 follow-ups: @mgol
4 years ago

One thing that may be important here - while we (the jQuery Core team) generally try to follow semver right now and only do breaking changes in major releases, we had to do a breaking change in 3.5 to resolve a security issue. Details on that change are included in the 3.5 upgrade guide: https://jquery.com/upgrade-guide/3.5/.

Note that Migrate will not automatically restore previous behavior here as we prefer Migrate to not reintroduce security issues by default. If this upgrade is too much for you to do at once, you can call:

jQuery.UNSAFE_restoreLegacyHtmlPrefilter();

to restore the older insecure behavior, as indicated in the upgrade guide. This is a Migrate API so it's only available if you load a new enough Migrate version.

You can then plan to remove this call in a future version.

Last edited 4 years ago by mgol (previous) (diff)

This ticket was mentioned in Slack in #core by mgol. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by desrosj. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by helen. View the logs.


4 years ago

#7 @TimothyBlynJacobs
4 years ago

Just sharing a comment I made in the dev chat:

If we upgrade to jQuery 3.0 do we anticipate using any jQuery 3.0 features? Or if WordPress Core won't be reliant on jQuery Migrate, ie we wouldn't be using any deprecated features, will that be possible to do while maintaining jQuery 1.x compatibility?

If we do anticipate needing to use jQuery 3 features, I think it’d leave people currently relying on the jQuery Migrate plugin without a path forward to the latest core. It would also mean plugin authors probably wouldn’t be able to safely use jQuery 3.0 features.

#8 in reply to: ↑ 2 @azaozz
4 years ago

Replying to mgol:

Note that Migrate will not automatically restore previous behavior here as we prefer Migrate to not reintroduce security issues by default.

Thanks for the clarification. Thinking it would be best if the WP implementation leaves this out for now, i.e. keeps the security fix. Then, if there are (lot of) problems during testing of WP 5.6, enable it temporarily for one release cycle.

@TimothyBlynJacobs:

If we upgrade to jQuery 3.0 do we anticipate using any jQuery 3.0 features?

This is a "tough question", but something we'll have to go through. A good, backwards compatible way seems to be to not use any jQuery 3.x features for (at least) the release cycle when jQuery is updated, i.e. start using them in WP 5.7. That gives plenty of time to everybody to update their code.

WP 5.7 is also the time to fix/refactor all core js to work without jQuery Migrate 3.x. Currently there are several scripts that are not compatible. This will also be the most "breaking" step for old plugins and themes, and will probably need some safeguards in order to do it right.

Last edited 4 years ago by azaozz (previous) (diff)

This ticket was mentioned in Slack in #core by sergey. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

@azaozz
4 years ago

#11 @azaozz
4 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

In 50564.diff:

  • Update jQuery to 3.5.1, and jQuery Migrate to 3.3.1.
  • Update jQuery UI to 1.12.1. This also brings jQuery UI in the core repository (vs. adding it from "node_modules") as the build process in UI 1.12.1 has changed quite a bit and doesn't match how it is used/enqueued in WP.

To keep backwards compatibility the (pre-built) jquery-ui.js was split in three parts:

  • All effects are concatenated in a (new) jquery/ui/effects.js.
  • All files from "group: Core" are concatenated in /ui/core.js. That also contains 'jquery-ui-position' and 'jquery-ui-widget'.
  • The files from "group: Widgets" and "group: Interactions", like draggable.js, droppable.js, sortable.js, etc. are still enqueued separately to optimize loading.
Last edited 4 years ago by azaozz (previous) (diff)

#13 @azaozz
4 years ago

(The above PR is fresher than 50564.diff.)

@ocean90 Seems you've last worked on adding/updating jQuery UI in core. Could you have a look please, mostly at the UI part :)

Looks like the old UI files in WP can be loaded using AMD. Wondering if WP should try to maintain that after the changes in the UI sources in 1.12.1?

Last edited 4 years ago by azaozz (previous) (diff)

#14 @desrosj
4 years ago

  • Type changed from defect (bug) to task (blessed)

Changing this to a task to more accurately reflect what's happening here.

#15 @azaozz
4 years ago

In https://github.com/WordPress/wordpress-develop/pull/536:

  • Add jquery-migrate.js v.3.3.1 to core and load it with $suffix instead of $dev_suffix (meaning the non-minified versions will be loaded on the front-end when SCRIPT_DEBUG is true).
  • Add jquery.min.js, update jquery.js to 3.5.1 non-minified. This should help when debugging and can be reverted in a future release.
  • Rebuild jQuery UI 1.12.1 and add it to core. The build process has changed quite a bit in 1.12.1. In order to keep the optimized loading (and maintain AMD/requirejs compatibility) ui/core.js had to be rebuild completely. Also some changes were needed in all other files. It is possible to pull the source and build UI in our Grunt but seems quite complex, all source files need edits.
  • Adjust tests to match the above changes.

Thinking this is ready. Please test! :)

Last edited 4 years ago by azaozz (previous) (diff)

#16 @azaozz
4 years ago

In 49101:

Update jQuery step two:

  • Add jquery-migrate.js v.3.3.1 to core and load it in debug mode when SCRIPT_DEBUG is true.
  • Add jquery.min.js, update jquery.js to 3.5.1 non-minified. This should help when debugging.
  • Rebuild jQuery UI 1.12.1 and add it to core.
  • Fix/adjust tests to match the above changes.

See #50564.

#17 @Hareesh Pillai
4 years ago

Now that jQuery UI is updated as part of this ticket, we can close #39943.

Last edited 4 years ago by Hareesh Pillai (previous) (diff)

#18 @Hareesh Pillai
4 years ago

I tried to run QUnit after this update and notice some console errors triggered by the migrate script, which should be fixed.

#19 @Mista-Flo
4 years ago

Hi there,

since the last commit in trunk, I have got some console errors and bugs on both classic and block editor.

mouse.js?ver=1.12.1:31 Uncaught TypeError: $.widget is not a function
    at mouse.js?ver=1.12.1:31
    at mouse.js?ver=1.12.1:22
    at mouse.js?ver=1.12.1:24
postbox.js?ver=5.6-alpha-48683-src:369 Uncaught TypeError: $(...).sortable is not a function
    at Object.init (postbox.js?ver=5.6-alpha-48683-src:369)
    at Object.add_postbox_toggles (postbox.js?ver=5.6-alpha-48683-src:260)
    at HTMLDocument.<anonymous> (post.js?ver=5.6-alpha-48683-src:312)
    at i (jquery.js?ver=3.5.1:2)
    at Object.fireWith [as resolveWith] (jquery.js?ver=3.5.1:2)
    at Function.ready (jquery.js?ver=3.5.1:2)
    at HTMLDocument.J (jquery.js?ver=3.5.1:2)

I use the wordpress-develop environment, with src (instead of build) folder.

I can reproduce with twentytwenty, twentyseventeen themes.
I can reproduce without any plugin enabled, and with classic editor and gutenberg plugin enabled.

For example when creating a post with classic editor, I'm not able to switch from Visual to Text mode.

Can you also reproduce?

I do not reproduce the issue if I checkout the commit sha just before the jquery update 2.

git checkout 0d313839e67fb446f5c70cf01c439c8426014f59

#20 follow-up: @azaozz
4 years ago

@hareesh-pillai Yeah, there are a bunch of warnings from jquery-migrate.js on pretty much all screens when you're running with SCRIPT_DEBUG. Most are about deprecated event shorthands like JQMIGRATE: jQuery.fn.click() event shorthand is deprecated. Should be pretty easy to fix these for 5.6 :)

@Mista-Flo hmm, cannot reproduce, running from either /build or /src. The UI widget is now part of ui/core.js, perhaps the browser is still using an old cached file? Could you try refreshing caches, and if still not working go to /src/wp-includes/js/jquery/ui/core.js?ver=1.12.1 and see if there's a $.widget = function... about 2/3 down.

If that's not there, i.e. the file wasn't "built" properly, try deleting /src/wp-includes/js and running build again. In some rare cases a file may get the wrong permissions, etc.

#21 @keraweb
4 years ago

Running the nightly build locally I got some jQuery weirdness, probably related to this.

$(elem).css( { 'top': '+=10' } );

This call gets run twice so the element gets a top of 20px instead of 10.
My first guess is that this might have something to do with migrate but I can't pinpoint the actual reason yet.

#22 in reply to: ↑ 20 ; follow-up: @Mista-Flo
4 years ago

Replying to azaozz:

@Mista-Flo hmm, cannot reproduce, running from either /build or /src. The UI widget is now part of ui/core.js, perhaps the browser is still using an old cached file? Could you try refreshing caches, and if still not working go to /src/wp-includes/js/jquery/ui/core.js?ver=1.12.1 and see if there's a $.widget = function... about 2/3 down.

If that's not there, i.e. the file wasn't "built" properly, try deleting /src/wp-includes/js and running build again. In some rare cases a file may get the wrong permissions, etc.

Hi @azaozz,

Thanks for your answer, I tried to rm -rf node_modules folder, npm install again, and npm run build, and I still have the same errors. I do not see any $.widget = function... in the file you mentioned.

So I try to delete the js folder inside src/wp-includes, and rebuild the assets, but it did not add all the needed files again, so I had to copy paste the same directory from the build folder to make it work, then with the build folder I don't have any console log errors, I just have a lot of jquery migrate console.log but no errors. I hope that can help.

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


4 years ago

#24 in reply to: ↑ 22 @azaozz
4 years ago

Replying to Mista-Flo:

So I try to delete the js folder inside src/wp-includes, and rebuild the assets, but it did not add all the needed files again, so I had to copy paste...

Hmm, that's strange. All UI files are now in WP, in /src/js/_enqueues/vendor/jquery/ui. They should get copied to /src/wp-includes/js/jquery/ui when running grunt build -dev or npm run build:dev, or npm run dev. All of these seem to work properly here. Not sure what could be going wrong, seems not possible that Gruntfile.js hasn't updated properly. Perhaps do another svn up/git pull (ensuring you're pulling from trunk), and try again?

#25 @malthert
4 years ago

I would like to add that all jQuery .ready() should be changed too in all WP included js.

As of 3.0 $( handler ) should be used instead of $( document ).ready( handler ) (and similar. see: https://api.jquery.com/ready/), as it is measurably faster and the recommended syntax.

Please let me know if I should create a separate ticket for this.

#26 @azaozz
4 years ago

In 49134:

Add svn:eol-style native to new files.

See #50564.

#27 @sabernhardt
4 years ago

@malthert A separate ticket for .ready() was opened today: #51519

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

mweichert commented on PR #536:


4 years ago
#29

Is this going into WP 5.6?

This ticket was mentioned in Slack in #core by clorith. View the logs.


4 years ago

#31 @Clorith
4 years ago

Before 5.6 code freeze, we should make sure to do a final pass of any jQuery/jQuery-migrate assets that may have received upstream updates (specifically as some inconsistencies were found in jQuery-migrate, see also #51621).

#32 @helen
4 years ago

In 49338:

External Libraries: Update jQuery Migrate to 3.3.2-pre.

This is a prerelease version to avoid some errors in 5.6 beta 2. We need to be sure that we ship with a released version by 5.6 RC.

Props mweichert.
Fixes #51621. See #50564.

#33 in reply to: ↑ 2 @TimoTijhof
4 years ago

Replying to mgol:

[…], we had to do a breaking change in 3.5 to resolve a security issue. Details on that change are included in the 3.5 upgrade guide: https://jquery.com/upgrade-guide/3.5/.

Note that Migrate will not automatically restore previous behavior here as we prefer Migrate to not reintroduce security issues by default. If this upgrade is too much for you to do at once, you can call:

jQuery.UNSAFE_restoreLegacyHtmlPrefilter();

to restore the older insecure behavior, as indicated in the upgrade guide. […]

As an anecdote, on Wikipedia and for the MediaWiki software, we enabled this to avoid broken plugins and theme widgets.

In a nut shell:

 $('<div> <span/> <p/> <u/> </div>')

This pattern was commonly used in Wikimedia codebases, and while we have (mostly) moved away from this in favour of more explicit templates, the pattern is still common in extensions and "user script" gadgets in the wild.

The pattern has also re-emerged to a small extent (by accident, and tolerated) in recent years due to frameworks like Svelte, Preact, and Vue popularizing similar shortcuts.

However, common as it might be, when parsed natively in a browser, it would <span/> as identical to <span>, thus leaving it open. The concept of self-closing elements doesn't exist in HTML. What does exist is that / is tolerated, and that some elements like <input>, <link>, and <br> are considered "void" (meaning, not "openeable"), so anything that follows them is naturally a sibling instead of a child.

Before jQuery 3.5.1, this would produce:

<div>
   <span></span>
   <p></p>
   <u></u>  
</div>

In jQuery 3.5.1+, this starts to produce:

<div>
   <span>
      <p>
         <u></u>
      </p>
   </span>
</div>

The way we're shipping this is by adding jQuery.UNSAFE_restoreLegacyHtmlPrefilter(); to the jquery+jquerymigrate payload.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

#35 @helen
4 years ago

@TimoTijhof Thank you so much for this concise explanation and clear example of what can happen. I suspect we have similar practices in userland, in particular in themes, which don't get very much pre-release testing outside of the default themes.

I'm leaving this open through RC so we continue the effort to communicate this outward and see if there are some static searches we can run against themes and plugins to find potential problems (example from @TimothyBlynJacobs: https://wpdirectory.net/search/01EQ9BZ9BTWZRDE8D225FC3W1M). I'm definitely a little skittish after seeing the fallout from 5.5, such as with .live(), which despite years of deprecation notices was still widely used. I think this is probably a significantly smaller issue than that but I'd rather show mindfulness.

#36 @SergeyBiryukov
4 years ago

In 49615:

Script Loader: Correct version for jQuery Migrate.

Follow-up to [49338].

See #51621, #50564.

This ticket was mentioned in Slack in #core by azaozz. View the logs.


4 years ago

@azaozz
4 years ago

#38 @azaozz
4 years ago

In 50564.2.diff: Update to the just released jQuery Migrate 3.3.2. Thanks @mgol!

#39 @azaozz
4 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 49649:

External Libraries: Update jQuery Migrate to 3.3.2.

Props mgol, azaozz.
Fixes #50564.

#40 @azaozz
4 years ago

  • Keywords commit dev-feedback added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopen for the 5.6 branch.

#41 @SergeyBiryukov
4 years ago

  • Keywords dev-reviewed added; dev-feedback removed

[49649] looks good to backport.

#42 @azaozz
4 years ago

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

In 49667:

External Libraries: Update jQuery Migrate to 3.3.2.

Props mgol, azaozz.
Merges [49649] to the 5.6 branch.
Fixes #50564.

#43 follow-up: @azaozz
4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening this to track the eventual need to add jQuery.UNSAFE_restoreLegacyHtmlPrefilter();. See comment 33 and comment 35.

Been trying to refine the plugins/themes directories searches. See: https://wpdirectory.net/search/01EQG05B6GSHPSECYTPH4KYNBP and https://wpdirectory.net/search/01EQG19QY1WX328JMSC7W6NQTJ. There are quite a few "hits" but most are for '<br /><br />' and even more (nearly all?) are in PHP (as far as I see).

In that terms wondering if circumventing the jQuery security fix in favor of a few plugins would be a good decision.

Version 0, edited 4 years ago by azaozz (next)

#44 @SergeyBiryukov
4 years ago

#51833 was marked as a duplicate.

#45 @a223123131
4 years ago

As mentioned in #51833
...the JQuery UI is a security issue Google is highlighting in Lighthouse. Think this should be in focuse compared with the other JQuery issues.

https://core.trac.wordpress.org/attachment/ticket/51833/lighthouse-bp.jpg

Last edited 4 years ago by SergeyBiryukov (previous) (diff)

#46 follow-up: @a223123131
4 years ago

In which WP version could thess changes be released? Is it still 5.6?

Last edited 4 years ago by a223123131 (previous) (diff)

#47 in reply to: ↑ 46 @SergeyBiryukov
4 years ago

  • Keywords commit dev-reviewed removed

Replying to a223123131:

In which WP version could thess changes be released? Is it still 5.6?

Yes, jQuery UI was updated to 1.12.1 in [49101]. This will be released in WordPress 5.6.

#49 in reply to: ↑ 43 @Clorith
4 years ago

Replying to azaozz:

Reopening this to track the eventual need to add jQuery.UNSAFE_restoreLegacyHtmlPrefilter();. See comment 2, comment 33 and comment 35.

[...]

In that terms wondering if circumventing the jQuery security fix in favor of a few plugins would be a good decision.

Ref the legacy HTML prefilter, some discussions in #core on Slack were also had, and the most likely course would be to not include something removed by the jQuery team for security for the WordPress release.

WordPress should instead judge the need for it it after the release (since it's a obscure thing to search for), and consider implementing it in the jQuery Migrate Helper if the core security team thinks it's an acceptable approach at that time.

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.


4 years ago

#52 @hellofromTonya
4 years ago

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

Per @helen

Doesn’t seem to have any further action items, can close as fixed

This ticket was mentioned in Slack in #core by clorith. View the logs.


4 years ago

Note: See TracTickets for help on using tickets.