Opened 7 years ago

Closed 5 years ago

Last modified 4 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 peterbe 6 years ago.

Download all attachments as: .zip

Change History (14)

comment:1 Changed 7 years ago by jacob

  • milestone set to 1.1
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 6 years ago by dgouldin

  • Cc dgouldin@… added

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

comment:3 Changed 6 years ago by peterbe

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

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 6 years ago by peterbe

comment:4 Changed 6 years ago by peterbe

Patch tested in Firefox and Opera only.
IE anyone?

comment:5 Changed 6 years ago by kmtracey

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

comment:6 Changed 6 years ago by kkubasik

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

comment:7 Changed 6 years ago by apollo13

  • Has patch set

comment:8 Changed 6 years ago by jacob

  • milestone changed from 1.1 to 1.2

Pushing to 1.2 for the same reason as #9983.

comment:9 Changed 6 years ago by jacob

  • Triage Stage changed from Accepted to Ready 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 6 years ago by ubernostrum

  • milestone changed from 1.2 to 1.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 5 years ago by carljm

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

comment:12 Changed 5 years ago by lukeplant

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

(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 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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