Labels missing for tags in newforms widgets

[Russell Keith-Magee]

The CheckboxSelectMultiple and RadioInput widgets in newforms add a <label> tag around their multiple-choice options, but don't specify a for="". This apparently causes problems with IE6/7.

[Malcolm Tredinnick]

More information garnered from the mailing list thread: the problem is that without these attributes, clicking on the label text does not activate the control.

[sandro]

All of the generated labels looked good to me, here's how I tested.

PRIVACY_CHOICES = (
    ('Y', 'YES'),
    ('N', 'NO'),
)
class SurveyForm(forms.Form):
    name = forms.CharField(max_length=20)
    likes_a = forms.BooleanField()
    likes_b = forms.BooleanField(widget=forms.CheckboxSelectMultiple(choices=PRIVACY_CHOICES))
    is_private = forms.BooleanField(widget=forms.RadioSelect(choices=PRIVACY_CHOICES))

[sandro]

Just ran into some test data to validate the issue:

CHOICES=[(1L, u'Marshall'), (2L, u'Miami'), (3L, u'East Carolina'), (4L, u'Virginia Tech'), (5L, u'Connecticut')]

class TeamSelectForm(forms.Form):
    teams = forms.MultipleChoiceField(choices=CHOICES, widget=forms.CheckboxSelectMultiple)


<p><label for="id_teams_0">Teams:</label> <ul>
<li><label><input type="checkbox" name="teams" value="1" id="id_teams_0" /> Marshall</label></li>
<li><label><input type="checkbox" name="teams" value="2" id="id_teams_1" /> Miami</label></li>
<li><label><input type="checkbox" name="teams" value="3" id="id_teams_2" /> East Carolina</label></li>
...
</ul></p>

Each list item should instead look like:

<li><input type="checkbox" name="teams" value="1" id="id_teams_0" /><label for="id_teams_0"> Marshall</label></li>

[sandro]

Added a quick fix for the data above. It might be a good idea to look for other label typos.

[sandro]

Added some tests, I'm sure there are other label inconsistencies but I wanted to post this progress as soon as I could. All of the attachments are duplicates can anyone clean them up?

[sandro]

Whoops, didn't mean to close the ticket...looks like I don't know how to use trac properly.

[Ivan Sagalaev <Maniac@…>]

Looks like this patch removes all label tags from radio and checkbox inputs altogether. I believe it's better to just write proper for="" for them because they are useful there. Patch follows.

P.S. I'm setting 'Design decision needed' because may be I'm missing something and labels are not desirable in RadioInput and CheckboxSelectMultiple. Though I can't imagine why...

[Ivan Sagalaev <Maniac@…>]

Patch with labels and proper fors

[sandro]

Glad to see some activity! All of the tests for my patch succeeded so I'm unsure of how it removed label tags for unrelated widgets. On another note though, my patch sucks as it maps the for attribute to the related value attribute instead of the related id attribute...agh!

Maybe we could get a css expert to weigh in on the correct usage of the label tag. ​webstandards.org talks a bit about them:

The article says we need to turn this:

<li><label for="id_language_0"><input type="radio" id="id_language_0" value="P" name="language" /> Python</label></li>

Into this:

<li><input type="radio" id="id_language_0" value="P" name="language" /> <label for="id_language_0">Python</label></li>

[anonymous]

All of the tests for my patch succeeded so I'm unsure of how it removed label tags for unrelated widgets.

Ah... Looks like I've misread your patch then. I didn't see that you've extracted inputs out of labels, I thought your removed labels completely. Sorry :-)

Maybe we could get a css expert to weigh in on the correct usage of the label tag.

Well.. According to HTML spec both usages are correct. When you wrap input inside a label you theoretically can skip for=".." and label will automatically relate to an enclosed input. However IE doesn't understand labels without for=".." which this whole bug is about :-).

From styling point of view it's hard to say which is better: sometimes one variant is easier to layout according to design, sometimes the other. I've left inputs inside labels simply for consistency with other Django widgets.

[sandro]

Well done Ivan, your patch fixes the problem and stays consistent with Django widgets. I've started a discussion on the google group to try and gain some insight as to which label style Django should support...input is welcome.

​http://groups.google.com/group/django-developers/browse_thread/thread/60ae465f9c453f89

[Maniac <Maniac@…>]

I'm inclined to move this ticket out of "Design decision needed" because both contributors agree on the latest patch. So it just has to be reviewed now.

[Batiste Bieler]

It a simple and annoying bug. Please do something about it.

[batiste@…]

Patch for labels in new forms. Add a "for" attribute when possible

[batiste@…]

$ ./runtests.py --settings t/settings forms
Ran 28 tests in 47.179s
OK

$ ./runtests.py --settings t/settings
Ran 257 tests in 177.968s
OK

[Jacob]

Fixed in [7693].

[Jacob]

Milestone 1.0 deleted