CheckboxInput should render 'False' value as unchecked

[Bob Thomas]

Currently, BooleanField will accept a value of 'False' and convert it to a boolean False in cleaned_data, for the case where it accepts data from a hidden field. However, if it is rendered as a CheckboxInput widget again, it will be rendered as checked, as the string 'False' from the initial data is converted to a True value.

Failed example:
    w.render('is_cool', 'False')
Expected:
    u'<input type="checkbox" name="is_cool" />'
Got:
    u'<input checked="checked" type="checkbox" name="is_cool" value="False" />'

This was happening to me in a strange case of a form validating for the preview stage of FormPreview, but failing validation after the POST, but there are probably other cases where someone might want to render a hidden value of 'False' into a CheckboxInput.

[Bob Thomas]

Accept string versions of True and False, with regression tests

[Malcolm Tredinnick]

I'd prefer to fix it so that nothing in Django is passing such a string into widgets. Checkbox widgets take booleans. It's not hard to pass in the right type. We should take advantage of Python's strong typing in this situation, not avoid it.

So I'm -1 on this change, preferring to fix the root problem instead.

[Bob Thomas]

It seems like CheckboxInput has always received a string, not a boolean, as its value input. A checked box submits a value of "on", and bool("on") is True. An unchecked box submits nothing, thus False. You can test this with a form containing a checkbox. If the checkbox is selected, and the form raises a ValidationError, the checkbox is rendered with a value of "on" like so:

<input checked="checked" type="checkbox" name="is_cool" value="on" />

but it should just be:

<input checked="checked" type="checkbox" name="is_cool" />

I think I agree with you that the data should be cleaned before being passed to render(), though I'm not sure of the best way to do it. Calling the field's clean() method won't work for invalid data. Something else needs to happen to the data in BoundField.as_widget before being passed to widget.render()

[Bob Thomas]

The patch on #9473 is following the same approach here. I don't think there's any reason to not fix this the way I've implemented it. If someone wants to re-implement field validation so that data is always sanitized separately, that seems more like a feature or improvement.

[Russell Keith-Magee]

I may be missing something here, but I concur with Malcolm. This isn't the same fix as #9473 - that patch deals with the value_from_datadict function, which needs to be liberal on what it accepts based on the various widgets that could be used to render booleans. This patch deals with the init and render methods, which should both be dealing with cleansed data that we can reliably know to be boolean.

@bthomas: your original report indicated that this was a problem with FormPreview. If you could provide an example of how this problem manifests, we could work out if this is a problem with FormPreview, or if there is some other problem at work here.

[Bob Thomas]

Sanitize input correctly in value_from_datadict

[Bob Thomas]

Replying to russellm:

I may be missing something here, but I concur with Malcolm. This isn't the same fix as #9473 - that patch deals with the value_from_datadict function, which needs to be liberal on what it accepts based on the various widgets that could be used to render booleans. This patch deals with the init and render methods, which should both be dealing with cleansed data that we can reliably know to be boolean.

Quite correct, of course. I've attached a patch that fixes value_from_datadict instead

[jkocherhans]

(In [12556]) Fixed #9336. Changed CheckboxInput to render 'True' and 'False' input strings as checked or not instead of as a value attribute. Thanks, bthomas.

[jkocherhans]

(In [12557]) [1.1.X] Fixed #9336. Changed CheckboxInput to render 'True' and 'False' input strings as checked or not instead of as a value attribute. Backport of r12556 from trunk.

[Jacob]

Milestone 1.2 deleted