WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 3 days ago

#51519 new defect (bug)

Remove use of jQuery ready()

Reported by: wpnomad Owned by:
Milestone: 5.8 Priority: normal
Severity: normal Version:
Component: General Keywords: has-patch has-unit-tests
Focuses: javascript Cc:

Description

ready() has been [deprecated](https://api.jquery.com/ready/), but we still use it in some places:

  • wp-content/themes/twentytwenty/assets/js/customize.js

(and maybe more).

Can this be reviewed? A report to change the same in WooCommerce core is here: https://github.com/woocommerce/woocommerce/issues/27945

Change History (8)

#1 @sabernhardt
7 months ago

  • Focuses javascript added
  • Keywords needs-patch added

Yes, several admin scripts and most bundled themes use .ready() at least once.

Related: #37110 and #50564

#2 @malthert
7 months ago

I reported this originally for WP (& also the WooCommerce issue is from me) - I think in fact we should not only change the .ready() event, but also include info about this change somewhere prominently, so all plugin authors are aware that they should change it too.

The process for this has already been started for all plugins sold on WooCommerce, so this will be finished before WP5.6 is released for those at least.

#3 @SergeyBiryukov
8 weeks ago

  • Milestone changed from Awaiting Review to 5.8

It looks like most of jQuery(document).ready() instances were removed in [50547] / #51812, but some are still left in these files:

  • js/_enqueues/vendor/mediaelement/wp-playlist.js
  • js/_enqueues/vendor/plupload/handlers.js
  • js/_enqueues/vendor/swfupload/handlers.js
  • js/_enqueues/vendor/thickbox/thickbox.js

These are all either WordPress files (and not external dependencies) or "adopted" libraries that have been patched in the past (in the case of Thickbox), so should be safe to patch.

#4 follow-up: @SergeyBiryukov
8 weeks ago

There are also a few instances in bundled themes:

  • wp-content/themes/twentyeleven/inc/theme-options.js
  • wp-content/themes/twentyeleven/js/showcase.js
  • wp-content/themes/twentyfifteen/js/functions.js
  • wp-content/themes/twentyfourteen/js/featured-content-admin.js
  • wp-content/themes/twentyseventeen/assets/js/global.js
  • wp-content/themes/twentysixteen/js/functions.js
  • wp-content/themes/twentytwenty/assets/js/customize.js
  • wp-content/themes/twentytwenty/assets/js/customize-preview.js

These may be harder to remove, as the themes must remain compatible with older WP versions they were released in, and possibly one or two versions before that.

#5 in reply to: ↑ 4 @peterwilsoncc
7 weeks ago

Replying to SergeyBiryukov:

These may be harder to remove, as the themes must remain compatible with older WP versions they were released in, and possibly one or two versions before that.

The jQuery( callback ) technique was added in jQuery 1.0 prior to WordPress's introduction of jQuery 1.1 in [4904] so I think removing the jQuery( document ).ready() statement on the older themes should be fine.

If I am missing something then the older themes may be able to use something along the lines of:

if ( document.readyState !== 'loading' ) {
        readyCallback();
} else {
        window.addEventListener( 'DOMContentLoaded', readyCallback, false );
}

#6 @Clorith
7 weeks ago

That's correct, it shouldn't be any concern replacing their use in the themes as well without the readyState workaround

This ticket was mentioned in PR #1190 on WordPress/wordpress-develop by pramodjodhani.


4 weeks ago

  • Keywords has-patch has-unit-tests added; needs-patch removed

Replace jQuery ready() with the alternate syntax $( function() {} ) because ready() has been deprecated.

Trac ticket:https://core.trac.wordpress.org/ticket/51519

#8 @prbot
3 days ago

Clorith commented on PR #1190:

Thank you for the PR, it does look like there's some accidentally included extra files related to PHP, namely the src/wp-includes/assets/script-loader-packages.php and tests/phpunit/data/link-manager.zip.

Would you be able to refresh your PR to not include these unrelated files?

Note: See TracTickets for help on using tickets.