Closed Bug 14887 Opened 20 years ago Closed 16 years ago

bugzilla should use <label> tag in forms

Categories

(Bugzilla :: User Interface, enhancement, P2)

2.10
enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 2.18

People

(Reporter: michael.j.lowe, Assigned: jwbaker)

References

Details

(Keywords: access)

Attachments

(1 file, 8 obsolete files)

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>
What browsers support this?  Communicator 4.61 on Linux doesn't seem to...
Erm... Mozilla!  (Some of us do actually use it some of the time.  It works :-)
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!
Status: NEW → ASSIGNED
Changing description to be more clear
Summary: HTML form controls → bugzilla should use <label> tag in forms
tara@tequilarista.org 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 .)
Assignee: terry → tara
Status: ASSIGNED → NEW
I find this bug important and I am willing to take it on. Assigning to self.
Status: NEW → ASSIGNED
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.
Somehow, I assaigned this bug to someone else. Reassigning to self.
Assignee: tara → zach
Status: ASSIGNED → NEW
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!
Status: NEW → ASSIGNED
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.
what does netscape for linux do when you feed it an html page with <label>s?
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.
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.
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)
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.
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.
Zach - what's the status of this patch?

Gerv
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. 
OS: Windows NT → All
Hardware: PC → All
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
Whiteboard: 2.12
Attached patch patch v3 (obsolete) — Splinter Review
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.
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
LABEL should not imply line breaks -- it's an inline element.  In which browser
does it?
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
Just from a random user perspective, the thought of <br>'s in <label>'s sounds 
awful.
> 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).
Huftis: You are commenting on a description of the bug which is incorrect :-) 
Please read my later comment. The bug is in Mozilla.

Gerv
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?
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.
Whiteboard: 2.12
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.
*** Bug 66462 has been marked as a duplicate of this bug. ***
so what do I need to do to get this into the latest version of bugzilla?
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.
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.
Adding "patch" keyword.
Keywords: patch
Nominating for 2.14, but is this ready right now, ie for 2.12 consideration?
QA Contact: matty
Whiteboard: 2.14
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.
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.
taking bug
Assignee: zach → st.n
Status: ASSIGNED → NEW
Thanks Stx! Lifesaver!
Whiteboard: 2.14 → 2.16
Attached patch patch 2001 (obsolete) — Splinter Review
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.
Keywords: review
Whiteboard: 2.16 → 2.12
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?
Whiteboard: 2.12 → 2.16
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.)
moving to real milestones...
Target Milestone: --- → Bugzilla 2.16
Priority: P3 → P5
Priority: P5 → P2
*** Bug 92752 has been marked as a duplicate of this bug. ***
Component: Bugzilla → User Interface
Product: Webtools → Bugzilla
Version: other → 2.10
faniz: can you confirm this patch doesn't have nasty effect on Navigator 4.x?
And that it still applies cleanly?

Gerv
Faniz? Are you still interested in pushing this patch?

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
Target Milestone: Bugzilla 2.16 → Future
It's probably best to wait until templatisation has finished, before
regenerating the patch.
Keywords: patch, review
Whiteboard: 2.16
Severity: normal → enhancement
Target Milestone: Future → Bugzilla 2.18
*** Bug 105578 has been marked as a duplicate of this bug. ***
Depends on: 110711
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..
Depends on: 123740
*** Bug 122898 has been marked as a duplicate of this bug. ***
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
Blocks: 159582
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
Attachment #12688 - Flags: review?
Attachment #18352 - Flags: review?
Attachment #26224 - Flags: review?
Attachment #13771 - Flags: review?
Attachment #26225 - Flags: review?
Attachment #26226 - Flags: review?
Attachment #18407 - Flags: review?
Attachment #26225 - Attachment is obsolete: true
Attachment #26225 - Flags: review?
Attachment #12688 - Attachment is obsolete: true
Attachment #12688 - Flags: review?
Attachment #13771 - Attachment is obsolete: true
Attachment #13771 - Flags: review?
Attachment #18352 - Attachment is obsolete: true
Attachment #18352 - Flags: review?
Attachment #18407 - Attachment is obsolete: true
Attachment #18407 - Flags: review?
Comment on attachment 26224 [details] [diff] [review]
patch 2001

Rotted.
Attachment #26224 - Flags: review? → review-
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?
Attachment #26226 - Flags: review? → review-
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.
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
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?
*** Bug 157573 has been marked as a duplicate of this bug. ***
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.
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
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.
Assignee: st.n → jwbaker
The existing patches are obsolete since moving to template system.  This patch
is versus 2.16.4.
Attachment #26224 - Attachment is obsolete: true
Attachment #26226 - Attachment is obsolete: true
Accepting
Status: NEW → ASSIGNED
We could use a patch againest the CVS tip.
Adds label fields to some radio buttons in knob.html.tmpl CVS revision 1.3, as
requested.
Attachment #145056 - Attachment is obsolete: true
Comment on attachment 145060 [details] [diff] [review]
Patch to knob.html.tmpl

r=gerv.

Gerv
Attachment #145060 - Flags: review+
Flags: approval? → approval+
Target Milestone: Bugzilla 2.20 → Bugzilla 2.18
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
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
No longer blocks: 159582
Depends on: 159582
your label spanned to much and broke stuff.
Blocks: 241270
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.