Opened 14 years ago

Closed 14 years ago

Last modified 13 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: dev
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

Description

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 14 years ago.

Download all attachments as: .zip

Change History (9)

by 3point2, 14 years ago

Attachment: inline-counters.diff added

comment:1 by Russell Keith-Magee, 14 years ago

milestone: 1.2
Triage Stage: UnreviewedAccepted

comment:2 by Russell Keith-Magee, 14 years ago

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 by Russell Keith-Magee, 14 years ago

@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 by Jannis Leidel, 14 years ago

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 by 3point2, 14 years ago

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 by Russell Keith-Magee, 14 years ago

@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 by Russell Keith-Magee, 14 years ago

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 by Jacob, 13 years ago

milestone: 1.2

Milestone 1.2 deleted

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