jquery gets included multiple times

[mariarchi]

ModelAdmin adds jquery.min.js to the self.media if self.actions is not False.
However, InlineModelAdmin by default includes jquery.min.js as well.

So given a model with N inlines by default we end up with N+1 jqueries.

It wouldn't be that bad, however, what makes it really bad is the fact that if the main form specifies a jquery plugin in the media definition it will be overwritten by vanilla redundant jquery.

The simplest fix would be to implement internal storage in Media object as a set which would remove redundant entries.

[sebleier@…]

Correct me if I'm wrong, but I thought there was already a mechanism to limit dupes in the admin media?

​http://code.djangoproject.com/browser/django/trunk/django/forms/widgets.py#L76

[Tobias McNulty]

Unable to reproduce. If this is still an issue for you, please provide your forms, models, and admin file or, better yet, a unit test demonstrating the issue.

[mariarchi]

Ooops, I was wrong, sorry.

However, if one will do that -

class MovieAdmin(admin.ModelAdmin):
    class Media:
        js = ("/media/js/jquery.min.js",
              "media/js/jquery.min.js",
              "media/js/jquery.min.js")

jquery will get included three times. Furthermore, if one codes up a class like

class MovieAdmin(admin.ModelAdmin):
    class Media:
        js = ("/admin_media/js/jquery.min.js",
              "/admin_media/js/jquery.min.js",
              "/admin_media/js/jquery.min.js")

none of this declarations will make it through, if jquery already got in the media object. Or all three will appear, if it wasn't there yet.

I think that this behavior is both inconsistent and wrong.

The main use case is that AFAIK the current way of creating pluggable admin/form features is to provide mixins. So,
assuming we have mixins MixinA, MixinB and MixinC then our admin class will probably look like

class MyAdmin(admin.ModelAdmin, MixinA, MixinB, MixinC):
    class Media:
        js = MixinA.Media.js + MixinB.Media.js + MixinC.Media.js

and guess what happens if all three use jquery.

[James Bennett]

Patch + test case

[James Bennett]

The root issue is that JS is added in a list comprehension, which generates the list of JS file paths to add before adding any of them; thus the same file path can be added multiple times by a single declaration. Patch attached above resolves this by using a loop which checks after adding each path, and also includes a regression test case which demonstrates the problem and passes after the fix is applied.

[James Bennett]

(In [12663]) Fixed #12879: Declaring the same JS file multiple times in a single Media instance now only includes that file once.

[James Bennett]

(In [12664]) [1.1.X] Fixed #12879: Declaring the same JS file multiple times in a single Media instance now only includes that file once. Backport of [12663] from trunk.

[James Bennett]

Note that the fix applied does not deal with the issue of specifying the same file once with a relative path and once with an absolute path; as far as I'm concerned, that's expected behavior for the moment, but maybe down the road we'll come up with some clever normalization trick to keep it from happening.

[James Bennett]

(In [12666]) [1.1.X] Expanded the fix in [12663] to cover CSS declarations, which were also affected by the bug. Refs #12879. Backport of [12665] from trunk.

[Jacob]

Milestone 1.2 deleted