Opened 14 years ago

Closed 13 years ago

Last modified 13 years ago

#13674 closed (fixed)

Javascript for adding inline fields doesn't update "for" attribute in label

Reported by: Jonas von Poser Owned by: Tay Ray Chuan
Component: contrib.admin Version: 1.2
Severity: Keywords: admin
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I do some checking for required fields using Javascript in the admin. Doing that, I noticed that adding inline fields doesn't update the "for" attribute for the new <label> fields. If the field is called "image", the newly added field label looks like this:

<label class="required" for="id_images-__prefix__-image">Image:</label>

This is the patch that solved it for me:

diff --git a/django/contrib/admin/media/js/inlines.js b/django/contrib/admin/media/js/inlines.js
index cf79023..90b50b5 100644
--- a/django/contrib/admin/media/js/inlines.js
+++ b/django/contrib/admin/media/js/inlines.js
@@ -75,6 +75,14 @@
                                    }).each(function() {
                                        var el = $(this);
                                        el.attr("name", el.attr("name").replace(/__prefix__/g, nextIndex));
+                                   })
+                                   .end()
+                                   .filter(function() {
+                                       var el = $(this);
+                                       return el.attr("for") && el.attr("for").search(/__prefix__/) >= 0;
+                                   }).each(function() {
+                                       var el = $(this);
+                                       el.attr("for", el.attr("for").replace(/__prefix__/g, nextIndex));
                                    });
                                if (row.is("tr")) {
                                        // If the forms are laid out in table rows, insert

Attachments (2)

0001-fix-updating-of-the-for-attribute-in-new-admin-inlin.patch (3.8 KB ) - added by Tay Ray Chuan 14 years ago.
13674_inlines_label_ids.diff (6.6 KB ) - added by Julien Phalip 13 years ago.

Download all attachments as: .zip

Change History (13)

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

Component: Uncategorizeddjango.contrib.admin
milestone: 1.3
Triage Stage: UnreviewedAccepted

in reply to:  description comment:2 by Tay Ray Chuan, 14 years ago

Replying to Jonas:

This is the patch that solved it for me:

diff --git a/django/contrib/admin/media/js/inlines.js b/django/contrib/admin/media/js/inlines.js
index cf79023..90b50b5 100644
--- a/django/contrib/admin/media/js/inlines.js
+++ b/django/contrib/admin/media/js/inlines.js
@@ -75,6 +75,14 @@
                                    }).each(function() {
                                        var el = $(this);
                                        el.attr("name", el.attr("name").replace(/__prefix__/g, nextIndex));
+                                   })
+                                   .end()
+                                   .filter(function() {
+                                       var el = $(this);
+                                       return el.attr("for") && el.attr("for").search(/__prefix__/) >= 0;
+                                   }).each(function() {
+                                       var el = $(this);
+                                       el.attr("for", el.attr("for").replace(/__prefix__/g, nextIndex));
                                    });
                                if (row.is("tr")) {
                                        // If the forms are laid out in table rows, insert

You should be modifying updateElementIndex() instead - it will be much cleaner.

diff --git a/django/contrib/admin/media/js/inlines.js b/django/contrib/admin/media/js/inlines.js
index cf79023..21e00ad 100644
--- a/django/contrib/admin/media/js/inlines.js
+++ b/django/contrib/admin/media/js/inlines.js
@@ -18,7 +18,7 @@
        $.fn.formset = function(opts) {
                var options = $.extend({}, $.fn.formset.defaults, opts);
                var updateElementIndex = function(el, prefix, ndx) {
-                       var id_regex = new RegExp("(" + prefix + "-\\d+)");
+                       var id_regex = new RegExp(prefix + "-__prefix__");
                        var replacement = prefix + "-" + ndx;
                        if ($(el).attr("for")) {
                                $(el).attr("for", $(el).attr("for").replace(id_regex, replacement));

comment:3 by Tay Ray Chuan, 14 years ago

Attached a patch for both the full and minified JS files.

Note that apart from changing the regex to search for __prefix___ instead of a number (\d+), the capturing parentheses () has also been removed, since we don't need the matched result.

comment:4 by Tay Ray Chuan, 14 years ago

Owner: changed from nobody to Tay Ray Chuan

comment:5 by Ramiro Morales, 13 years ago

#15106 was a duplicate and contained a pattch similar to the fix suggested by the OP of this ticket.

comment:6 by Julien Phalip, 13 years ago

Triage Stage: AcceptedReady for checkin

The patch applies and fixes this bug. See also #14303 for a related (but different) bug.

comment:7 by Russell Keith-Magee, 13 years ago

Patch needs improvement: set
Triage Stage: Ready for checkinAccepted

Patch applies and fixes the bug, but introduces a different bug -newly created inlines don't validate.

To reproduce:

  • Use a parent model with a TabularInline child, with extra=3
  • apply patch 0001
  • Add a new parent object
  • Add 1 extra inline row
  • Populate all 4 inline objects (the 3 default provided, plus the 1 added inline).
  • Try to save the object

The object will fail validation, and lose the data for the fourth inline. If you re-enter and save, the data is saved successfully.

comment:8 by Julien Phalip, 13 years ago

Triage Stage: AcceptedReady for checkin

Thanks for spotting this issue, Russ - and sorry for having missed it. I've attached a patch which should fix the root problem. Taking the liberty to RFC again so it gets on your radar for the 1.3 home stretch.

by Julien Phalip, 13 years ago

comment:9 by Russell Keith-Magee, 13 years ago

Resolution: fixed
Status: newclosed

In [15436]:

Fixed #13674 -- Ensure that labels on added inlines fields have the right 'for' attribute. Thanks to Jonas for the report, and Julien Phalip for the patch.

comment:10 by Russell Keith-Magee, 13 years ago

In [15437]:

(The changeset message doesn't reference this ticket)

comment:11 by Jacob, 13 years ago

milestone: 1.3

Milestone 1.3 deleted

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