Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#14830 closed (fixed)

Default value for radio button not preserved when dynamically creating new inlines

Reported by: Julien Phalip Owned by: nobody
Component: contrib.admin Version: 1.2
Severity: Keywords: sprintdec2010
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Here is a simple test case illustrating the problem:

Models:

    from django.db import models

    class MyModel(models.Model):
        name = models.CharField(max_length=100, blank=True)
    
    CHOICES = (
        (u'1', u'blue'),
        (u'2', u'red'),
    )
    
    class RelatedModel(models.Model):
        MyModel = models.ForeignKey(MyModel)
        color = models.CharField(max_length=1, choices=CHOICES, default='1')

Admin:

    from django.contrib import admin
    from .models import MyModel, RelatedModel
    
    class RelatedModelInline(admin.TabularInline):
        model = RelatedModel
        radio_fields = {"color": admin.HORIZONTAL}
    
    class MyModelAdmin(admin.ModelAdmin):
        inlines = [RelatedModelInline]
    
    admin.site.register(MyModel, MyModelAdmin)

The problem is that after clicking "Add another Related Model" more than once in the admin, the default value for the radio button (i.e. 'blue' checked) is not preserved. What's happening is that when the hidden inline form template is cloned the first time, it loses that default value. The first clone does get the default value, but every subsequent clone doesn't have it because the template has lost it during the first cloning.

I narrowed it down to the following lines in inlines.js:

    var template = $("#" + options.prefix + "-empty");
    var row = template.clone(true);
    row.removeClass(options.emptyCssClass)
        .addClass(options.formCssClass)
        .attr("id", options.prefix + "-" + nextIndex)
        .insertBefore($(template));

As soon as the first clone is inserted into the DOM, the template loses the default value. Really weird. Now what's even weirder is that the following simple test case does work, however:

    <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
    <head>
        <style>
            .empty-form {
                display:none;
            }
        </style>
    <title></title>
    <script type="text/javascript" src="jquery.js"></script>
    </head>
    <body>    
    
    <form id="original-form" class="empty-form" action="" method="post">
        <input type="radio" name="radio-test" value="I'm a checked radio button" checked="checked" />
        <input type="radio" name="radio-test" value="I'm another checked radio button" />
    </form>
    
    <a href="#" alt="" id="clone-form">Clone</a>
    
    <script type="text/javascript" language="Javascript">
        $(document).ready(function() {
            var counter = 0;
            $('#clone-form').click(function(){
                counter += 1;
                var template = $('#original-form');
                var row = template.clone(true);
                row.removeClass('empty-form')
    				    .addClass('cloned-form')
    				    .attr("id", 'clone-' + counter)
    				    .insertBefore($(template));
            });
        });
    </script>
    
    </body>
    </html>

The code above tries to match the Django inline jquery business as close as possible (e.g. using same jquery.js), but the cloning here is working perfectly... So my guess is that there must be some strange side effects happening in inlines.js or in other javascript code loaded in the admin.

A related issue has been wontfixed for jquery itself: http://bugs.jquery.com/ticket/3879
The issue observed here is slightly different though, as it is the cloned object that loses the default value, not the clonee.

Anyway, it's hard to figure out whether it is a Django or jQuery issue at this stage.

Attachments (1)

14830_inlines_radio_buttons_r15048.diff (6.1 KB ) - added by Julien Phalip 13 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 by Julien Phalip, 13 years ago

milestone: 1.3

Marking 1.3 as recommended by DrMeers on IRC, since this is a bug.

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

Triage Stage: UnreviewedAccepted

I can validate that I've seen this with my own two eyes; but I'm not sure how to fix it either.

comment:3 by antoinemartin, 13 years ago

The issue comes from the fact that when the row is inserted, the radio elements have the name as in the template. As only one radio element with the same name can be checked, the browser "unchecks" the radio in the template.

To fix this, simply move the addition of the new form in the document DOM after having changed the names and ids in the template:

         row.removeClass(options.emptyCssClass)
	      .addClass(options.formCssClass)
	      .attr("id", options.prefix + "-" + nextIndex)
          // Remove insertion from here
	  //    .insertBefore($(template));

...

        row.find("input,select,textarea,label,a").each(function() {
		updateElementIndex(this, options.prefix, totalForms.val());
	});
        // Insert the new form when it has been fully edited
	row.insertBefore($(template));
	// Update number of total forms
	$(totalForms).val(nextIndex + 1);

comment:4 by Julien Phalip, 13 years ago

Has patch: set

Well spotted, antoinemartin! Your suggestion fixes the problem, thanks! Patch attached.

by Julien Phalip, 13 years ago

comment:5 by Julien Phalip, 13 years ago

Triage Stage: AcceptedReady for checkin

Even though this is not recommended practice, I'm taking the liberty to RFC my own patch to be sure this annoying bug grabs the attention of core devs before 1.3 ships ;-)

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

Resolution: fixed
Status: newclosed

In [15420]:

Fixed #14830 -- Ensure that radio buttons on inlines preserve their default value. Thanks to Julien Phalip for the report and patch, and to antoinemartin for the diagnosis.

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

In [15421]:

[1.2.X] Fixed #14830 -- Ensure that radio buttons on inlines preserve their default value. Thanks to Julien Phalip for the report and patch, and to antoinemartin for the diagnosis.

Backport of r15420 from trunk.

comment:8 by Jacob, 13 years ago

milestone: 1.3

Milestone 1.3 deleted

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