Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#47342 closed defect (bug) (worksforme)

Get version of WordPress's jQuery

Reported by: mizzinc's profile mizzinc Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.2.1
Component: Script Loader Keywords: close
Focuses: Cc:

Description (last modified by SergeyBiryukov)

When accessing:

$ver = $GLOBALS['wp_scripts']->registered['jquery']->ver;

Instead of returning just a number it now adds '-wp'.

Which then of course is causing a 404 when attempting to register the script 'https://ajax.googleapis.com/ajax/libs/jquery/' . $ver . '/jquery.min.js'

Change History (12)

#1 @Hareesh Pillai
5 years ago

Hi @mizzinc, welcome to WordPress Trac! Thanks for the ticket.

Can you please help understand the use case? Why do we need to register the script from an external site again, if WordPress already comes bundled with jQuery?

#2 @mizzinc
5 years ago

Hi @hareesh-pillai

I suppose there could be a few reasons, in this scenario its just related to site speed.

#3 @azaozz
5 years ago

The version string change is from #47020. The version you load from ajax.googleapis.com is the old, unpached jQuery version.

Looking at $ver = $GLOBALS['wp_scripts']->registered['jquery']->ver;, frankly I don't think this is a "proper way" to use the script loader in WP. The version strings there are intended to refresh caching when the files have changed, nothing more. There are several "improper" or non-standard version strings in there. If you have to load from an external source, please use the full URL. However in this case that will load the unpatched version.

#4 @SergeyBiryukov
5 years ago

  • Component changed from General to Script Loader
  • Description modified (diff)

#5 @desrosj
5 years ago

  • Keywords close added

It's worth noting that the wp-includes/js/jquery/jquery.js file is an already minified version of the library.

This is also a unique situation because WordPress is stuck on jQuery 1.12.4 (which is unmaintained by the jQuery team) until all compatibility issues are addressed in #37110. There may be a slight sacrifice of performance, but the version bundled with WordPress is more secure.

I agree with @azaozz that this is definitely a non-standard way to use the version property of a script. If this usage was encouraged, there would be a wp_get_enqueued_script_version() or something similar to retrieve the version.

Marking as a close candidate, but leaving open for others to weigh in.

#6 follow-up: @garrett-eclipse
5 years ago

Hello,

Just wanted to indicate that this results in WSOD in any case the theme was built to use that version for the CDN call.

Had this happen on two sites we took over from another developer. Both were built with Roots Sage and implemented the Roots Soil module jQuery CDN. As they used this version string to call jQuery it returned as a 404 and all reliant jQuery scripts failed leaving the page blank.

I've flagged this to the developers of Roots Soil here - https://github.com/roots/soil/issues/226
And provided a PR here - https://github.com/roots/soil/pull/227
*I simply did a str_replace to strip the -wp suffix.

I'm not saying this is the right way to go about this but wanted to flag that it will probably affect alot of Roots users and any other plugin/theme that uses a CDN service. Is it easy to run a regex search in the Plugin/Theme repos to flag any that use the registered['jquery']->ver.
Note: This ticket used $GLOBALS['wp_scripts'] but in Roots Soil it uses wp_scripts().

All the best

#7 @azaozz
5 years ago

Was looking at what can be done in core to "remedy" the improper use of the version strings, but it's a tough call. We can't just remove the -wp there, need to change the version string to bust caches. But whatever we change it to it will still be an invalid version...

Ideally plugins that replace the local copy of jQuery will be updated to not replace it, and use the patched version distributed with WP.

Also this plugin https://wordpress.org/plugins/use-google-libraries/ (which is unfortunately outdated/seems unsupported) is "doing it right". See https://plugins.trac.wordpress.org/browser/use-google-libraries/tags/1.6.2.2/use-google-libraries.php#L354.

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

#8 in reply to: ↑ 6 ; follow-up: @azaozz
5 years ago

Replying to garrett-eclipse:

Thanks for alerting the devs of Roots Soil. The "proper" solution would be to stop loading the unpatched jQuery from a CDN, and load the patched version from WP.

As @desrosj pointed out above, the version in core is more secure than the one available elsewhere. #47020 was a security hardening that backported a fix from jQuery 3.4.0.

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

#9 in reply to: ↑ 8 @garrett-eclipse
5 years ago

Replying to azaozz

Thanks @azaozz I appreciate the information and direction. I've updated my PR to include the note and will leave it to the Soil developers to address.
My comment - https://github.com/roots/soil/pull/227#issuecomment-494959543

#10 follow-up: @mizzinc
5 years ago

This turned into an interesting thread. Hope I didnt cause more trouble than its worth.
Decided to use local copy as suggested by @azaozz

#11 in reply to: ↑ 10 @azaozz
5 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Replying to mizzinc:

No trouble at all, thanks for reporting this :)

Yeah, ideally we should upgrade to the latest jQuery and then keep it current, see #37110. Then things like this won't happen. In any case, plugins that redirect loading of local scripts to external CDNs should be more strict when checking the versions, see the above example (there may also be user privacy issues, but that's for the site admins that use the plugins to decide, provided they understand the implications).

On the other hand, literally all WP sites use jQuery, both in wp-admin and the front-end. And there are tons and tons of old/outdated/unsupported scripts that use it. Also, jQuery 4.0 is on the way, see https://github.com/jquery/jquery/milestone/7. Perhaps we should do that when it is out, maybe WP 5.4 or 5.5?. It will take tons of back-compat checking and fixing but it's worth it :)

#12 @ocean90
5 years ago

#47418 was marked as a duplicate.

Note: See TracTickets for help on using tickets.