#12882 closed Uncategorized (fixed)
jQuery.noConflict() in admin breaks site specific code with jQuery
| Reported by: | Owned by: | Jannis Leidel | |
|---|---|---|---|
| Component: | contrib.admin | Version: | dev |
| Severity: | Normal | Keywords: | jQuery admin |
| Cc: | ales.zoulek@…, yed@…, rob@…, semente@…, michael.c.strickland@…, Gabriel Hurley | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | yes | Patch needs improvement: | yes |
| Easy pickings: | no | UI/UX: | no |
Description (last modified by )
JQuery javascipt included by django admin calls noConflict method ( http://api.jquery.com/jQuery.noConflict/ ). This method removes variable $ shortcut from the global namespace.
Site specific code can include an own jQuery code to enrich admin pages. In this case, commonly used variable $ is undefined and page is broken.
Attachments (3)
Change History (34)
comment:1 by , 15 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 15 years ago
| Cc: | added |
|---|
comment:3 by , 15 years ago
| Cc: | added |
|---|
comment:4 by , 15 years ago
comment:5 by , 15 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:6 by , 15 years ago
jezdez: Admin's jquery.min.js may overide some site-specific javascript. And since there are tons of jQuery plugins, versions and possible combinations, (and I suspect a lot of more customized admin sites uses jQuery) site should be always able to enforce it's jQuery version (and thus be responsible for and core disfunctionalities it may cause).
At the moment, overwriting default jquery is possible either by:
- rewring ModelAdmin.media property (in every admin option class, or by monkeypatching)
- copy'n'pasting templates/admin/change_list.html and changing only "extrahead" block
Extending "extrahead" block in base.html or site_base.html is not possible since change_list.html writes it's media (including jquery) AFTER {{block.super}}.
Possible solutions would be one of:
- Including all admin javascripts in base.html (What is the dynamic contruction of list of internal JS files to include on every page good for, anyway? :) )
- Introduce {% block customjavascript %} block tag in admin/base.html that would go after {% block extrahead %} since there's no easy way to put custom tags at the end of extrahead block.
Workaround:
- Extend {% block blockbots %} tag in custom base_site.html and put custom jQuery there.
follow-up: 9 comment:7 by , 15 years ago
about plugins, see symbolic example
<head>
<script JQuery by Site>
<script Plugin 1 (by site)>
<script JQuery by Django>
<script Plugin 2 (by Django)> (eq. actions.min.js)
<head>
In example above JQuery by Django redefine jquery instance, so Plugin 1 is not defined on new jQuery instance. For example it can be solved by ommiting first jQuery and putting Plugin 1 include after Plugin 2. (But jquery by django is included in media part which is last in head, so it needs some work to override it (especially if it can be generic solution for whole project and not only one template))
In fact it is not main topic of this ticket, but i would like to show more issues with jQuery include by Django. I my opinion is not a bad idea use this great library, but O guess it should be done in more customizable way and with respect about site which can use own jquery (maybe in worst case in different version) with it's own jQuery plugins. Present situation must be hacked by ugly workarounds in client code.
comment:8 by , 15 years ago
And finally my workaround for noConflict issue is
jQuery.noConflict = function() { return this; }; {# HACK - Django calls it #}
in right place in html document.
comment:9 by , 15 years ago
Replying to krejcik:
In fact it is not main topic of this ticket, but i would like to show more issues with jQuery include by Django. I my opinion is not a bad idea use this great library, but O guess it should be done in more customizable way and with respect about site which can use own jquery (maybe in worst case in different version) with it's own jQuery plugins. Present situation must be hacked by ugly workarounds in client code.
Absolutely, that's why we are here, fixing that bug :)
comment:10 by , 15 years ago
| Cc: | added |
|---|
It is possible to completely encapsulate Django's jQuery version and its plugins, but I'm curious if that's the best thing to do here. If it's known that the Django admin has jQuery available via jQuery in the global namespace, custom admin site changes can code to that. I, for one, like knowing I can use Django's version of jQuery for some custom widgets and such.
However, if we do want to encapsulate things so different versions of jQuery can co-exist, it is possible following this skeleton code:
(function(window, document, version, callback) {
var j, d;
var loaded = false;
if (!(j = window.jQuery) || version > j.fn.jquery || callback(j)) {
var script = document.createElement("script");
script.type = "text/javascript";
script.src = "/path/to/jquery.js";
script.onload = script.onreadystatechange = function() {
if (!loaded && (!(d = this.readyState) || d == "loaded" || d == "complete")) {
callback((j = window.jQuery).noConflict(1), loaded = true);
j(script).remove();
}
};
document.documentElement.childNodes[0].appendChild(script);
}
})(window, document, "1.4", function($, jquery_loaded) {
// plugin code goes here.
});
What this code does is checks if there is an existing version of jQuery loaded and checks it meets a given minimum version and if so, uses that jQuery. If it doesn't exist, or doesn't meet the minimum version, it dynamically adds a script tag to load its own jQuery and uses that. The version loaded is encapsulated within the anonymous function and will not conflict with any other Javascript libraries (e.g. other versions of jQuery, prototype, etc.).
It seems a bit overkill to me, but if it's the best option, feel free to use it.
-Rob
comment:11 by , 15 years ago
Because jQuery has stable API, which is almost backward compatible, I agree that solving different version seems to be overkill.
Personaly I see problem only in noConflict call and in jQuery instance replacement by another JS include (which cause lost of previously defined jQuery plugins as desciribed in a previous post).
by , 15 years ago
| Attachment: | 12882.1.diff added |
|---|
Pass true to noConflict as mentioned on http://api.jquery.com/jQuery.noConflict/
comment:12 by , 15 years ago
In the attached patch I'm referring to the part of jQuery's documentation:
"If necessary, we can free up the jQuery name as well by passing true as an argument to the method. This is rarely necessary, and if we must do this (for example, if we need to use multiple versions of the jQuery library on the same page), we need to consider that most plug-ins rely on the presence of the jQuery variable and may not operate correctly in this situation."
Question is, can we test this somehow? Does it solve your issue, krejcik?
comment:13 by , 15 years ago
"In this case, commonly used variable $ is undefined and page is broken."
krejcik: can't this be solved by just doing wrapping your code in a function? Like so:
(function($) {
// your jQuery code goes here.
})(jQuery);
I do this very commonly in my own code, just to ensure that if someone decides to use another library or screw with the $ variable (not that uncommon on large projects), my scripts can still be reused just fine when jQuery is in noConflict mode.
My apologies if I am missing something about why this wouldn't work.
comment:14 by , 15 years ago
taylormarshall: yes it can be used in this way. but I guess main problem is in jQuery plugins. (after second jQuery include, previosly included plugins are lost
comment:15 by , 15 years ago
| Cc: | added |
|---|
comment:16 by , 15 years ago
| Cc: | added; removed |
|---|
comment:17 by , 15 years ago
| Cc: | added |
|---|
comment:18 by , 15 years ago
| Description: | modified (diff) |
|---|---|
| Summary: | jQuery.noConflict() in admin brokes site specific code with jQuery → jQuery.noConflict() in admin breaks site specific code with jQuery |
comment:19 by , 15 years ago
| Has patch: | set |
|---|
comment:20 by , 15 years ago
| Patch needs improvement: | set |
|---|
I'm not sure yet if the 12882.2.diff patch is enough and the right approach. I've been playing with the idea of making the shipped jQuery live in a namespace, as in
var django = {};
django.query = jQuery.noConflict();
comment:21 by , 15 years ago
| Needs tests: | set |
|---|
comment:22 by , 15 years ago
| Cc: | added |
|---|
comment:23 by , 15 years ago
jezdez: Use of jQuery.noConflict(true) will remove jQuery from the global namespace, which means that any jQuery call that appears after that line on the page will stop working. For example, if you have both a stacked inline and a tabular inline, the second stops working since the first removed jQuery (I've tested that).
So, the options left are:
- provide a mechanism to stop multiple instances of jQuery being loaded
- either based on overriding template blocks
- or something like Rob Hudson's code
- your django namespace idea.
comment:24 by , 15 years ago
Personally I'm a fan of namespacing Django's copy of jQuery.
That way developers who want to use the vanilla admin jQuery can do so with a reliable, stable namespace for it while those who want their own copy, own plugins, etc. can do so conflict-free. That strategy would work well with the custom admin code I have written currently and seems like a more maintainable mechanism in the long run.
Preventing loading of multiple jQuery instances via template blocks sounds messy and likely to be broken by people hacking at the admin.
Rob Hudson's code definitely works but still feels brittle to me, like if someone includes their script in the wrong place. I'd rather see something more robust and separate.
That's my $0.02 for what it's worth.
comment:25 by , 15 years ago
++1 on django's admin keeping its own jquery isolated in its own namespace
the admin is already too brittle and has by far taken up more of my development time than any other aspect of working with django. tracking the media includes (with inlines!) is a real chore just to identify who still is including jquery.
comment:26 by , 15 years ago
With regards to the original bug here, I don't think we have got to the bottom of it. jQuery.noConflict() is supposed to restore the original definition of $, which has been saved in a global variable _$ when jQuery initializes. I can see that this may fail if 3 or more instances of jQuery are loaded, but it should work correctly with 2.
If we go for the Django namespace, then the call should be:
django.jQuery = jQuery.noConflict(true);
...so that the original jQuery is restored. All code/plugins will need to be updated for this, but in all cases so far it should be a minimal change - it needs to look like this:
function($) { ... }(django.jQuery);
by , 15 years ago
| Attachment: | 12882.3.diff added |
|---|
Moved admin jQuery into our own namespace to lower the risk of clash with third party apps that use jQuery.
comment:27 by , 15 years ago
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
comment:29 by , 13 years ago
| Easy pickings: | unset |
|---|---|
| Resolution: | fixed |
| Severity: | → Normal |
| Status: | closed → reopened |
| Type: | → Uncategorized |
| UI/UX: | unset |
The problem with [16415] is that if I include my own jQuery in the admin's extrahead block, which the jQuery copy shipped by the admin overwrites, the admin will only restore window.$ and won't restore window.jQuery. This breaks things like django-selectable (which expects the jQuery UI plugin to be loaded on the jQuery object), and most jQuery code which lives in a jQuery(function($) { ... }); scope.
This should be fixed by reverting [16415] and requiring jQuery users to bring their own jquery script tag; admin-only addons can still use the django.jQuery(function($){ ... }); idiom.
comment:30 by , 13 years ago
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
Please don't reopen old tickets but open new ones for issues that have been fixed.
I'm not sure what you are saying with ".. causing problem with strip of jQuery plugins after redefinition". Mind elaborating?