Django

Code

Ticket #13068 (closed: fixed)

Opened 5 months ago

Last modified 4 months ago

The "Add another [inline object]" javascript doesn't respect prepopulated_fields settings

Reported by: hejsan Assigned to: seanbrant
Milestone: 1.2 Component: django.contrib.admin
Version: SVN Keywords:
Cc: hr.bjarni+django@gmail.com, brant.sean@gmail.com, gabrielhurley Triage Stage: Accepted
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

If I have an inline form with a prepopulated field, i.e:

class SomethingInline(admin.TabularInline):
    model = Something
    prepopulated_fields = {'title_slug': ('title',)}
    extra = 2

Then the extra inlines added by the new javascript dont automatically populate the title_slug field when entering a title.

BTW: The alternating row colors also dissappear for me in firefox when I click "Add another.."

Attachments

prepopulate.diff (6.0 kB) - added by seanbrant on 03/16/10 20:23:16.
prepopulate-with-9983-fix.diff (6.2 kB) - added by seanbrant on 03/16/10 21:32:37.
Also fixes #9983
models.py (1.2 kB) - added by carljm on 03/16/10 21:36:39.
models.py for easily testing this bug and associated ones
prepopulate-with-9983-and-9784-fix.diff (7.3 kB) - added by seanbrant on 03/16/10 22:09:14.
prepopulate-with-9983-and-9784-fix.2.diff (7.0 kB) - added by seanbrant on 03/16/10 22:15:20.
Removed some code that was not being used
prepopulate-with-9983-and-9784-fix.3.diff (7.2 kB) - added by seanbrant on 03/16/10 22:54:52.
Fixed adding the list as part of the css class hack
prepopulate.2.diff (12.3 kB) - added by seanbrant on 03/27/10 17:31:30.
Fixes this ticket as well as the other tickets mentioned.
prepopulate.3.diff (12.4 kB) - added by seanbrant on 03/27/10 17:54:31.
prepopulate.4.diff (12.4 kB) - added by seanbrant on 03/27/10 20:26:45.
Works with r12873
prepopulate.5.diff (12.5 kB) - added by seanbrant on 04/06/10 19:16:14.
Fixes luke's comments

Change History

03/09/10 22:02:33 changed by carljm

  • needs_better_patch changed.
  • needs_tests changed.
  • version changed from 1.2-beta to SVN.
  • milestone set to 1.2.
  • needs_docs changed.
  • stage changed from Unreviewed to Accepted.

Confirmed the prepopulated_fields issue.

On Firefox 3.5.8 I don't see any problem with the alternating row colors; in any case, if that actually is a problem in some browsers, it should get its own separate Trac issue.

03/12/10 09:07:40 changed by robhudson

I have a similar case that seems related. I suspect any widget in an inline that requires events to fire will fail here since the inline addition happens dynamically and a new DOM element is inserted but the events aren't added to the new DOM element.

03/13/10 09:47:43 changed by robhudson

An update. I added a slug field to an existing project to test this myself and also noticed that the HTML output also contains inline Javascript that looks like the following:

    document.getElementById("id_special_set-__prefix__-header_slug").onchange = function() { this._changed = true; };
    
    document.getElementById("id_special_set-__prefix__-header").onkeyup = function() {
        var e = document.getElementById("id_special_set-__prefix__-header_slug");
        if (!e._changed) { e.value = URLify(document.getElementById("id_special_set-__prefix__-header").value, 200); }
    }

03/16/10 19:07:49 changed by seanbrant

  • owner changed from nobody to seanbrant.
  • has_patch set to 1.

I added a new jQuery plugin called prepopulate, usage: $('input').prepopulate(jquery_object_containing_dependent_inputs, maxLength);. I reworked the code in prepopulate_fields.js.html to use this new plugin. I also fixed the bug this ticket is for, by hooking up the plugin to newly added rows. I have tested this, but with all js fixes it could use some more testing in whatever browser you are using.

03/16/10 19:18:03 changed by seanbrant

  • cc changed from hr.bjarni+django@gmail.com to hr.bjarni+django@gmail.com, brant.sean@gmail.com.

03/16/10 20:23:16 changed by seanbrant

  • attachment prepopulate.diff added.

03/16/10 21:19:05 changed by carljm

Confirmed that this patch does fixe the issue for tabular inlines. Stacked inlines are still broken; the easy solution is to copy-paste the new JS from tabular.html to stacked.html. That seems to have been done before with other JS, but it's nasty; we should find a way to avoid duplicating all that identical JS.

The current patch here also fixes #9264. It does not yet fix #9983 or #9784; I'm looking into how difficult that would be to add.

03/16/10 21:32:37 changed by seanbrant

  • attachment prepopulate-with-9983-fix.diff added.

Also fixes #9983

03/16/10 21:34:16 changed by seanbrant

Just attached a patch that fixes 9983. I will post a link to this ticket over on that ticket to keep those interested in the loop.

03/16/10 21:36:39 changed by carljm

  • attachment models.py added.

models.py for easily testing this bug and associated ones

03/16/10 21:39:39 changed by carljm

Since Django doesn't yet have an automated testing framework for Javascript, I'm attaching a models.py with the necessary fields and admin configuration to manually test this bug as well as #9264, #9983, and #9784. Just create an app with this models.py, add it to INSTALLED_APPS, syncdb, run the admin and add a Question. If all the referenced bugs were fixed, you'd see:

  • The slug1 field populated from both "name" and "pubdate" (including if pubdate is populated via the calendar picker)
  • The slug2 field populated from both "status" dropdown and "name"
  • The same behavior for both tabular and stacked inlines, including if you "add another"

All of my testing so far has been on FF 3.5.8 on Linux. Once the patch seems ready otherwise I can do cross-browser testing on Chrome Windows, Chrome Linux, FF 3.6 Windows, Opera 10 Windows, Safari 4.0 Windows, and IE 8 Windows (incl 7 compat mode).

03/16/10 22:09:14 changed by seanbrant

  • attachment prepopulate-with-9983-and-9784-fix.diff added.

03/16/10 22:15:20 changed by seanbrant

  • attachment prepopulate-with-9983-and-9784-fix.2.diff added.

Removed some code that was not being used

03/16/10 22:54:52 changed by seanbrant

  • attachment prepopulate-with-9983-and-9784-fix.3.diff added.

Fixed adding the list as part of the css class hack

03/16/10 23:03:31 changed by seanbrant

The only thing i'm not sure of is L100 of tabular.html. The problem is we need to get the target fields list of dependencies but the dynamic fields are added by cloning a hidden set of fields. This works but the current version of jQuery does not support cloning "data" attached to the DOM element. I choose to figure out the data by matching the template field biased on the target fields class names, then get the data from that. This still seems a little nasty, so I open for suggestions. I think one way to fix it would be to dig into inlines.js and add our own clone "data" feature. I held off for now not wanting this patch to get to large.

Also I have not messed with the stacked inline code yet, but that needs to be fixed up once we tighten down the current patch.

03/17/10 21:16:11 changed by carljm

  • needs_better_patch set to 1.

03/27/10 17:31:30 changed by seanbrant

  • attachment prepopulate.2.diff added.

Fixes this ticket as well as the other tickets mentioned.

03/27/10 17:34:54 changed by seanbrant

  • needs_better_patch deleted.

With this latest patch I modified inlines.js so that copying a DOM elements data attribute works.

03/27/10 17:54:31 changed by seanbrant

  • attachment prepopulate.3.diff added.

03/27/10 20:26:45 changed by seanbrant

  • attachment prepopulate.4.diff added.

Works with r12873

04/06/10 16:09:33 changed by lukeplant

This looks pretty good. Some comments:

  • 'console.log()' call stops it from working at all in Firefox.
  • For inlines that have been added via javascript, pre-populating from the drop down boxes is broken (whether I use keyboard or mouse). This is the same for Firefox (3.5.8 for Linux) and Chrome (5.0.342.7 beta for Linux).

If you could fix these, and then if someone can do the cross browser testing on Windows/Mac (like carljm offered), I will commit this. Many thanks.

04/06/10 16:38:49 changed by seanbrant

Thanks luke I can look at those issues tonight.

04/06/10 17:43:05 changed by gabrielhurley

  • cc changed from hr.bjarni+django@gmail.com, brant.sean@gmail.com to hr.bjarni+django@gmail.com, brant.sean@gmail.com, gabrielhurley.

I'm happy to help with cross-browser/cross-platform testing. I've got both Mac and Windows machines running most of the major browsers. I'll cc myself here so I know when the new patch goes up.

04/06/10 19:16:14 changed by seanbrant

  • attachment prepopulate.5.diff added.

Fixes luke's comments

04/07/10 18:18:09 changed by lukeplant

OK, I've been able to confirm the latest patch fixes #13068, #9264, #9983 and #9784 on Chrome 5.0.342.7 beta (Linux), Firefox 3.5.8 (Linux), Opera 10.10 (Linux), Internet Explorer 7, Safari 4.0.4 (Windows). On that basis I doubt there will be problems for IE 8 or Mac browsers, so I will go ahead and commit, but testing on those platforms is welcome.

Many thanks Sean, Carl and Rob for the work on this ticket, I know that javascript can take ages to debug! Thanks also for making it easy to test out.

04/07/10 18:37:49 changed by lukeplant

  • status changed from new to closed.
  • resolution set to fixed.

(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.

04/07/10 20:03:14 changed by gabrielhurley

I was writing this up when the patch got merged, but here's my testing report:

Patch tested successfully on the following platforms & browsers:

Mac OS X:

  • Safari 4
  • Firefox 3.6
  • Chrome 4 (stable channel)

Windows XP:

  • Internet Explorer 8
  • Chrome 4 (developer channel)
  • Firefox 3.6
  • Safari 4

Windows Vista:

  • Internet Explorer 7
  • Chrome 4 (stable)

Add/Change #13068 (The "Add another [inline object]" javascript doesn't respect prepopulated_fields settings)




Change Properties
Action