Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#22896 closed enhancement (fixed)

Prevent plugins from overriding jQuery in the admin

Reported by: nacin's profile nacin Owned by: nacin's profile nacin
Milestone: 3.6 Priority: normal
Severity: normal Version:
Component: External Libraries Keywords: dev-feedback has-patch
Focuses: Cc:

Description

This is getting ridiculous. It's time to force our own version of jQuery and avoid it being unregistered/re-registered by conventional means.

That's not to say it wouldn't be "impossible" (that's just not realistic), but it can prevent any plugin or theme doing its overriding blindly on init from breaking the admin.

Attachments (8)

brokentheme.diff (1.2 KB) - added by adamsilverstein 11 years ago.
functions.wp-scripts.diff (699 bytes) - added by adamsilverstein 11 years ago.
22896.patch (561 bytes) - added by SergeyBiryukov 11 years ago.
With proper formatting
22896-with-backbone-and-underscore.diff (677 bytes) - added by adamsilverstein 11 years ago.
externals.php (1.3 KB) - added by adamsilverstein 11 years ago.
unit test for #22896
22896.unittest.diff (1.5 KB) - added by adamsilverstein 11 years ago.
unit test for #22896, take 2
22896-unittest-2.diff (1.4 KB) - added by adamsilverstein 11 years ago.
updated unit test for #22896
22896-unittest-3.diff (1.5 KB) - added by adamsilverstein 11 years ago.
removed enqueue line, set screen back to starting screen

Download all attachments as: .zip

Change History (70)

#1 @sabreuse
11 years ago

Yes, please.

#2 @nacin
11 years ago

#19568 was marked as a duplicate.

#3 @knutsp
11 years ago

  • Cc knut@… added

#4 follow-up: @ipstenu
11 years ago

We have to keep in mind legitimate use cases (Use Google Libraries, for example). Make it harder yes please. Also for themes. Non .org hosted themes can be prevented from doing_it_wrong() too

#5 in reply to: ↑ 4 ; follow-up: @helenyhou
11 years ago

Replying to ipstenu:

We have to keep in mind legitimate use cases

This does say "in the admin", in which case there is no legitimate reason to load jQuery from elsewhere :)

#6 @toscho
11 years ago

  • Cc info@… added

#7 follow-up: @F J Kaiser
11 years ago

  • Cc 24-7@… added

+1 for getting rid of this annoyance.

But maybe there's another possibility: Make _doing_it_wrong() non disable-able with WP_DEBUG, so everybody (incl. customers of so-called "premium"-plugins/-themes) start being aware of the crap they just have installed.

Anyway, I'm in for that (or any related) change, no matter at which cost. At those who want to change the "type" away from "defect/bug": Dare to.

#8 in reply to: ↑ 5 @Ipstenu
11 years ago

Replying to helenyhou:

Replying to ipstenu:

We have to keep in mind legitimate use cases

This does say "in the admin", in which case there is no legitimate reason to load jQuery from elsewhere :)

"It'll speed up my backend!" (Honestly I don't know why you'd want to load included files from another site anyway, but one man's silly is another's need.)

#9 @chipbennett
11 years ago

  • Cc chip@… added

#10 in reply to: ↑ 7 ; follow-ups: @kevinjohngallagher
11 years ago

  • Cc kevinjohngallagher added
  • Type changed from defect (bug) to enhancement

My concern with things like this is that it's a fairly sweeping functionality change, and a lockdown, that has the potential to have an incredibly large impact; especially for those not using WordPress in a "traditional" way. I completely understand that we're the smallest fish in the sea here, but we haven't developed a WP theme in at least 18 months; while almost all of our WP work in the last 2.5 years has come from backend customisation and integration. This would (potentially) put a major dent in that.

Replying to helenyhou:

Replying to ipstenu:

We have to keep in mind legitimate use cases

This does say "in the admin", in which case there is no legitimate reason to load jQuery from elsewhere :)

I think we can all admit that with 17 million WP installs out there, the idea that not one of them has a "legitimate reason" for anything seems unrealistic.

Here's a few off the top of my head:

  • IE6 support
  • SLA compliance
  • Digital Security compliance
  • Regression testing
  • Customised SaaS offerings
  • Reliance on external code
  • Not moving to jQuery 2.0 next release - thats a huge fear right now (no IE6/7/8 support)

.

Replying to F J Kaiser:

But maybe there's another possibility: Make _doing_it_wrong() non disable-able with WP_DEBUG, so everybody (incl. customers of so-called "premium"-plugins/-themes) start being aware of the crap they just have installed.

Customers don't care about "the crap they just have installed" from an elitist developer perspective. They just want something that works. We have to start thinking of the actual user, and not just get on our high horse because someone is doing something that is wrong in our opinion.
.

Anyway, I'm in for that (or any related) change, no matter at which cost. At those who want to change the "type" away from "defect/bug": Dare to.

Done.
This is not a bug.
It's working as intended.
It's the very definition of an Enhancement.
I can see some reasons why people want it, thats grand.
But it's not a bug, as the code works just fine as is :)

#11 @nacin
11 years ago

Not moving to jQuery 2.0 next release - thats a huge fear right now (no IE6/7/8 support)

I can quell that fear right now. We'll be moving to 1.9, and the jQuery team will be supporting 1.9 in parallel with 2.0 as the same featureset and simply a difference in browser support. I expect dropping IE7 in the admin may happen in 2013; dropping IE8 will surely happen eventually, probably after Windows XP is EOL.

If you load jQuery from elsewhere in the admin, you risk breaking the rest of WordPress. That's without a doubt the number 1 thing on my mind right now.

Here's the problem I'm trying to solve. A theme hooks into init and de-registers jQuery, then registers another one elsewhere. This overrode the admin jQuery accidentally. That's just bad. I'm not saying we should prevent jQuery replacement all together. (Let's face it, it'll always be possible to do.) Just that we should prevent this accidental and stupid situation from occurring, because it breaks tons and tons of WordPress sites.

#12 in reply to: ↑ 10 @F J Kaiser
11 years ago

  • Summary changed from Prevent plugins from overriding jQuery in the admin to has_post_format() should accept an array

Replying to kevinjohngallagher:

I think we can all admit that with 17 million WP installs out there, the idea that not one of them has a "legitimate reason" for anything seems unrealistic.

Please keep in mind that when there're 17 million installations, then the "custom jQuery location"-scenario has a ~0.001% use case where it's maybe needed. With such a user base you first have to support the majority, then care about edge cases. Maybe you should jump into WP.SE or the wp.org support forums and try to help novice users for some time to get a feeling for the major reasons when they got the famous "white screen".

#13 @F J Kaiser
11 years ago

  • Summary changed from has_post_format() should accept an array to Prevent plugins from overriding jQuery in the admin

sorry - title changed by accident

#14 @Ipstenu
11 years ago

Speaking as one who does that, F J Kaiser, I still think kevin has a point. It's a small number, and just something to factor in, but not something we should just sweep under the rug because it's a small number or because (IMO) it's a weird/silly thing to do.

Personally? I want this so bad my teeth hurt. :) But as a lesson learned from the recent wpdb->prepare, I think a doing_it_wrong first may be the way to go.

#15 @lightningspirit
11 years ago

  • Cc lightningspirit@… added

I would say if any plugin breaks the admin, the user will disable it for sure, but in the other hand I understand that this practice should be avoidable and I think calling doing_it_wrong should be the way to go.

#16 in reply to: ↑ 10 @F J Kaiser
11 years ago

Replying to kevinjohngallagher:

Here's a few off the top of my head:

  • IE6 support
  • SLA compliance
  • Digital Security compliance
  • Customised SaaS offerings
  • Reliance on external code

IE 6 support was dropped by WP officially more than one and a half years ago. You can read even more about it here. Even MicroSoft now has a countdown site.

Replying to Ipstenu:

Speaking as one who does that, F J Kaiser, I still think kevin has a point. (...)

Maybe. I just want to read the exact point and not just buzz words. And as you can see in the linked post by Jane: You can't even navigate in IE6. Now I'm wondering what exactly the SLA promises the client...

Replying to lightningspirit:

I would say if any plugin breaks the admin, the user will disable it for sure, (...)

After years that I've spent in the German/WPDE support forums and on WP.SE, I can promise you one thing for sure: The only thing the casual user will do is going into "white screen of death! panic!!"-mode and run down the next support alley. :)

#17 in reply to: ↑ description ; follow-up: @adamsilverstein
11 years ago

  • Cc ADAMSILVERSTEIN@… added

Replying to nacin:

This is getting ridiculous. It's time to force our own version of jQuery and avoid it being unregistered/re-registered by conventional means.

That's not to say it wouldn't be "impossible" (that's just not realistic), but it can prevent any plugin or theme doing its overriding blindly on init from breaking the admin.

i'd like to take a stab at this, i've seen this issue myself in the past. your summary state "Here's the problem I'm trying to solve. A theme hooks into init and de-registers jQuery, then registers another one elsewhere. This overrode the admin jQuery accidentally. That's just bad." can you point me to bad theme or give a line of code i can add to twentyeleven so it does the bad thing you mention. i think i know what you mean, but theres nothing like a reproducible test case for debugging.

#18 in reply to: ↑ 17 @chipbennett
11 years ago

Replying to adamsilverstein:

can you point me to bad theme or give a line of code i can add to twentyeleven so it does the bad thing you mention. i think i know what you mean, but theres nothing like a reproducible test case for debugging.

  1. Google "wp_deregister_script jquery"
  2. Pick any tutorial that appears in the search results
  3. For extra-fun breakage, pick one from 2010 or earlier, so that you're registering jQuery 1.3.x

#19 @adamsilverstein
11 years ago

i have attached two files. the first patch mangles the default twentytwelve theme by calling wp_deregister_script('jquery'); and requeueing jquery 1.3.2 from google. it breaks the interface and throws all sorts of javascript errors, give it a try, fun!

the second patch is my proposed solution. it is a sanity check that prevents wp_deregister_script from registering 'jquery' when in the dashboard. i did my very best to follow the WordPress coding practices, but I'm new here so clean me up if I did something wrong!

I'm with nacin on preventing this, i work with lots of end users and every single one would call me if they installed a theme or plugin that does this and their Dashboard went haywire!

#20 @adamsilverstein
11 years ago

  • Keywords has-patch 2nd-opinion added

@SergeyBiryukov
11 years ago

With proper formatting

#21 follow-up: @adamsilverstein
11 years ago

SergeyBiryukov: just to clarify re:your code corrections - i didn't need brackets because of the length of the conditional, right? did I mess anything else up or is that all you changed? thanks again!

#22 follow-up: @ocean90
11 years ago

  • Keywords 2nd-opinion removed

@adamsilverstein

We also use tabs instead of spaced of indentations.

Take a look at the handbook, there are our coding standards documented.

#23 in reply to: ↑ 22 ; follow-up: @adamsilverstein
11 years ago

Replying to ocean90:

@adamsilverstein

We also use tabs instead of spaced of indentations.

Take a look at the handbook, there are our coding standards documented.

yes, aware of tabs vs. spaces, did my patch have spaces? if so, my editor (phpstorm) introduced them when i copied the block from above. sure looked like tabs, but it must have changed them. i'm going to make sure i can see tabs vs spaces and not make that mistake again.

thanks for the coding standard link, i had only seen the page here: WordPress Coding Standards which looks similar... no harm reviewing :)

#24 in reply to: ↑ 23 @adamsilverstein
11 years ago

Replying to adamsilverstein:

Replying to ocean90:

@adamsilverstein

We also use tabs instead of spaced of indentations.

Take a look at the handbook, there are our coding standards documented.

yes, aware of tabs vs. spaces, did my patch have spaces? if so, my editor (phpstorm) introduced them when i copied the block from above. sure looked like tabs, but it must have changed them. i'm going to make sure i can see tabs vs spaces and not make that mistake again.

thanks for the coding standard link, i had only seen the page here: WordPress Coding Standards which looks similar... no harm reviewing :)

really like phpstorm, but default settings are wonky, was turning tabs to spaces!

#25 in reply to: ↑ 21 ; follow-up: @SergeyBiryukov
11 years ago

Replying to adamsilverstein:

i didn't need brackets because of the length of the conditional, right?

Yes, per the Brace Style section in the standards, single line blocks can omit braces for brevity.

I've also changed "in the Dashboard" to "in the admin", since Dashboard is just that specific screen.

#26 in reply to: ↑ 25 @adamsilverstein
11 years ago

Replying to SergeyBiryukov:

Replying to adamsilverstein:

i didn't need brackets because of the length of the conditional, right?

Yes, per the Brace Style section in the standards, single line blocks can omit braces for brevity.

I've also changed "in the Dashboard" to "in the admin", since Dashboard is just that specific screen.

right, ok. will try to stop making you fix my mistakes :)

#27 @mordauk
11 years ago

having this introduced will save me hours of support every single week, so yes please.

#28 @chriscct7
11 years ago

  • Cc chriscct7@… added

#29 @auniquename
11 years ago

I am faced with a similar problem, only in reverse. When the core wp included jquery and jquery ui changes, the plugin I am working on will likely break. Yes, one should keep pace with wp revisions and maintain their plugin, but the work to support multiple wp and jquery versions in a plugin will quickly snowball and reduce the code to a horrific snafu. It would be nice to be able to produce software that will run unmaintained for at least a year or two. Quarterly updates and a bazillion version checks and conditionals seems silly to me.

You see, I want to use jquery ui 1.10.0 in my plugin as it includes fixes that will make my life much easier. My development version of my plugin currently uses the wp bundled jquery and jquery ui as it is intended to be used, but makes performance and polish concessions to do so. Then when wp 3.6, 3.7, 3.8 ... come out with new bundled js libs, it is going to be a nightmare to maintain. I am sure I am not the only one in this dilemma, though perhaps few think it through as thoroughly as I...

Basically, my plugin makes heavy use of jquery ui dialogs and fully scoped css jquery ui themes. I am thinking of compatibility and isolation from the start. However, using fully scoped jquery ui themes results in issues that the jquery ui team had not thought of until recently and have only begun to address in v1.10.0.

So if I want to use jquery 1.9.0 and jqueryui 1.10.0 in my plugin I am going to have to bundle it and isolate it somehow. This would give me the performance, and functionality I desire but runs the risk of contaminating the wp core if not done properly.

My plugin will produce considerable amounts of dynamically generated javascript. So far the only thing I can think of to achieve the desired result and have a plugin that works for more than a few months without constant maintenance, is to include my required js libs with my dynamically generated js functions all wrapped into a single anonymous js function generated on the fly in php.

This is a two way street, and the current situation sucks for traffic going either way.

Long story short, I think wp needs to seriously rethink how it implements bundled js libs. The whole *_enqueue_script concept may just have to go eventually. Leave it there for old plugin compatibility but deprecate it and finally remove it, then establish a standard for isolating js lib dependencies.

If I am thinking seriously about this for my own plugin then I think that the core wp coding team should be having similar thoughts. Sure it is a lot of work and not ideal, and will result in large amounts of revisions throughout the product. But if I have to make such concessions for a stable plugin that does not disturb the wp core, then I think the core should be taking similar steps to protect itself.

I'm still stewing this over in my head and have not yet decided on the proper solution. But for now wrapping it all with the required libs in one anonymous function is all I have.

Another approach might be a rethought js lib loader, one that plugins and the core alike can use, but differing from the current one in that it would sandbox each and every lib registered with it. I hate the idea of using cdns for js libs, again for software stability and lifetime considerations. It makes no sense for a site or code to depend on others that may or may not be there in the future.

Apologies for the long post, and it may not help much, but this is a complicated problem that needs a solution in order to produce stable software with a reasonable shelf-life. If I come up with a good solution I will surely let you know.

Sorry... more... a little research on the older approaches:

http://sourceforge.net/projects/jsloader/ (virtually ancient at this point)

http://www.andresvidal.com/jsl (better but still does not solve the problem)

http://code.google.com/p/javascript-loader/ (looking pretty weak, and stale...)

https://github.com/getify/LABjs (oh I remember this guy! but look at the readme...)

https://github.com/jrburke/requirejs http://requirejs.org/ (now we're getting somewhere...)

What's this AMD pattern? A quick Bing ... (Google is evil now with a cloaked tracking redirect on all result links, anyway...) So what is "the AMD pattern"?

http://unscriptable.com/code/Using-AMD-loaders/

AMD = Asynchronous Module Definition Aha! Well what exactly is that? The previous link begins to explain...

http://www.commonjs.org/specs/

And lists these as choices:

RequireJS http://requirejs.org/

curl.js http://github.com/unscriptable/curl

bdLoad http://bdframework.org/bdLoad

JSLocalnet http://www.localnet.org.es/

Yabble http://github.com/jbrantly/yabble

PINF http://github.com/pinf/loader-js

And interestingly:

cssx http://github.com/unscriptable/cssx

XStyle http://github.com/kriszyp/xstyle

define() huh? Hmmm...

http://unscriptable.com/2011/09/30/amd-versus-cjs-whats-the-best-format/

https://github.com/amdjs/amdjs-api/wiki

I think I'm going to take a good look at RequireJS. I don't think it will solve this problem, but might get me closer to a solution. I suspect the answer may be to create an AMD fork of jquery and jquery ui, or better yet just get the jquery people to adopt it.

The answer is out there and it is up to people like us to push for the solution. (sorry for all the link spam and long rambling post, but I'm just getting back into web development and writing things out brainstorm style helps me sort it all out..)

Bottom line, in my opinion there needs to be major changes to both wp and jquery to solve this problem in the long term, but I'll see what kind of band-aids I can come up with in the meantime

*****

EDIT: wrapping a custom named noconflict jquery in an anonymous function does not work - whatever loads jquery last wins... (I should know better than to trust stackoverflow comments)

At this point I see two possible solutions for wp plugin authors and the wp core:
1) Do not use jQquery - write your own uniquely named javascript functions - no libs.
2) Rename (prefix) every single jquery and jquery plugin function (ugh)

Today I will give RequireJS and its Asynchronous Module Definition implementation a try and see if it is of any benefit for this dilemma.

*****

EDIT2: Hmmmm.... Check out the end of the unminified jQuery source:

// Expose jQuery as an AMD module, but only for AMD loaders that
// understand the issues with loading multiple versions of jQuery
// in a page that all might call define(). The loader will indicate
// they have special allowances for multiple jQuery versions by
// specifying define.amd.jQuery = true. Register as a named module,
// since jQuery can be concatenated with other files that may use define,
// but not use a proper concatenation script that understands anonymous
// AMD modules. A named AMD is safest and most robust way to register.
// Lowercase jquery is used because AMD module names are derived from
// file names, and jQuery is normally delivered in a lowercase file name.
// Do this after creating the global so that if an AMD module wants to call
// noConflict to hide this version of jQuery, it will work.
if ( typeof define === "function" && define.amd && define.amd.jQuery ) {
	define( "jquery", [], function () { return jQuery; } );
}

Note: multiple versions of jQuery

Promising... now if jQuery UI and Iris do the same then I might be all set!
Oh, it seems they do not... but perhaps if I wrap them in defines as suggested...

http://requirejs.org/docs/why.html

http://requirejs.org/docs/whyamd.html

https://github.com/amdjs/amdjs-api/wiki/AMD

This is making my brain hurt! But I think I have found the answer here. Bunch of work to test it out... but it looks like this is the way to go - the standard to use to allow WordPress and plugins written for it to use the js libs they require in safety. (?) But will it work for me now, before WordPress implements an AMD js loader? Or do I have to contribute to WordPress before I can make a js heavy plugin and count on it to work for more than a few months? Sigh.

I am not just concerned with the admin side of WordPress - while a factor in my plugin, the conflicts occur for me on the front end in collision with other plugins (I could live with using the native wp jQuery in admin where I need minimal functionality, but it is on the front end where the real problems are...)

I think maybe I should stop hijacking this ticket and start one dedicated to discussion of an AMD js loader for WordPress...

New feature request ticket created:
http://core.trac.wordpress.org/ticket/23285

I now return you to your regularly scheduled bitching and moaning.

*****

EDIT3: Quick update, I went with prefixing my desired jQuery, jQuery UI and Iris versions. I just did a find replace on the unminified sources: jQuery -> PREFIXjQuery and jquery -> prefixjquery (both case sensitive), then concatenated and minified into a single prefix-libs.js file. (With a PREFIXjQuery.noConflict() call right in the concatenated libs file right after the prefixed jQuery and before the prefixed plugins.) Then registered and enqueued that file in the normal way. So far it looks like I have successfully isolated multiple jQuery versions without errors in WordPress (admin and front end) as confirmed by several calls:

alert(jQuery().jquery);
alert(PREFIXjQuery().prefixjquery);

PREFIXjQuery(document).ready(function($) {

	alert(jQuery().jquery);
	alert($().prefixjquery);

});

jQuery(document).ready(function($) {

	alert($().jquery);
	alert(PREFIXjQuery().prefixjquery);

});

Which reports 1.8.3, 1.9.0 and 1.8.3, 1.9.0 and 1.8.3, 1.9.0 as expected. (WP v3.5)

So that's my bulky band-aid to stem the hemorrhagic tide.

Ugly, hacky and inefficient, I know, but by my reasoning there is no point to me implementing an AMD loader in my plugin unless the WordPress core is doing the same, and once wp implements an AMD loader, I will not need to do so in my plugin. So for the correct, unhacky solution, I wait, and if motivated strongly enough - contribute, though I'd prefer to see a more experienced developer implement an AMD loader in the wp core code.

This way, in the meantime, I can carry on developing a plugin that works the way I want and will continue to work regardless of jQuery changes in WordPress.

Also, this an answer to the original issue of this ticket. WordPress could do the same as I have for my plugin by creating WPjQuery/wpjquery prefixed versions of jQuery and its plugins to be used by the core admin functionality, that would not be over-ridable by plugins. Though as mentioned and requested a proper AMD loader implementation is the correct solution as I currently see it.

*****

EDIT4: Yup, confirmed isolation of jQuery using my hack. Yay! Now I have some thoughts for the jQuery UI team on DOM management. It seems no one over there tests functions under scoped css themes. Stuff keeps dropping out of scope depending on what functions are run on the elements. Which is ultimately my whole drive here. Reliable and complete scoping of jQuery UI themes.

Last edited 11 years ago by auniquename (previous) (diff)

#30 follow-ups: @azaozz
11 years ago

With jQuery there is a better way to do this: let the WordPress jQuery load first, then load your desired version in the footer or right before it's needed, load jQuery UI or any other plugins you want to use with the different version and do:

var myjq = jQuery.noConflict(true);

Ref: http://api.jquery.com/jQuery.noConflict/#example-4

Then in your scripts you can use the version you want:

(function($){
// your code here uses the alternative jQuery version
}(myjq));

(function($){
// this uses the standard jQuery version
}(jQuery));

This is the proper way to load several versions of jQuery and have them all work and (of course) doesn't affect or break anything in WordPress.

#31 in reply to: ↑ 30 @adamsilverstein
11 years ago

Replying to azaozz:

With jQuery there is a better way to do this: let the WordPress jQuery load first, then load your desired version in the footer or right before it's needed, load jQuery UI or any other plugins you want to use with the different version and do:

var myjq = jQuery.noConflict(true);

Ref: http://api.jquery.com/jQuery.noConflict/#example-4

Then in your scripts you can use the version you want:

(function($){
// your code here uses the alternative jQuery version
}(myjq));

(function($){
// this uses the standard jQuery version
}(jQuery));

This is the proper way to load several versions of jQuery and have them all work and (of course) doesn't affect or break anything in WordPress.

brilliant! thanks.

#32 @adamsilverstein
11 years ago

in any case, all this patch does is prevent wp_deregister_script('jquery') in the admin.

#33 in reply to: ↑ 30 ; follow-ups: @auniquename
11 years ago

Replying to azaozz:

With jQuery there is a better way to do this: let the WordPress jQuery load first, then load your desired version in the footer or right before it's needed, load jQuery UI or any other plugins you want to use with the different version and do:

var myjq = jQuery.noConflict(true);

Ref: http://api.jquery.com/jQuery.noConflict/#example-4

Then in your scripts you can use the version you want:

(function($){
// your code here uses the alternative jQuery version
}(myjq));

(function($){
// this uses the standard jQuery version
}(jQuery));

This is the proper way to load several versions of jQuery and have them all work and (of course) doesn't affect or break anything in WordPress.

*****

I of course tried exactly that first (see long boring story above). Like so:

/* included minified jQuery */

var mypluginjq = jQuery.noConflict();

/* included minified jQuery UI */
/* included minified Iris */

mypluginjq(document).ready(function($) {

    /* My Plugin jQuery stuff */

});

As instructed here:
http://docs.jquery.com/Using_jQuery_with_Other_Libraries
and here:
http://docs.jquery.com/Core/jQuery.noConflict

It did not work. When I called a jQuery version check outside and within the scoped functions it reported the same version everywhere - the last one loaded. And when called from other plugins, all reported the same version of jQuery, the last one loaded.

The difference being perhaps that I am mostly trying to use fully scoped jQuery UI themes and I see detail here that most do not or perhaps never will.

My guess is that no one has ever tested enough functionality to see a difference. Or actually made version calls.

Maybe I screwed something up, but I don't think so. I'll try a very simple test page to see...

Version 1, edited 11 years ago by auniquename (previous) (next) (diff)

#34 follow-up: @wonderboymusic
11 years ago

I'm gonna need to see more of your plugin code before I am convinced that we're all stupid. I am currently on a production site running different jQuery in admin / front end, running jQuery UI, running tons of Backbone, Underscore, etc and have had zero problems. Can you put some code in a Gist or attach some code to the ticket that shows what is breaking?

#35 in reply to: ↑ 34 @auniquename
11 years ago

Replying to wonderboymusic:

I'm gonna need to see more of your plugin code before I am convinced that we're all stupid. I am currently on a production site running different jQuery in admin / front end, running jQuery UI, running tons of Backbone, Underscore, etc and have had zero problems. Can you put some code in a Gist or attach some code to the ticket that shows what is breaking?

*****

Not accusing anyone of being stupid. That would be mean. If anyone, it is me. But I do not think so. I cannot share my plugin code currently as I am writing it for a private client.

I will create a test package to illustrate.

The idea is to try and run multiple versions of jQuery on the same page when you are not in control of the order in which they are loaded, which without some guiding standard or controlling framework is not currently possible. (the current wp situation)

Please standby for test demonstration. I might prove myself wrong. But I think I already know the answer, it will depend on when and where jQuery functions are called.

Oh right... the key here is that all plugins must use a named alias for the version of jQuery loaded, which is not the case. I will demonstrate. Standby, it will take a bit...

Last edited 11 years ago by auniquename (previous) (diff)

#36 in reply to: ↑ 33 @SergeyBiryukov
11 years ago

Replying to auniquename:

Replying to azaozz:

var myjq = jQuery.noConflict(true);

Ref: http://api.jquery.com/jQuery.noConflict/#example-4

I of course tried exactly that first (see long boring story above). Like so:

var mypluginjq = jQuery.noConflict();

Note that your code doesn't set the removeAll parameter, which tells jQuery to remove all the variables from the global scope (including jQuery itself). The link above is an example of loading two different jQuery versions at the same time.

#37 in reply to: ↑ 33 ; follow-up: @azaozz
11 years ago

Replying to auniquename:

I of course tried exactly that first (see long boring story above)...

I may be mis-reading something but as Sergey pointed out above

var mypluginjq = jQuery.noConflict();

won't do it. You need

var mypluginjq = jQuery.noConflict(true);

so both jQuery and $ are returned to the original values. See ​http://api.jquery.com/jQuery.noConflict/#example-4

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

#38 in reply to: ↑ 37 @auniquename
11 years ago

Replying to azaozz:

I may be mis-reading something but as Sergey pointed out above

var mypluginjq = jQuery.noConflict();

won't do it. You need

var mypluginjq = jQuery.noConflict(true);

so both jQuery and $ are returned to the original values. See ​http://api.jquery.com/jQuery.noConflict/#example-4

*****

OK, I see now, thank you all for your patience and helpfulness. I know this is not really the place for such assistance and this is somewhat a tangent to the original topic of this ticket (though not entirely).

So what I want to do in my myplugin-libs.js file is:

/* included minified jQuery */

var mypluginjq = jQuery.noConflict(true);

(function($){

/* included minified jQuery UI */
/* included minified Iris */

}(mypluginjq));

...and in my myplugin.js

mypluginjq(document).ready(function($) {

    /* Finally, my plugin jQuery stuff */

});

No ugly hacks needed! Sweet. This makes the demonstration I was working on redundant, though it would still illustrate where the standard approach will go wrong...

So the question remains, what about plugin authors who have not thought it through like this and simply use the WordPress bundled jQuery as recommended in the codex? Their plugins will break in a relatively short time. I still think an AMD style solution of some kind is required in the long run. In any case, I'd say some additions to http://codex.wordpress.org/Function_Reference/wp_enqueue_script are in order. If not there, then some more appropriate and prominent location.

#39 @auniquename
11 years ago

Hrm. This approach is not working for jQuery plugins. I cannot make the call to jQuery.noConflict(true) before loading jQuery plugins, as they still reference (jQuery). So if I make the jQuery.noConflict call after including my desired jQuery plugins the question is will the jQuery plugins collide? I'll have to find a way to test that.

This approach does seem to work (my-libs.js):

/* included minified jQuery */
/* included minified jQuery UI */
/* included minified Iris */
var MYjQuery = jQuery.noConflict(true);

Confirmed in (my.js):

alert("Native jQuery version: " + jQuery().jquery);
alert("Native jQuery UI version: " + jQuery.ui.version);
alert("MY jQuery version: " + MYjQuery().jquery);
alert("MY jQuery UI version: " + MYjQuery.ui.version);

MYjQuery(document).ready(function($) {
	alert("Native jQuery version: " + jQuery().jquery);
	alert("Native jQuery UI version: " + jQuery.ui.version);
	alert("MY jQuery version: " + $().jquery);
	alert("MY jQuery UI version: " + $.ui.version);
});

jQuery(document).ready(function($) {
	alert("Native jQuery version: " + $().jquery);
	alert("Native jQuery UI version: " + $.ui.version);
	alert("MY jQuery version: " + MYjQuery().jquery);
	alert("MY jQuery UI version: " + MYjQuery.ui.version);
});

Yields the expected version in all places called (both scripts loaded normally via wp_register_script, wp_enqueue_script and forcing a native jQuery UI load adding it to the dependencies array for my.js ).

Now the question is what to do about Iris, as it is not a jQuery plugin, but is dependent on it. I have the hook set and checked for my plugin options page so on the admin side all this is only loaded on my options page anyway, and I do not need Iris for my front end but would be nice to have the option though...

edit: actually Iris is a jQuery plugin, it's included Color.js is not however.

Do I want to depend on the native version of Iris and risk my plugin breaking at some point in the future? Hmmm. I would prefer to include and isolate Iris as well, so I can count on it always being there and behaving as expected. It appears to be globally scoped. What to do? (not really asking here, just mulling it over...)

Ah, but wait! What if somebody else loads WP native (and/or non-native) jQuery and jQuery UI on the same page before and/or after I do... so many test cases dammit.

Last edited 11 years ago by auniquename (previous) (diff)

#40 @adamsilverstein
11 years ago

brilliant, patient, thoughtful discussion! now lets get this patch committed!

#41 @nacin
11 years ago

I don't think the '?' check is proper here. While a script registered via enqueue() does split based on the query string, one that is simply registered does not. Likewise, WP_Dependencies::remove() avoids the check.

#42 @nacin
11 years ago

In 23377:

Stop recommending the init hook in the _doing_it_wrong() message for too-early scripts and styles. Instead, recommend the three _enqueue_scripts hooks. If they're noticing they are doing it wrong, let's push them to 100% correct, not partly correct.

see #22896.

#43 @nacin
11 years ago

After reading through this thread and also talking to scribu in IRC, I am going to proceed with a patch that acts more broadly but is slightly less agressive.

Basically, is_admin() is the first of two checks we should be making. The other is whether the current hook is admin_enqueue_scripts. Chances are, if they are using the correct hook, then they know what they are doing. This still prevents blind de-registrations, and should solve the vast majority of support requests while still providing some flexibility to plugins and technical requirements.

By broadly, I mean, let's block all of jQuery and jQuery UI*, not just jQuery.

(* I left out jQuery UI effects because it just seems like a waste of array size.)

#44 @nacin
11 years ago

In 23378:

Do not allow accidental or negligent deregistering of critical scripts in the admin. (Defined for now as jQuery and jQuery UI.) Show minimal remorse if the correct hook is used. see #22896.

#45 @SergeyBiryukov
11 years ago

In 23379:

Mark the string for translation. see #22896.

#46 @adamsilverstein
11 years ago

thanks for the attention, this change will be appreciated :)

#47 follow-ups: @azaozz
11 years ago

Perhaps Underscore.js and Backbone.js should be in the $no list too, see #23262.

#48 in reply to: ↑ 47 @adamsilverstein
11 years ago

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

Replying to azaozz:

Perhaps Underscore.js and Backbone.js should be in the $no list too, see #23262.

added these in patch 22896-with-backbone-and-underscore.diff​

#49 in reply to: ↑ 47 @adamsilverstein
11 years ago

Replying to azaozz:

Perhaps Underscore.js and Backbone.js should be in the $no list too, see #23262.

i closed this ticket as it was already resolved and committed. i opened another ticket - #23417 - to add backbone and underscore to the $no list

#50 @nacin
11 years ago

#23417 was marked as a duplicate.

#51 @nacin
11 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

I left this ticket open with [23378] because I didn't think it was finalized. I closed #23417 as it is nice to keep the conversations in one place. At least as long as a ticket doesn't get exceedingly long or out of hand, and I think we're good here.

I actually meant to commit Backbone and Underscore a few days ago, but the commit got rejected as my working copy was not up to date.

#52 @chriscct7
11 years ago

  • Cc chriscct7@… removed

#53 follow-up: @nacin
11 years ago

In 23391:

Add underscore and backbone to the do-not-deregister list of scripts.

props adamsilverstein.
see [23378]. see #22896.

@adamsilverstein
11 years ago

unit test for #22896

#54 in reply to: ↑ 53 ; follow-up: @adamsilverstein
11 years ago

Replying to nacin:

In 23391:

Add underscore and backbone to the do-not-deregister list of scripts.

props adamsilverstein.
see [23378]. see #22896.

thanks for fixing that, i thought since this ticket was so long and patched it deserved a new ticket, but you know better.

i added a unit test :) please review, thanks.

#55 @chriscct7
11 years ago

@nacin might be a bug in Trac but when I un-CC'd, I'm still getting the emails for this.

#56 in reply to: ↑ 54 ; follow-ups: @nacin
11 years ago

Replying to adamsilverstein:

i thought since this ticket was so long and patched it deserved a new ticket, but you know better.

Not a problem. This is ticket has a very high signal-to-noise ratio, and isn't terribly long. Contrast with #23119... and #11817.

i added a unit test :) please review, thanks.

Great. Some points:

  • No need for setUp or tearDown if you have no code there. Parent methods will be called.
  • Use @ticket 23417 in a properly indented doc block to label the test as linked to this ticket. You can then run it with phpunit --group 23417.
  • We call these dependencies, and there are existing tests. This method probably go in Test_Dependencies_jQuery (tests/dependencies/jquery.php).

#57 in reply to: ↑ 56 @adamsilverstein
11 years ago

Replying to nacin:

Replying to adamsilverstein:

i thought since this ticket was so long and patched it deserved a new ticket, but you know better.

Not a problem. This is ticket has a very high signal-to-noise ratio, and isn't terribly long. Contrast with #23119... and #11817.

i added a unit test :) please review, thanks.

Great. Some points:

  • No need for setUp or tearDown if you have no code there. Parent methods will be called.
  • Use @ticket 23417 in a properly indented doc block to label the test as linked to this ticket. You can then run it with phpunit --group 23417.
  • We call these dependencies, and there are existing tests. This method probably go in Test_Dependencies_jQuery (tests/dependencies/jquery.php).

thanks for the unit-test feedback, i appreciate the help.

i will update my test and add it to the proper file and resubmit as a diff.

@adamsilverstein
11 years ago

unit test for #22896, take 2

#58 in reply to: ↑ 56 ; follow-up: @adamsilverstein
11 years ago

Replying to nacin:

Replying to adamsilverstein:

i thought since this ticket was so long and patched it deserved a new ticket, but you know better.

Not a problem. This is ticket has a very high signal-to-noise ratio, and isn't terribly long. Contrast with #23119... and #11817.

i added a unit test :) please review, thanks.

Great. Some points:

  • No need for setUp or tearDown if you have no code there. Parent methods will be called.
  • Use @ticket 23417 in a properly indented doc block to label the test as linked to this ticket. You can then run it with phpunit --group 23417.
  • We call these dependencies, and there are existing tests. This method probably go in Test_Dependencies_jQuery (tests/dependencies/jquery.php).

i reworked as a diff, does that look better?

i'm trying to write unit tests for every bug i encounter, they seem super useful and fairly straightforward to me. it would be great if there was a 'has-unit-test' tag available i could use to mark tests for review!

#59 in reply to: ↑ 58 ; follow-up: @duck_
11 years ago

Replying to adamsilverstein:

i reworked as a diff, does that look better?

PHPUnit provides an assertTrue() method that would be nicer to use instead of assertEquals(true, ...).

@adamsilverstein
11 years ago

updated unit test for #22896

#60 in reply to: ↑ 59 @adamsilverstein
11 years ago

Replying to duck_:

Replying to adamsilverstein:

i reworked as a diff, does that look better?

PHPUnit provides an assertTrue() method that would be nicer to use instead of assertEquals(true, ...).

thanks for the tip, appreciate it... i'm new to unit tests.

i cleaned the patch up and re-posted.

@adamsilverstein
11 years ago

removed enqueue line, set screen back to starting screen

#61 @nacin
11 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from reopened to closed

In 1223/tests:

Test that certain core scripts cannot be deregistered in the admin. props adamsilverstein. fixes #22896.

#62 @jeherve
11 years ago

  • Cc jeremy+wp@… added
Note: See TracTickets for help on using tickets.