Opened 5 months ago
Last modified 2 days ago
#22975 new enhancement
Remove deprecated jQuery methods from core to be safe for jQuery 1.9
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 3.6 |
| Component: | External Libraries | Version: | |
| Severity: | normal | Keywords: | needs-patch |
| Cc: | cklosowski@…, catalin.dogaru@…, donjohn.fmmi@…, me@… |
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 (20)
Change History (86)
- 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.
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.
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', ... );
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
- 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
cklosows — 5 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:
↓ 17
trepmal — 5 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.

However, for new custom fields, the field is created and is then duplicated after updating/publishing/saving draft.
comment:12
nacin — 5 months ago
In 23182:
comment:13
nacin — 5 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
TobiasBg — 5 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:
↓ 16
nacin — 5 months ago
In 23183:
comment:16
in reply to:
↑ 15
markmcwilliams — 5 months ago
SergeyBiryukov — 5 months ago
comment:17
in reply to:
↑ 11
;
follow-ups:
↓ 18
↓ 20
SergeyBiryukov — 5 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.
comment:18
in reply to:
↑ 17
cdog — 5 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 results making the code more readable.
comment:19
cdog — 5 months ago
- Cc catalin.dogaru@… added
comment:20
in reply to:
↑ 17
;
follow-up:
↓ 56
SergeyBiryukov — 5 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
cdog — 5 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.
comment:22
nacin — 5 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.
comment:23
ocean90 — 4 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
ocean90 — 4 months ago
#23204 was marked as a duplicate.
comment:25
ocean90 — 4 months ago
Patch for 1.9 final: ticket:23204:jquery.diff
comment:26
nacin — 4 months ago
In 23301:
comment:27
Lwangaman — 4 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
ocean90 — 4 months ago
- Keywords has-patch added; needs-patch removed
wonderboymusic — 4 months ago
comment:29
wonderboymusic — 4 months ago
#23388 was marked as a duplicate.
comment:30
SergeyBiryukov — 4 months ago
jquery.1.9.1.diff fixes the error when adding or updating custom fields (comment:17).
comment:31
nacin — 3 months ago
In 23421:
comment:32
ocean90 — 3 months ago
- Keywords needs-patch added; has-patch removed
I will start to replace the deprecated methods now.
comment:33
ocean90 — 3 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.
comment:34
ocean90 — 3 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
nacin — 3 months ago
In 23444:
comment:36
follow-up:
↓ 37
nacin — 3 months ago
Let's also patch Thickbox. It is dead upstream.
comment:37
in reply to:
↑ 36
ocean90 — 3 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
ocean90 — 3 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
ocean90 — 3 months ago
22975.consolidated.patch combines 22975.thickbox.patch and 22975.browser.patch. And updates hoverIntent to r6, see [19605].
comment:40
ocean90 — 3 months ago
SergeyBiryukov — 3 months ago
comment:41
follow-up:
↓ 42
SergeyBiryukov — 3 months ago
In [23444], the selector in post.js appears to be redundant: 22975.taxonomy-checklist.patch.
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:
↓ 44
azaozz — 3 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:
↓ 45
ocean90 — 3 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
azaozz — 3 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).
SergeyBiryukov — 3 months ago
comment:46
in reply to:
↑ 42
SergeyBiryukov — 3 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
ocean90 — 3 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
nacin — 3 months ago
In 23516:
comment:49
nacin — 3 months ago
In 23517:
comment:50
nacin — 3 months ago
In 23518:
comment:51
nacin — 3 months ago
- Keywords has-patch commit removed
comment:52
nacin — 3 months ago
In 23536:
comment:53
follow-up:
↓ 54
SergeyBiryukov — 3 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
nacin — 3 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.
SergeyBiryukov — 3 months ago
comment:55
in reply to:
↑ 42
;
follow-up:
↓ 59
SergeyBiryukov — 3 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
SergeyBiryukov — 3 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
ocean90 — 7 weeks ago
- Keywords needs-patch added
comment:58
aniketpant — 6 weeks ago
- Cc me@… added
comment:59
in reply to:
↑ 55
ocean90 — 4 weeks 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:
↓ 62
anastis — 3 weeks 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
ocean90 — 10 days ago
In 24252:
aaroncampbell — 2 days ago
comment:62
in reply to:
↑ 60
aaroncampbell — 2 days ago
22975.diff adds the jquery-core version to jquery. This will fix the issue brought up in comment 60
comment:63
ryan — 2 days ago
Looks good.
comment:64
aaroncampbell — 2 days ago
- Keywords commit has-patch added; needs-patch removed
comment:65
markjaquith — 2 days ago
In 24314:


In 23180: