Opened 14 years ago

Closed 13 years ago

Last modified 12 years ago

#12882 closed Uncategorized (fixed)

jQuery.noConflict() in admin breaks site specific code with jQuery

Reported by: krejcik@… 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 Jannis Leidel)

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)

12882.1.diff (858 bytes ) - added by Jannis Leidel 14 years ago.
Pass true to noConflict as mentioned on http://api.jquery.com/jQuery.noConflict/
12882.2.diff (1.4 KB ) - added by Jannis Leidel 14 years ago.
The collapse widget needs jQuery, too
12882.3.diff (11.0 KB ) - added by Jannis Leidel 14 years ago.
Moved admin jQuery into our own namespace to lower the risk of clash with third party apps that use jQuery.

Download all attachments as: .zip

Change History (34)

comment:1 by Jannis Leidel, 14 years ago

Triage Stage: UnreviewedAccepted

comment:2 by anonymous, 14 years ago

Cc: ales.zoulek@… added

comment:3 by Jiri Suchan, 14 years ago

Cc: yed@… added

comment:4 by Jannis Leidel, 14 years ago

I'm not sure what you are saying with ".. causing problem with strip of jQuery plugins after redefinition". Mind elaborating?

comment:5 by Jannis Leidel, 14 years ago

Owner: changed from nobody to Jannis Leidel
Status: newassigned

comment:6 by Ales Zoulek, 14 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.

comment:7 by krejcik, 14 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 krejcik, 14 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.

in reply to:  7 comment:9 by Jannis Leidel, 14 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 Rob Hudson, 14 years ago

Cc: rob@… 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 krejcik, 14 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 Jannis Leidel, 14 years ago

Attachment: 12882.1.diff added

Pass true to noConflict as mentioned on http://api.jquery.com/jQuery.noConflict/

comment:12 by Jannis Leidel, 14 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?

by Jannis Leidel, 14 years ago

Attachment: 12882.2.diff added

The collapse widget needs jQuery, too

comment:13 by taylormarshall, 14 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 krejcik, 14 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 Guilherme Gondim (semente) <semente@…>, 14 years ago

Cc: semente@… added

comment:16 by Guilherme Gondim (semente) <semente@…>, 14 years ago

Cc: semente@… added; semente@… removed

comment:17 by Gabriel Hurley, 14 years ago

Cc: Gabriel Hurley added

comment:18 by Jannis Leidel, 14 years ago

Description: modified (diff)
Summary: jQuery.noConflict() in admin brokes site specific code with jQueryjQuery.noConflict() in admin breaks site specific code with jQuery

comment:19 by Ramiro Morales, 14 years ago

Has patch: set

comment:20 by Jannis Leidel, 14 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();
Last edited 13 years ago by Jannis Leidel (previous) (diff)

comment:21 by Jannis Leidel, 14 years ago

Needs tests: set

comment:22 by anonymous, 14 years ago

Cc: michael.c.strickland@… added

comment:23 by Luke Plant, 14 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 Gabriel Hurley, 14 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 crucialfelix, 14 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 Luke Plant, 14 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);
Version 0, edited 14 years ago by Luke Plant (next)

by Jannis Leidel, 14 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 Jannis Leidel, 14 years ago

Resolution: fixed
Status: assignedclosed

(In [12966]) Fixed #12882 - Moved the admin's jQuery into our own namespace to lower the risk of a clash with third party apps that use jQuery.

comment:28 by Jannis Leidel, 13 years ago

In [16415]:

Stopped the admin JavaScript code from completely removing the jQuery object from the global namespace to ease the use of jQuery plugins. The django.jQuery object is still supposed to be used by Django's own JavaScript files. Refs #12882.

comment:29 by anonymous, 13 years ago

Easy pickings: unset
Resolution: fixed
Severity: Normal
Status: closedreopened
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 Jannis Leidel, 13 years ago

Resolution: fixed
Status: reopenedclosed

Please don't reopen old tickets but open new ones for issues that have been fixed.

comment:31 by Jacob, 12 years ago

milestone: 1.2

Milestone 1.2 deleted

Note: See TracTickets for help on using tickets.
Back to Top