Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#50919 closed defect (bug) (fixed)

wp_localize_script( 'jquery') has stopped working

Reported by: rajeshsingh520's profile rajeshsingh520 Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 5.5.1 Priority: normal
Severity: normal Version: 5.5
Component: Script Loader Keywords: has-patch commit
Focuses: Cc:

Description

Hi,

In my plugin i user wp_localize_script( 'jquery', 'pi_global_day_time', $return ); to locaive certail variable but after upgrading to 5.5 the varible are not getting localized

Attachments (1)

50919.patch (717 bytes) - added by mukesh27 4 years ago.
Initial patch.

Download all attachments as: .zip

Change History (20)

#1 @johnbillion
4 years ago

  • Component changed from General to Script Loader
  • Keywords reporter-feedback added
  • Version set to 5.5

Welcome and thanks for the report @rajeshsingh520 .

What's the exact problem you're seeing? Are the variables still being passed but just not localized? Or are they missing entirely? Some example code would be a great help.

#2 @rajeshsingh520
4 years ago

Yes the issue was still there, it was infect affecting my other plugins as well

you can reproduct the issue on your end

all the wp_localize_script( 'jquery', 'pi_global_day_time', $return ); localizing with 'jquery' are not getting added in the front end

I fixed it in my plugin by changing it to localize with some other handler script

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

#3 @TimothyBlynJacobs
4 years ago

#50948 was marked as a duplicate.

#4 follow-up: @TimothyBlynJacobs
4 years ago

It looks like this happens because wp_localize_script rewrites jquery to jquery-core. In WordPress 5.4, jquery didn't have a script of its own, and was instead a wrapper for jquery-core and jquery-migrate. In #37110, this was changed so that jquery has a script of its own and doesn't reference jquery-core at all.

An immediate fix for this would be dropping that rewrite in wp_localize_script, but it seems like there could be other scenarios where plugins/themes are relying on jquery-core being actually enqueued. I think it'd make the most sense to change jquery back to a wrapper for jquery-core. Particularly since we'll be reintroducing jquery-migrate in 5.6 when we update to jQuery 3.

#5 @desrosj
4 years ago

  • Milestone changed from Awaiting Review to 5.5.1

Moving to 5.5.1 for discussion.

#6 in reply to: ↑ 4 @TobiasBg
4 years ago

Replying to TimothyBlynJacobs:

but it seems like there could be other scenarios where plugins/themes are relying on jquery-core being actually enqueued. I think it'd make the most sense to change jquery back to a wrapper for jquery-core.

This might indeed also helps with a related issue that I noticed today: If a plugin previously had deliberately enqueued jquery-core (because its JS files didn't need jquery-migrate), the jquery.js file might now loaded twice (e.g. if another plugin or the theme is also enqueuing it). This leads to issues if other jQuery plugins (like jquery-ui and similar) are loaded, as the second copy of jquery.js then overrides the extended jQuery JS object again.
Right now, this can be fixed by not enqueuing jquery-core directly, but it takes away the possibility to deliberately not load jquery-migrate.

So, indeed, if jquery could be made a "wrapper" or "alias" for jquery-core (until the new jquery-migrate is integrated), the just-described problem and the original ticket problem should be solved.

#7 @TobiasBg
4 years ago

That said, maybe replacing

$scripts->add( 'jquery', '/wp-includes/js/jquery/jquery.js', array(), '1.12.4-wp' );

with

$scripts->add( 'jquery', false, array( 'jquery-core' ), '1.12.4-wp' );

might already be sufficient.

#8 @TimothyBlynJacobs
4 years ago

Yeah I think that change makes the most sense.

#9 @johnbillion
4 years ago

  • Keywords needs-patch added; reporter-feedback removed

#10 @mukesh27
4 years ago

So we have to revert this 48323 to fix the this issue.

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

@mukesh27
4 years ago

Initial patch.

#11 @mukesh27
4 years ago

  • Keywords has-patch added; needs-patch removed

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


4 years ago

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


4 years ago

#14 @SergeyBiryukov
4 years ago

  • Keywords commit added

Just noting for reference that comment:10 is a bit misleading and the patch doesn't actually revert all of [48323], just restores the jquery handle as an alias for jquery-core.

50919.patch looks good to me, since we'll need to add a newer version of jquery-migrate in 5.6, per #50564, as noted in comment:4.

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

#15 @SergeyBiryukov
4 years ago

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

In 48890:

Script Loader: Change the jquery handle back to an alias for jquery-core.

This ensures that wp_localize_script( 'jquery', ... ) continues to work as expected, since WP_Scripts::localize() rewrites the jquery handle to jquery-core internally.

Follow-up to [48323].

Props mukesh27, rajeshsingh520, johnbillion, TimothyBlynJacobs, TobiasBg.
Fixes #50919.

#16 @SergeyBiryukov
4 years ago

In 48891:

Script Loader: Change the jquery handle back to an alias for jquery-core.

This ensures that wp_localize_script( 'jquery', ... ) continues to work as expected, since WP_Scripts::localize() rewrites the jquery handle to jquery-core internally.

Follow-up to [48323].

Props mukesh27, rajeshsingh520, johnbillion, TimothyBlynJacobs, TobiasBg.
Merges [48890] to the 5.5 branch.
Fixes #50919.

#17 @davidbaumwald
4 years ago

Per https://travis-ci.com/github/WordPress/wordpress-develop/jobs/378540395, updating the tests to reference jquery-core.

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

#18 @SergeyBiryukov
4 years ago

In 48892:

Tests: Update wp_add_inline_script() unit tests to account for the jquery handle being an alias for jquery-core again.

Follow-up to [48323], [48324], [48890].

Props davidbaumwald, audrasjb.
See #50919.

#19 @SergeyBiryukov
4 years ago

In 48893:

Tests: Update wp_add_inline_script() unit tests to account for the jquery handle being an alias for jquery-core again.

Follow-up to [48323], [48324], [48890].

Props davidbaumwald, audrasjb.
Merges [48892] to the 5.5 branch.
See #50919.

Note: See TracTickets for help on using tickets.