14887 - bugzilla should use <label> tag in forms

Status: RESOLVED FIXED

(Bugzilla :: User Interface, enhancement, P2)

Description

[Michael Lowe]

Should put <label></label> tags around all checkbox and radio button
control + labels pairs, so it is possible to click on a control label to select
the control.  eg.

<label><input type=checkbox>Here is a control label</label>

Comment 1

[Terry Weissman]

What browsers support this?  Communicator 4.61 on Linux doesn't seem to...

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Erm... Mozilla!  (Some of us do actually use it some of the time.  It works :-)

Comment 3

[Terry Weissman]

Well, fine and good.  But I don't use it yet, and I'm not going to start just so
that I can fix this bug, and I'm not going to try and fix this bug without being
able to test my fixes.

So, even though you guys have voted this bug up real high, I won't work on it
now.  But I'll be happy to look over any patches you want to submit!

Comment 4

[Jesse Ruderman]

Changing description to be more clear

Comment 5

[Terry Weissman]

[e-mail] is the new owner of Bugzilla and Bonsai.  (For details,
see my posting in netscape.public.mozilla.webtools,
news://news.mozilla.org/38F5D90D.F40E8C1A%40geocast.com .)

Comment 6

[Zach Lipton [:zach]]

I find this bug important and I am willing to take it on. Assigning to self.

Comment 7

[Dave Miller [:justdave]]

Since there was question of it further up in the comments here, yes, this is 
defined in the HTML 4.01 specification.

http://www.w3.org/TR/html401/interact/forms.html#h-17.9

So I'd say go ahead and do it.

Comment 8

[Zach Lipton [:zach]]

Attachment: Patch for query.cgi to send to landfill for testing

Comment 9

[Zach Lipton [:zach]]

Somehow, I assaigned this bug to someone else. Reassigning to self.

Comment 10

[Zach Lipton [:zach]]

again! Bugzilla now is setting this to new and filling my email with whiney 
notes. I talked to baloo about this yesterday in #mozwebtools, the patch I 
posted isn't going to work on netscape for linux, but I might be able to add 
some JS to do browser detection (actully, I wouldn't need JS, I could use 
cgi (wow, what a thought) and only show the <label> tags on mozilla. 

I won't be able to work on this now, but I might have some time in a few 
weeks, so I'll accept the bug since it is clearly something important to the 
people who voted it up to 16 votes!

Comment 11

[Dave Miller [:justdave]]

I would just exclude it on Netscape for Linux, instead of excluding everything 
except Mozilla.  It is part of the HTML 4.01 specification, which means any 
browser that's following the standards should honor it.

Comment 12

[Jesse Ruderman]

what does netscape for linux do when you feed it an html page with <label>s?

Comment 13

[Zach Lipton [:zach]]

Attachment: New patch for the bug form to send to landfill for testing. This one uses a two new subs!

Comment 14

[Zach Lipton [:zach]]

Ok everyone, I just posted another patch. This one uses two subs in 
CGI.pl to do the regexp checking of the user agent and print the label tags. 
This new patch patches the bug_form.pl file to use the subs at the 
approate places. After this is put on landfill and deemed working (it might 
take a few tries), I will make this patch extend to other forms and get it 
checked in.

Comment 15

[Dave Miller [:justdave]]

The main page on landfill says this is being tested there (didn't see anything 
in this bug report to say it had been posted on landfill).

I can verify that the <label> tags are appearing in the source, and that it does 
NOT work on Navigator 4.6 (Windows).  Doesn't break it either, though, so no 
need to remove them for that user agent.

Comment 16

[Zach Lipton [:zach]]

I don't have access to windows to test this, but are the label tags in the 
source for anything but mozilla (it shouldn't)

Comment 17

[Dave Miller [:justdave]]

They are in the source for Netscape 4.6 (Windows).  Netscape doesn't honor them, 
but it doesn't break it, either.  It should be included in the source unless we 
know it breaks a browser, because it is part of the standard, and the standard 
was written that browsers are supposed to just ignore tags they don't 
understand.

Comment 18

[Zach Lipton [:zach]]

great! Looks like everything is working fine with this. I'll finish this up by 
doing this for some other fields and all should be well.

Comment 19

[Gervase Markham [:gerv]]

Zach - what's the status of this patch?

Gerv

Comment 20

[Zach Lipton [:zach]]

The status of this patch is several things.

1. the patch that is attached to the bug is broken. I forgot some ;'s and 
things were majorly broken. Tara fixed it on landfill. 

2. I am low on time and have not gotten around to finishing it. In the mean 
time, the fixed version of the patch works fine and it should be checkedin. I 
need to get around to writing a new patch to query.cgi that uses the 
printLabel and printEndLabel subs, because some browsers break with 
the label tag. 

Comment 21

[Gervase Markham [:gerv]]

Problem is that landfill's bugzilla install is so hacked about, last I heard, 
tara was going to nuke it... If you want some files from there, let me know 
ASAP. 

If you are happy to sort this out, could you create new patches for CGI.pl, 
query.cgi and <the other file> and attach them? I'll make sure they appear on 
Landfill.

Adding 2.12 as reasonably low-risk, high-votes bug.

Gerv

Comment 22

[Zach Lipton [:zach]]

Attachment: patch v3

Comment 23

[Tara Hernandez]

This is now available on landfill.  The v3 version of the patch is still wildly
busted, so posting my fixed version for you perusal.  Please let me and/or zach
know which browsers you've hit the pages with, and if there was any broken
behavior.

Comment 24

[Tara Hernandez]

Attachment: v3 without all the compile errors :)

Comment 25

[Gervase Markham [:gerv]]

Doesn't break Netscape 4.75. Works correctly in Mozilla 20001025 except that 
Netscape 4 seems to imply a <BR> for </LABEL><LABEL> and Mozilla doesn't. This 
means that, for example, all the radio buttons (Leave as ASSIGNED, Resolve 
as... etc.) are on the same line.

This is bad. And ugly.

Gerv

Comment 26

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

LABEL should not imply line breaks -- it's an inline element.  In which browser
does it?

Comment 27

[Gervase Markham [:gerv]]

In fact, this bug isn't quite what I described. <BR> tags inside <LABEL> tags 
get ignored by Mozilla. dbaron - is this correct behaviour?

The "workaround" is to move the <BR>s outside the <LABEL> tags. Tara - you might 
need to patch up your patch :-)

Gerv

Comment 28

[timeless]

Just from a random user perspective, the thought of <br>'s in <label>'s sounds 
awful.

Comment 29

[Karl Ove Hufthammer]

> Doesn't break Netscape 4.75. Works correctly in Mozilla
> 20001025 except that 
> Netscape 4 seems to imply a <BR> for </LABEL><LABEL>
> and Mozilla doesn't.

I don't see this as a big problem, since soon, few bugs entered into BugZilla 
will be entered using Netscape <6. And using the 'label' element is really much 
more practical (good for usability).

Comment 30

[Gervase Markham [:gerv]]

Huftis: You are commenting on a description of the bug which is incorrect :-) 
Please read my later comment. The bug is in Mozilla.

Gerv

Comment 31

[Tara Hernandez]

Not to mention that assuming the world will upgrade to 6.0 is overly 
optimistic at best, and certainly not a reason avoid fixing bad 4.x behavior.

Anybody tried looking at this under (aaaii)IE(eeeee) yet?

Comment 32

[Zach Lipton [:zach]]

Everything works fine in IE 5 on the Mac. IE does understand <label> and 
it does everything it should. There might be a mozilla html element bug 
here. Has this been reported already. This bug should be an rtm++ one 
(knowing PDT it won't be). Pulling the 2.12 because of the mozilla 
problem.

Comment 33

[Jesse Ruderman]

See bug 43771 and Gerv's bug 58940 for the Mozilla problems.  What happened to 
the idea of moving the <br>s outside of the <label>s for this bug?

Btw, does the patch move the Commit button on show_bug.cgi down a line?  I 
often click on the text for the last "knob" when I'm trying to click Commit.

Comment 34

[Dave Miller [:justdave]]

*** Bug 66462 has been marked as a duplicate of this bug. ***

Comment 35

[Alec Flett]

so what do I need to do to get this into the latest version of bugzilla?

Comment 36

[Alec Flett]

btw, in the smaller patch I had in bug 66462, I put the <br>'s outside the
<label> (as they should have been!) and it seems to work fine.

Comment 37

[Jacob Steenhagen]

http://bugzilla.mozilla.org/showattachment.cgi?attach_id=23405
Personally, I prefer the patch from the bug 66462 as it doesn't have the <br>
problem mentioned in this bug.  Also, as is stated both here and in the other
bug, this is now standard HTML (v4) so it's supported by more than just Mozilla.
 And by the very nature of HTML, any borwser that doesn't support this feature
will simply ignore it.

Comment 38

[Robin Lionheart]

Adding "patch" keyword.

Comment 39

[Matthew Tuck [:CodeMachine]]

Nominating for 2.14, but is this ready right now, ie for 2.12 consideration?

Comment 40

[Stephan Niemz]

I don't understand why you're checking if the HTTP_USER_AGENT contains
Mozilla before printing a <label>.  Any UA who doesn't understand that tag
should just ignore it, and probably will.  So IMHO the tag could be sent
out unconditionally.  This would also make things easier - no new sub-s,
no additional require "CGI.pl", etc.

Comment 41

[Zach Lipton [:zach]]

This actually needs to be changed. We shouldn't check for a mozilla UA, 
but we should move the tags outside of the <br>. I am SWAMPED with 
work on 'testmanager' along with kiosk mode...So can someone be really 
nice and fix this? Basically, it would require moving printlabel() calls 
outside of <br> tags and taking out the UA checking. We should still keep 
the printlabel() and printendlabel() subs around, incase we find another 
incompatibility with a browser that we need to filter out.

Comment 42

[Stephan Niemz]

taking bug

Comment 43

[Zach Lipton [:zach]]

Thanks Stx! Lifesaver!

Comment 44

[Stephan Niemz]

Attachment: patch 2001

Comment 45

[Stephan Niemz]

Attachment: patch without removal of trailing whitespace to improve readability for reviewers

Comment 46

[Stephan Niemz]

Attachment: patch without removal of trailing whitespace to improve readability for reviewers

Comment 47

[Stephan Niemz]

Patch is attached, please review and check in.  Patch 2001 is the "real
thing", the other one lacks whitespace changes (diff -ubB) to improve
readability for reviewers.

Sorry, the patch is quite big, since there are lots of
<input type="checkbox"> and <input type="radio"> in Bugzilla.  But it
is still only a change in the HTML output, so it's easy to test and is
almost riskless.  Could this still make it for 2.12?  (Nominating.)

I have tested most (maybe 80%) of the <label>s with Mozilla 0.8 on my
local Bugzilla installation and it works fine.

Comment 48

[Dave Miller [:justdave]]

sorry, but 2.12 is feature-locked and this will take too much time to review to 
sneak it in anyway.  Putting back to 2.16.  You do want us to release 2.12, 
right?

Comment 49

[Stephan Niemz]

That's what I feared, but please also consider
1. the number of votes (and CCs) on this bug,
2. its low-risk nature (only minor change in HTML, regardless of the amount) and
3. after other changes have been made, collisions will likely occur and this
   patch has to be redone, at least in part.

It took me about two or three hours to make this patch, so a review plus
checkin could be done in no more than an hour, I suppose.

Or maybe only check in the highly visible files like query.cgi, bug_form.pl,
createattachment.cgi and userprefs.cgi (with all these new email options where
this fix would be of great benefit) this time.

And yes, I'm eagerly waiting the 2.12 release, but I also want to help make it
as good as possible. :)  (Who knows when the next release will come out.)

Comment 50

[Dave Miller [:justdave]]

moving to real milestones...

Comment 51

[Dave Miller [:justdave]]

*** Bug 92752 has been marked as a duplicate of this bug. ***

Comment 52

[Gervase Markham [:gerv]]

faniz: can you confirm this patch doesn't have nasty effect on Navigator 4.x?
And that it still applies cleanly?

Gerv

Comment 53

[Gervase Markham [:gerv]]

Faniz? Are you still interested in pushing this patch?

Gerv

Comment 54

[Gervase Markham [:gerv]]

This patch has, from a visual inspection, almost certainly rotted. If one of the
nine people who voted for this bug wishes to regenerate the relevant parts of
it, please do so, and move it back to 2.16.

We also need the definitive word on cross-browser testing of the changes. No-one
seems to know if this breaks any browsers or not.

Gerv

Comment 55

[Matthew Tuck [:CodeMachine]]

It's probably best to wait until templatisation has finished, before
regenerating the patch.

Comment 56

[Robin Lionheart]

*** Bug 105578 has been marked as a duplicate of this bug. ***

Comment 57

[Christian Reis]

If nobody disagrees, I will use this bug as a tracker for various label-related
bugs I will file (and fix). Better to have small patches than big ones, you
heard the man..

Comment 58

[Bradley Baetz (:bbaetz)]

*** Bug 122898 has been marked as a duplicate of this bug. ***

Comment 59

[Andreas Franke (gone)]

From bug 122898, with respect to the query page:

> I recommend:
> Alt+Q submit _q_uery
> Alt+S _s_ummary
> Alt+D a _d_escription entry
> Alt+U _U_RL
> Alt+W Status _w_hiteboard
> Alt+K _K_eyword

Comment 60

[Tobias Burnus]

Note that the <label> should also be used for type=input und for <select> in
order to be compatible with
a) W3C Web Content Accessibility Guidelines, Priority 2
   http://www.w3.org/TR/WAI-WEBCONTENT/
b) the US Sec.508 (http://www.access-board.gov/sec508/508standards.htm)
   as tested by http://bobby.watchfire.com

Comment 61

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 26224 [details] [diff] [review]
patch 2001

Rotted.

Comment 62

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 26226 [details] [diff] [review]
patch without removal of trailing whitespace to improve readability for reviewers

Rotted.

All the dependencies are fixed, so if this is a meta-bug it should be fixed as
well.  Christian (or anyone else), is there any more work to be done here?

Comment 63

[Christian Reis]

I'll do a review of the code in CVS HEAD and will open more bugs if they are
missing, closing this bug if not.

Comment 64

[Asbjørn Ulsberg]

At 2002-08-11 12:15 Tobias Burnus wrote:

> Note that the <label> should also be used for type=input und for <select> in
> order to be compatible with [...] W3C Web Content Accessibility Guidelines.

*All* form elements should have an appurtenant <label> tag if it has some kind
of description. It's not only <input type="checkbox"> and <input
type="radiobutton"> that has usage for it, all <input>'s, <select>'s, <button>'s
etc, will be more accessible if their descriptive text is wrapped in a <label>
tag. Also, the <label> tag should use the 'for' attribute to explicitly tell
what field it belongs to, and not the alternative <label><input></label> syntax,
because the former works better in all browsers.

And while speaking of accessibility, the <table> tag is used way too much on
bugzilla. It should only be used for tabular data, and CSS should be used for
design purposes like positioning. The bugzilla design is so simple that it is
quite easy to accomplish the same things with CSS that are now accomplished with
<table>'s.

-Asbjørn

Comment 65

[Robin Lionheart]

64> Also, the <label> tag should use the 'for' attribute to 
64> explicitly tell what field it belongs to, and not the
64> alternative <label><input></label> syntax, because the
64> former works better in all browsers.

Of all the browsers I've tested, Internet Explorer is the only one in
which <label for=...> works and <label><input></label> doesn't work.

I agree this makes <label for=...> preferable, but let's not obfuscate
"only the former works in Internet Explorer" as "the former works better
in all browsers". Unless you are aware of any others?

Comment 66

[Jouni Heikniemi]

*** Bug 157573 has been marked as a duplicate of this bug. ***

Comment 67

[Jouni Heikniemi]

I just duped bug 157573 on this; that bug has a patch by burnus that is somewhat
more up to date than those attached to this bug. If someone picks up the ball, I
believe that's a good place to start.

Comment 68

[Jeff Walden [:Waldo]]

Adding access keyword.

Also cf. http://diveintoaccessibility.org/day_28_labeling_form_elements.html

Comment 69

[Jeffrey Baker]

Uploading a patch to edit.html.tmpl which adds label fields for each radio
button in the "knob" field.  This increases usability quite a lot.  I hope this
will be accepted.  I am also reassigning to myself, since the assignee hasn't
been here in three years.

Comment 70

[Jeffrey Baker]

Attachment: Adds label fields to some radio buttons in edit.html.tmpl

The existing patches are obsolete since moving to template system.  This patch
is versus 2.16.4.

Comment 71

[Jeffrey Baker]

Accepting

Comment 72

[Vlad Dascalu]

We could use a patch againest the CVS tip.

Comment 73

[Jeffrey Baker]

Attachment: Patch to knob.html.tmpl

Adds label fields to some radio buttons in knob.html.tmpl CVS revision 1.3, as
requested.

Comment 74

[Gervase Markham [:gerv]]

Comment on attachment 145060 [details] [diff] [review]
Patch to knob.html.tmpl

r=gerv.

Gerv

Comment 75

[Vlad Dascalu]

Checking in template/en/default/bug/knob.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/knob.html.tmpl,v  <--
 knob.html.tmpl
new revision: 1.4; previous revision: 1.3
done

Comment 76

[timeless]

your label spanned to much and broke stuff.