WordPress.org

Make WordPress Core

#22975 closed enhancement (fixed)

Remove deprecated jQuery methods from core to be safe for jQuery 1.9

Reported by: ocean90 Owned by:
Milestone: 3.6 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords: has-patch
Focuses: Cc:

Description

Today jQuery 1.9 Beta was released.

jQuery 1.9 has removed many of the items we deprecated during the last few versions of jQuery.
To test, we recommend that you start with the jQuery Migrate plugin since it will warn you about any deprecated features the code may depend on. Just include these two script tags in your code, replacing your existing jQuery script include:

<script src="http://code.jquery.com/jquery-1.9.0b1.js"></script>

<script src="http://code.jquery.com/jquery-migrate-1.0.0b1.js"></script>

The attached patch adds both scripts to core so that you can test it. There are already some notices which we should try to reduce.

Attachments (21)

jquery-1.9-migrate.patch (374.0 KB) - added by ocean90 16 months ago.
22975-apply-fix.patch (813 bytes) - added by ocean90 16 months ago.
22975.custom-fields.patch (745 bytes) - added by SergeyBiryukov 16 months ago.
22975.custom-fields.diff (738 bytes) - added by cdog 16 months ago.
22975.custom-fields.2.diff (376 bytes) - added by cdog 16 months ago.
22975.custom-fields.3.diff (383 bytes) - added by SergeyBiryukov 16 months ago.
With proper formatting
22975.rc1.patch (42.5 KB) - added by ocean90 16 months ago.
22975.patch (7.5 KB) - added by ocean90 15 months ago.
jquery.1.9.1.diff (41.5 KB) - added by wonderboymusic 15 months ago.
22975.replace-live.patch (2.1 KB) - added by ocean90 14 months ago.
22975.thickbox.patch (992 bytes) - added by ocean90 14 months ago.
22975.browser.patch (1.9 KB) - added by ocean90 14 months ago.
22975.consolidated.patch (5.3 KB) - added by ocean90 14 months ago.
jquery.migrate.1.1.1.patch (5.0 KB) - added by ocean90 14 months ago.
22975.taxonomy-checklist.patch (723 bytes) - added by SergeyBiryukov 14 months ago.
22975.taxonomy-checklist.2.patch (743 bytes) - added by SergeyBiryukov 14 months ago.
22975.taxonomy-checklist.3.patch (707 bytes) - added by SergeyBiryukov 14 months ago.
22975.taxonomy-checklist.3b.patch (1.0 KB) - added by ocean90 12 months ago.
22975.migrate.1.2.1.patch (3.7 KB) - added by ocean90 11 months ago.
22975.diff (691 bytes) - added by aaroncampbell 11 months ago.
22975.suggest.patch (1.2 KB) - added by ocean90 11 months ago.

Download all attachments as: .zip

Change History (91)

comment:1 scribu16 months ago

  • Milestone changed from Future Release to 3.6

comment:2 nacin16 months ago

  • Keywords has-patch commit added

comment:3 nacin16 months ago

In 23180:

Update to jQuery 1.9 Beta 1 in trunk (3.6-alpha).

Temporarily includes jQuery in original, unminified form, for ease of testing and bug reporting.

jQuery 1.9 introduces a "migrate" script for deprecated and removed behavior. Warnings are issued for methods we should not be using, and should be fixed in core. These warnings are sent to the JavaScript console and are collected in jQuery.migrateWarnings.

See http://blog.jquery.com/2012/12/17/jquery-1-9-beta-1-released/ for more.

props ocean90, see #22975.

comment:4 nacin16 months ago

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

Setting to needs-patch for removal of warnings.

Note that I needed to make a change to jQuery Migrate before landing it. PR here: https://github.com/jquery/jquery-migrate/pull/5.

comment:5 nacin16 months ago

Seems like the dependency tree in [23180] would break with concatenation. Migrate needs to load after jQuery, not before.

We should just place the migration code directly into jQuery core for now. If we need to keep it for plugins (which is likely the case), we can figure out a better way.

comment:6 follow-ups: nacin16 months ago

Anyone else imagining this as a good idea?

$scripts->add( 'jquery', false, array( 'jquery-core', 'jquery-noconflict', 'jquery-migrate' ) );
$scripts->add( 'jquery-core', ... );
$scripts->add( 'jquery-noconflict', ... );
$scripts->add( 'jquery-migrate', ... );

comment:7 follow-up: trepmal16 months ago

Getting a Javascript error using Enter New on custom fields.
http://content.screencast.com/users/trepmal/folders/Jing/media/2e3dc6ea-4680-442f-8e71-b414da1f184d/00000690.png

In screenshot, first 2 migrate notices are on page load, 3rd notice and error when clicking Enter New

comment:8 in reply to: ↑ 7 ; follow-up: ocean9016 months ago

Replying to trepmal:

Getting a Javascript error using Enter New on custom fields.

That's already fixed upstream, see https://github.com/jquery/jquery-migrate/issues/4

ocean9016 months ago

comment:9 cklosows16 months ago

  • Cc cklosowski@… added

Also Seeing this when trying to use .toggle() for a show/hide within a plugin page. Was able to duplicate @trepmal's case as well.

TypeError: Cannot call method 'apply' of undefined http://localhost/wordpress/wp-includes/js/jquery/jquery-migrate.js?ver=1.0.0b1:349

comment:10 in reply to: ↑ 8 cklosows16 months ago

Replying to ocean90:

Replying to trepmal:

Getting a Javascript error using Enter New on custom fields.

That's already fixed upstream, see https://github.com/jquery/jquery-migrate/issues/4

Confirmed that fixed it. Thanks @ocean90

comment:11 follow-up: trepmal16 months ago

22975-apply-fix.patch fixes the Enter New problem.

"Add Custom Field" (and Update for existing custom fields) button results in javascript error.
http://content.screencast.com/users/trepmal/folders/Jing/media/7d2aae71-6083-42ae-99c9-dc18c5534049/00000693.png
However, for new custom fields, the field is created and is then duplicated after updating/publishing/saving draft.

comment:12 nacin16 months ago

In 23182:

Update jQuery Migrate plugin to latest, fixing .toggle(boolean) usage. see #22975.

comment:13 nacin16 months ago

Here's the latest jQuery Migrate (live) — http://code.jquery.com/jquery-migrate-git.js. That's what [23182] applies. I didn't even see the chatter on this ticket committing, as one of the lead jQuery developers alerted me. He's very happy to see that we got some testers. :-)

comment:14 in reply to: ↑ 6 TobiasBg16 months ago

Replying to nacin:

Seems like the dependency tree in [23180] would break with concatenation. Migrate needs to load after jQuery, not before.

I can't reproduce problems with this. http://localhost/wp-admin/load-scripts.php?c=0&load[]=jquery-migrate,jquery,utils&ver=3.6-alpha works fine, jQuery is first (with .noConflict(); at the end), then jquery-migrate.js.

Replying to nacin:

$scripts->add( 'jquery-noconflict', ... );

This should also get a dependency on jquery-core. This would allow plugins to enqueue jQuery without jquery-migrate, if it is sure that it doesn't need the backwards compatibility part.

comment:15 follow-up: nacin16 months ago

In 23183:

Have the 'jquery' script handle be a parent of both jQuery core and jQuery Migrate. see #22975.

comment:16 in reply to: ↑ 15 markmcwilliams16 months ago

Replying to nacin:

In 23183:

Have the 'jquery' script handle be a parent of both jQuery core and jQuery Migrate. see #22975.

Should the version not be updated as per [23182] or will it not matter at this early stage?

Last edited 16 months ago by markmcwilliams (previous) (diff)

comment:17 in reply to: ↑ 11 ; follow-ups: SergeyBiryukov16 months ago

Replying to trepmal:

"Add Custom Field" (and Update for existing custom fields) button results in javascript error.

Confirmed. The error happens at the very beginning of wpList.add():
http://core.trac.wordpress.org/browser/tags/3.5/wp-includes/js/wp-lists.js#L310

Seems that jQuery 1.9 has a problem if the string starts with \n. Also reproduced with this fiddle: http://jsfiddle.net/VnCrU/.

22975.custom-fields.patch fixes the error.

cdog16 months ago

comment:18 in reply to: ↑ 17 cdog16 months ago

Replying to SergeyBiryukov:

22975.custom-fields.patch fixes the error.

It fixes the error but throws the following warning:

JQMIGRATE: $(html) HTML strings must start with '<' character

Removing both leading characters \n\t as shown in attachment:22975.custom-fields.diff seems to get rid of the warning too.

Edit: Is there any reason for using those special characters at the beginig of each concatenated string? Removing them would produce the same result making the code more readable.

Version 1, edited 16 months ago by cdog (previous) (next) (diff)

comment:19 cdog16 months ago

  • Cc catalin.dogaru@… added

comment:20 in reply to: ↑ 17 ; follow-up: SergeyBiryukov16 months ago

Replying to SergeyBiryukov:

Seems that jQuery 1.9 has a problem if the string starts with \n. Also reproduced with this fiddle: http://jsfiddle.net/VnCrU/.

Appears to be a result of the fix in http://bugs.jquery.com/ticket/11290.

comment:21 cdog16 months ago

Please take a look at: attachment:22975.custom-fields.2.diff. I think a better approach would be to trim leading spaces as noted in http://bugs.jquery.com/ticket/11290#comment:4. Should also solve similar issues if they exist.

SergeyBiryukov16 months ago

With proper formatting

comment:22 nacin16 months ago

A developer using wp_localize_script( 'jquery', ... ) (to inexplicably pass values to, well, I don't know, they're doing it wrong) will find that their code breaks now.

ocean9016 months ago

comment:23 ocean9016 months ago

Attached a patch for jQuery 1.9 RC1. Includes also the updated version of the Migrate script. I used the git Version since it includes already a new fix.

comment:24 ocean9015 months ago

#23204 was marked as a duplicate.

comment:26 nacin15 months ago

In 23301:

jQuery 1.9 final. jQuery Migrate 1.0. Uncompressed for now, while we iron out kinks.

props jorbin. see #22975.

comment:27 Lwangaman15 months ago

  • Cc donjohn.fmmi@… added

jQuery 1.9 has specifically deprecated the methods ".live()" and ".browser()" in favour of ".on()" and ".support()" respectively.
This breaks "wp-includes/js/admin-bar.js" (or "wp-includes/js/admin-bar.min.js") which makes use of the "$.browser()" method to target Mozilla. Hovering over admin bar items when using jQuery 1.9 results in these errors in the console:
Uncaught TypeError: Cannot read property 'mozilla' of undefined ---- admin-bar.min.js:1
It will be necessary to target the specific functionality of mozilla that is different from other browsers in order to use "$.support()".

comment:28 ocean9015 months ago

  • Keywords has-patch added; needs-patch removed

ocean9015 months ago

comment:29 wonderboymusic15 months ago

#23388 was marked as a duplicate.

comment:30 SergeyBiryukov15 months ago

jquery.1.9.1.diff fixes the error when adding or updating custom fields (comment:17).

comment:31 nacin14 months ago

In 23421:

jQuery 1.9.1 and jQuery Migrate 1.1.0.

Remains uncompressed for now, until we work out all 1.9.x issues.
Fixes custom fields.

props ocean90, wonderboymusic. see #22975.

comment:32 ocean9014 months ago

  • Keywords needs-patch added; has-patch removed

I will start to replace the deprecated methods now.

comment:33 ocean9014 months ago

I have started with the .live() event.

$ ack ".live\(" -l --ignore-dir=wp-content
wp-admin/js/categories.js
wp-admin/js/inline-edit-post.js
wp-admin/js/inline-edit-tax.js
wp-admin/js/post.js
wp-admin/js/tags.js
wp-includes/js/jquery/jquery-migrate.js
wp-includes/js/swfupload/handlers.js
wp-includes/js/thickbox/thickbox.js

22975.replace-live.patch adresses

  • wp-admin/js/inline-edit-post.js
  • wp-admin/js/inline-edit-tax.js
  • wp-admin/js/post.js
  • wp-admin/js/tags.js

I ignored external libs like Thickbox. wp-admin/js/categories.js isn't used anymore, see #23486.

wp-admin/js/post.js: I have no idea what these lines should do, it seems that it does nothing. Sergey gave me the hint.

Last edited 14 months ago by ocean90 (previous) (diff)

comment:34 ocean9014 months ago

Next: $.browser

ack "(\\$|jQuery)\.browser" --ignore-dir=wp-content -l
wp-admin/js/color-picker.js
wp-includes/js/admin-bar.js
wp-includes/js/autosave.js
wp-includes/js/imgareaselect/jquery.imgareaselect.js
wp-includes/js/jquery/jquery-migrate.js
wp-includes/js/jquery/jquery.hotkeys.js
wp-includes/js/jquery/suggest.js
wp-includes/js/thickbox/thickbox.js

wp-admin/js/color-picker.js is handled in #23484.

wp-includes/js/admin-bar.js uses $.browser to detect WebKit to fix the focus bug. See [22249].

wp-includes/js/autosave.js uses $.browser to detect Safari. See #18341.

comment:35 nacin14 months ago

In 23444:

Stop using jQuery.live(). props ocean90. see #22975.

comment:36 follow-up: nacin14 months ago

Let's also patch Thickbox. It is dead upstream.

ocean9014 months ago

ocean9014 months ago

comment:37 in reply to: ↑ 36 ocean9014 months ago

  • Keywords has-patch added; needs-patch removed

Replying to nacin:

Let's also patch Thickbox. It is dead upstream.

.live() is done in 22975.thickbox.patch.

In 22975.browser.patch I have replaced the jQuery.browser calls with navigator.userAgent. Except in Thickbox - It would work without the two lines, but I leave it in, since it seems not to hurt.

comment:38 ocean9014 months ago

There is another jQuery.browser call in admin-bar.js. It's in the hoverintent lib. Added in [18488]. Looking at the original version there seems not to be such a call. Will talk to azaozz.

comment:39 ocean9014 months ago

22975.consolidated.patch combines 22975.thickbox.patch and 22975.browser.patch​. And updates hoverIntent to r6, see [19605].

comment:41 follow-up: SergeyBiryukov14 months ago

In [23444], the selector in post.js appears to be redundant: 22975.taxonomy-checklist.patch.

comment:42 in reply to: ↑ 41 ; follow-ups: ocean9014 months ago

Replying to SergeyBiryukov:

In [23444], the selector in post.js appears to be redundant: 22975.taxonomy-checklist.patch.

Yeah, just saw that I have added the wrong patch which has included the console.log() call.

One small disadvantage is, that the event is now fired for each checkbox, not only the popular ones. We could add some parent element checks, but IMO the current way is not such harmful.

comment:43 follow-up: azaozz14 months ago

...but IMO the current way is not such harmful.

As far as I remember that bit of JS was causing slow running problems in (older) IE when there are a lot of categories. Probably better to keep it on the "popular" checkboxes only.

comment:44 in reply to: ↑ 43 ; follow-up: ocean9014 months ago

Replying to azaozz:

As far as I remember that bit of JS was causing slow running problems in (older) IE when there are a lot of categories. Probably better to keep it on the "popular" checkboxes only.

The performance one was r21737, see comment:ticket:21106:16. It had used previously :checkbox, which was replaced with [type="checkbox"]. So the change should be okay - Sergey, can you confirm?

comment:45 in reply to: ↑ 44 azaozz14 months ago

Replying to ocean90:

You're right. It also doesn't need the two selectors as we are looking at all checkboxes in the two <ul>:

.on( 'click', 'input[type="checkbox"]', function() {...

would be better (that JS and postbox really needs redoing).

comment:46 in reply to: ↑ 42 SergeyBiryukov14 months ago

Replying to ocean90:

One small disadvantage is, that the event is now fired for each checkbox, not only the popular ones.

Would 22975.taxonomy-checklist.2.patch make more sense then?

The performance one was r21737, see comment:ticket:21106:16. It had used previously :checkbox, which was replaced with [type="checkbox"]. So the change should be okay - Sergey, can you confirm?

Correct.

comment:47 ocean9014 months ago

  • Keywords commit added

22975.taxonomy-checklist.patch won't work, because li.popular-category can be added after the DOM is ready.

Commit candidates:

comment:48 nacin14 months ago

In 23516:

jQuery Migrate 1.1.1. props ocean90. see #22975.

comment:49 nacin14 months ago

In 23517:

Imporove a selector targeting taxonomy meta boxes. props SergeyBiryukoc. see #22975.

comment:50 nacin14 months ago

In 23518:

Stop using deprecated jQuery API (.browser and .live). Do manual UA sniffing where still necessary. Improve selector performance by using delegated events. props ocean90. see #22975.

comment:51 nacin14 months ago

  • Keywords has-patch commit removed

comment:52 nacin14 months ago

In 23536:

Imporove a selector targeting taxonomy meta boxes. props SergeyBiryukov. see #22975.

Uses correct patch, reverting [23517].

comment:53 follow-up: SergeyBiryukov14 months ago

Not sure why [23517] got reverted. Per comment:47, 22975.taxonomy-checklist.2.patch is the correct patch.

comment:54 in reply to: ↑ 53 nacin14 months ago

Replying to SergeyBiryukov:

Not sure why [23517] got reverted. Per comment:47, 22975.taxonomy-checklist.2.patch is the correct patch.

ocean90 had sent me a note — he mixed up the two patches in that comment. .2 wouldn't work. The first one does. Hence the swap.

comment:55 in reply to: ↑ 42 ; follow-up: SergeyBiryukov14 months ago

Replying to ocean90:

One small disadvantage is, that the event is now fired for each checkbox, not only the popular ones.

Seems like 22975.taxonomy-checklist.3.patch would be the correct optimization here. We only want the checkbox synchronization between "All Categories" and "Most Used" tabs to fire when clicking on a popular category itself, not on its child categories (like before [23444]) or any category (like now).

comment:56 in reply to: ↑ 20 SergeyBiryukov14 months ago

Replying to SergeyBiryukov:

Seems that jQuery 1.9 has a problem if the string starts with \n. Also reproduced with this fiddle: http://jsfiddle.net/VnCrU/.

Appears to be a result of the fix in http://bugs.jquery.com/ticket/11290.

Custom fields (comment:11) and menus (#23681) are affected.

In current trunk it's a console message rather than a JS error thanks to jQuery Migrate 1.1.0 ([23421]).

comment:57 ocean9013 months ago

  • Keywords needs-patch added

comment:58 aniketpant12 months ago

  • Cc me@… added

comment:59 in reply to: ↑ 55 ocean9012 months ago

Replying to SergeyBiryukov:

Seems like 22975.taxonomy-checklist.3.patch would be the correct optimization here. We only want the checkbox synchronization between "All Categories" and "Most Used" tabs to fire when clicking on a popular category itself, not on its child categories (like before [23444]) or any category (like now).

Committed in [24116].

comment:60 in reply to: ↑ 6 ; follow-up: anastis12 months ago

Replying to nacin:

Anyone else imagining this as a good idea?

$scripts->add( 'jquery', false, array( 'jquery-core', 'jquery-noconflict', 'jquery-migrate' ) );
$scripts->add( 'jquery-core', ... );
$scripts->add( 'jquery-noconflict', ... );
$scripts->add( 'jquery-migrate', ... );

This seems to have make it into beta 2 in the form:

$scripts->add( 'jquery', false, array( 'jquery-core', 'jquery-migrate' ) );
$scripts->add( 'jquery-core', '/wp-includes/js/jquery/jquery.js', array(), '1.9.1' );
$scripts->add( 'jquery-migrate', '/wp-includes/js/jquery/jquery-migrate.js', array(), '1.1.1' );

Is the jquery-migrate plugin going to be removed in 3.6 or in some future version?

Also, I've seen some plugins (notably WooCommerce) checking for the jQuery version like this:

if ( isset( $wp_scripts->registered['jquery']->ver ) && $wp_scripts->registered['jquery']->ver < '1.7' )

Perhaps it would be a good idea to add a version number to 'jquery' so that this kind of code remains functional?

comment:61 ocean9011 months ago

In 24252:

jQuery Migrate 1.2.1. see #22975.

aaroncampbell11 months ago

comment:62 in reply to: ↑ 60 aaroncampbell11 months ago

22975.diff adds the jquery-core version to jquery. This will fix the issue brought up in comment 60

Last edited 11 months ago by aaroncampbell (previous) (diff)

comment:63 ryan11 months ago

Looks good.

comment:64 aaroncampbell11 months ago

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

comment:65 markjaquith11 months ago

In 24314:

Add the jQuery version to the 'jquery' script alias

  • Allows people to check it and get the right result

see #22975. props aaroncampbell.

comment:66 ocean9011 months ago

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

ocean9011 months ago

comment:67 ocean9011 months ago

  • Keywords has-patch added; needs-patch removed

22975.suggest.patch removes the IE browser check since the method .bgiframe() on $result has no function because the method doesn't exist.

And it removes the check for Mozilla. keydown works for me in all browsers. Tested also via Browserstack down to Firefox 3.6.

comment:68 markjaquith11 months ago

Looks good to me. Go for it.

comment:69 ocean9011 months ago

In 24371:

Remove deprecated jQuery API .browser in suggest.js. No need for a replacement since it works without too. see #22975.

comment:70 ocean9011 months ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.