Opened 8 years ago

Closed 6 years ago

Last modified 5 years ago

#9264 closed (fixed)

ModelAdmin: multipe items in prepopulated_fields breaks the auto-fillin

Reported by: pickelman@… Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords:
Cc: dgouldin@… Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In an admin.ModelAdmin class, this will pre-populate the 'slug' field on-the-fly in the admin:

prepopulated_fields = { 'slug': ('name',) }

However this will cause neither to be pre-populated on-the-fly:

prepopulated_fields = { 'slug': ('name',), 'another_slug': ('name2',) }

Attachments (1)

9264.patch (1.2 KB) - added by Peter Bengtsson 8 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 8 years ago by Jacob

milestone: 1.1
Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

comment:2 Changed 8 years ago by David Gouldin

Cc: dgouldin@… added

I was not able to reproduce this. Can anybody else?

comment:3 Changed 8 years ago by Peter Bengtsson

Resolution: fixed
Status: newclosed

I have reviewed this too.

No, it's not reproducible. It does work perfectly well to have multiple slug fields like this:

class FooModelAdmin(admin.ModelAdmin):
    prepopulated_fields = {'slug': ('name',),
                           'slug2': ('name2',) }

BUT! If you have multiple slug fields that expect to use the same fields you have a problem. For example:

class FooModelAdmin(admin.ModelAdmin):
    prepopulated_fields = {'slug': ('name',),
                           'slug2': ('name',)
                          }

What happens is effectively this:

... 
    document.getElementById("id_name").onkeyup = function() {
        var e = document.getElementById("id_slug");
        ...
    }

    document.getElementById("id_name").onkeyup = function() {
        var e = document.getElementById("id_slug2");
        ...
    }
...

Which is fundamentally wrong since it overwrites the keyup event on the same element.

So attached is a patch to prepopulated_fields_js.html which use addEvent(my_dom_obj, 'keyup', function(){}) rather that my_dom_obj.onkeyup=function(){}

Changed 8 years ago by Peter Bengtsson

Attachment: 9264.patch added

comment:4 Changed 8 years ago by Peter Bengtsson

Patch tested in Firefox and Opera only.
IE anyone?

comment:5 Changed 8 years ago by Karen Tracey

Resolution: fixed
Status: closedreopened

If there's a patch to be tested, it doesn't sound like this is fixed?

comment:6 Changed 7 years ago by Kevin Kubasik

This patch will conflict with the js patch in #9983, however. The patch there has been updated to include this fix.

comment:7 Changed 7 years ago by Florian Apolloner

Has patch: set

comment:8 Changed 7 years ago by Jacob

milestone: 1.11.2

Pushing to 1.2 for the same reason as #9983.

comment:9 Changed 7 years ago by Jacob

Triage Stage: AcceptedReady for checkin

This really should be updated to use jQuery now, but it's not a big deal. We can commit it and clean it up later if needed.

comment:10 Changed 7 years ago by James Bennett

milestone: 1.21.3

I'm going to bump this off the 1.2 milestone, for a couple reasons:

  1. There are several tickets (this one, #9110, #9784 and #9983) all exposing cases where the event handling for prepopulated_fields falls over in specific edge cases, so the real solution is not to fix any of these individually but rather to make prepopulate_from more robust.
  2. I suspect the admin-ui work will be coming up for discussion in the 1.3 timeframe, and that's a more solid place to talk about dealing with this.

comment:11 Changed 7 years ago by Carl Meyer

#7985 was a duplicate of this, and also had a patch.

comment:12 Changed 6 years ago by Luke Plant

Resolution: fixed
Status: reopenedclosed

(In [12937]) Fixed #13068, #9264, #9983, #9784 - regression with pre-populated fields and javascript inlines, and related bugs.

Thanks to hejsan for the report, and to Sean Brant and Carl Meyer for the patch.

#13068 is a regression caused by the new javascript inline forms in the
admin. The others were existing javascript bugs with prepopulated fields.
Since the solution depends on jQuery and would likely be very hard to
backport without it, it will not be backported to 1.1.X.

comment:12 Changed 5 years ago by Jacob

milestone: 1.3

Milestone 1.3 deleted

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