Opened 9 years ago

Closed 9 years ago

Last modified 7 years ago

#13459 closed (fixed)

nonzero extra inlines and "add another" create duplicate <tr> ids

Reported by: 3point2 Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


The first new row of an admin inline created by the "add another" button has the same id as the immediately preceding "extra" row.

An admin model that contains an inline with the "extra" option set to a non zero value creates <tr>s with id suffixes starting from 1. When the JavaScript "add another" function is called, it creates id suffixes assuming that the first row id was numbered 0.

This may have been a side effect of fixing #13175. Using forloop.counter0 in the inline templates when generating the extra inlines solves the problem.

Attachments (1)

inline-counters.diff (2.6 KB) - added by 3point2 9 years ago.

Download all attachments as: .zip

Change History (9)

Changed 9 years ago by 3point2

Attachment: inline-counters.diff added

comment:1 Changed 9 years ago by Russell Keith-Magee

milestone: 1.2
Triage Stage: UnreviewedAccepted

comment:2 Changed 9 years ago by Russell Keith-Magee

Resolution: fixed
Status: newclosed

(In [13068]) Fixed #13459 -- Corrected ID numbering of dynamically added inlines in the admin. Thanks to 3point2 for the report.

comment:3 Changed 9 years ago by Russell Keith-Magee

@3point2 - Thanks for the patch, but in this case the right solution is to correct the numbers allocated to new rows, not to modify the numbering of the static rows to suit the allocation scheme in the dynamic portion.

comment:4 Changed 9 years ago by Jannis Leidel

Hm, maby I've looked at this code too long, but I thought I've fixed this in r12871. Are you sure this fixes the issue and also doesn't conflict with the actual ids of the rows?

comment:5 Changed 9 years ago by 3point2

Resolution: fixed
Status: closedreopened

jezdez: this bug existed in r13063. the fix in r12871 did solve the problem in #13175, but only for rows created by javascript. the numbering for static rows created by "extra" still started at 1, which a) disagreed with the final_attrs id, b) created a duplicate id conflict with the javascript rows. i agree with you that russellm's fix conflicts with the actual ids of the rows in final_attrs.

russellm: the patch does solve the duplicate ID problem, but also re-opens #13175. given that widget id's start at 0, wouldn't it make sense for row ids to also be zero based? (as in my patch)

ideally, the id would come from the formset itself, not forloop.counter. this would ensure that the formset's id would match those of the widgets it contains. the only attribute i could find that seems like it might be of use is {{ inline_admin_form.formset.auto_id }}, but that is set to "id_%s". i'm not sure that the right approach to fix this is.

comment:6 Changed 9 years ago by Russell Keith-Magee

@3point2: My apologies, you are correct. For some reason I convinced myself that the indicies for widget IDs in a formset were 1-based, not 0-based. Let this be a lesson to us all that cold+flu meds and code don't mix :-)

comment:7 Changed 9 years ago by Russell Keith-Magee

Resolution: fixed
Status: reopenedclosed

(In [13083]) Fixed #13459 -- Corrected numbering of tr elements in the admin. Reverts the changes introduced in r13068. Thanks to 3point2 for the report and patch.

comment:8 Changed 7 years ago by Jacob

milestone: 1.2

Milestone 1.2 deleted

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