Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#14830 closed (fixed)

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

Reported by: julien 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: UI/UX:

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

Download all attachments as: .zip

Change History (9)

comment:1 Changed 4 years ago by julien

  • milestone set to 1.3
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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

comment:2 Changed 4 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

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 Changed 4 years ago by antoinemartin

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 Changed 4 years ago by julien

  • Has patch set

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

Changed 4 years ago by julien

comment:5 Changed 4 years ago by julien

  • Triage Stage changed from Accepted to Ready 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 Changed 4 years ago by russellm

  • Resolution set to fixed
  • Status changed from new to closed

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 Changed 4 years ago by russellm

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 Changed 4 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

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