Unable to focus INPUT element inside a LABEL element

NEW
Assigned to

Status

()

16 years ago
6 months ago

People

(Reporter: mikko.rantalainen, Assigned: mats)

Tracking

({testcase})

Trunk
testcase
Points:
---
Bug Flags:
wanted-next +
blocking1.9 -
wanted1.9 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(9 attachments)

(Reporter)

Description

16 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030630
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030630

If a LABEL element contains an INPUT element and attribute FOR, then the INPUT
element cannot be focused with mouse. I'll attach a test case.

Reproducible: Always

Steps to Reproduce:
1. Open attached test case (color selection)
2. Click label "Other" (or text input) and try to type your favorite color

Actual Results:  
Focus is transferred to radio button and there seems to be no way to enter text
in the input field. User must press TAB once to access the text input.

Expected Results:  
This is a bit unclear because strictly following the spec by letter results to
Mozilla's behaviour. However, the best way would be to activate the radio button
to select it *and* set focus to element that is inside the label element (the
text field). Yes, that behaviour could be  against the spec but it makes sense
(at least for me) and it's the only way to make a text input that selects a
radio button when it's focused without using javascript. Bare minimum should be
that if the input field inside label is clicked, the input field is focused so
that the user can enter text. Activating the text inside the label element could
focus the radio button according to the spec.


The HTML spec[1] says [2] that the label element should transfer focus to
associated control, but it doesn't define what should happen if a LABEL is
associated with two controls (I guess the spec tried to skip that issue by
disallowing nested labels).

[1] http://www.w3.org/TR/html401/interact/forms.html#edef-LABEL
[2] http://www.w3.org/TR/html401/interact/forms.html#idx-label-2

Possibly related bugs:
bug 146066
bug 136326
(Reporter)

Comment 1

16 years ago
Created attachment 128314 [details]
Testcase for selecting input inside a label with for attribute

Try to type something in the text input field.
It's harder than one would expect...

Comment 2

16 years ago
This is invalid HTML. If a control is within a LABEL, then the "for" attribute
shouldn't be used.
(Reporter)

Comment 3

16 years ago
I agree that the *correct behaviour* for the test case isn't that clear.
However, if you want to claim that the attached test case contains invalid HTML,
please, post some evidence. http://validator.w3.org/ says it's valid XHTML 1.1.
Strict (I know, that validator isn't perfect) and I cannot locate anything in
the spec that disallows such construct.

The spec says that a label can be associated with exactly one control. I read
the as if a LABEL element contains both FOR attribute and control as it's
content, then the FOR attribute should override the behaviour and the content
should be inaccessible (or at least, one cannot focus it). I *hope* that really
isn't the intent of the spec.
> I *hope* that really isn't the intent of the spec.

Seems to be pretty clearly the intent to me -- clicking the contents of the
label should trigger the label's default click action when the click event
bubbles up, and the default click action of a label with a "for" attribute is to
focus the control identified by the "for" attribute.

The spec does clearly state that the "for" attribute association overrides the
"containment" association, so the label is definitly associated with the radio
button, not the textfield.

So where is the bug, exactly?
(Assignee)

Comment 5

16 years ago
I would expect that clicking inside the text control itself should not trigger
the label behaviour, but instead the focus should stay with that control.
Keywords: testcase
Should clicking inside the text control fire the label's onfocus event handler?
 What about its onclick event handler?
(Assignee)

Comment 7

16 years ago
Created attachment 128368 [details]
Testcase

Hmm, perhaps this is correct behaviour after all...
It also seems compatible to what Opera and IE does.

Comment 8

15 years ago
*** Bug 222744 has been marked as a duplicate of this bug. ***

Comment 9

15 years ago
*** Bug 249339 has been marked as a duplicate of this bug. ***

Comment 10

15 years ago
*** Bug 254247 has been marked as a duplicate of this bug. ***
I reproduced this bug in 'e-casio' that is major shopping site in Japanese with
2004081208-trunk/WinXP.
Created attachment 156037 [details]
e-casio's problem(radio-button + label > select)

Updated

14 years ago
Assignee: core.layout.form-controls → nobody
QA Contact: desale → core.layout.form-controls
# In any case, events targeted at form controls within a label must not be 
# handled by the label itself.
 -- http://whatwg.org/specs/web-forms/current-work/#label
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
Hardware: PC → All

Comment 14

13 years ago
I'm finding a related problem with some web pages, and was hoping you guys could tell me if it's an exact dupe or not. I'll create an attachment which follows.

The bug occurs if you have an input field ("field 2") which is enclosed within a label, and the 'for' property of that label is the same as the 'id' property of a previous field ("field 1", which is not associated with the label). In that situation, the focus for field 2 always jumps back to field 1. It doesn't do it if  the input tag for field 2 is outside the label though.

Culd you please have a look at the following attachment, and let me know if it's this bug?

Comment 15

13 years ago
Created attachment 211254 [details]
Demonstrates the label/input weirdness for 2 fields

Updated

13 years ago
Severity: minor → normal

Comment 16

13 years ago
Created attachment 211255 [details]
This one doesn't show the bug

As a contrast, I'm also attaching a version where the <input> tage for field 2 isn't enclosed by the <label>. In this case, the field behaves normally.

Comment 17

13 years ago
Created attachment 220454 [details]
Shows that even the for attribute is not required to reproduce this bug

This test case shows that even a "for" attribute is not required to reproduce this.

Updated

13 years ago
Attachment #220454 - Attachment mime type: text/plain → text/html

Updated

13 years ago
Flags: blocking1.9a1?

Updated

13 years ago
Flags: blocking1.9a1? → blocking1.9+
Whiteboard: [wanted-1.9]

Updated

13 years ago
Flags: blocking1.9+ → blocking1.9-
(Assignee)

Comment 18

13 years ago
Created attachment 233567 [details]
Testcase #7
(Assignee)

Updated

13 years ago
Assignee: nobody → mats.palmgren
Summary: Unable to focus INPUT element inside a LABEL element → [FIX] Unable to focus INPUT element inside a LABEL element
(Assignee)

Comment 19

13 years ago
Created attachment 233569 [details] [diff] [review]
Patch rev. 1

This patch tries to implement:

http://whatwg.org/specs/web-forms/current-work/#label
 "In any case, events targeted at form controls (or other interactive
  elements, e.g. links) within a label must not be handled by the
  label itself."
Attachment #233569 - Flags: superreview?(bzbarsky)
Attachment #233569 - Flags: review?(bzbarsky)

Comment 20

13 years ago
(In reply to comment #19)
> Created an attachment (id=233569) [edit]
> Patch rev. 1

That seems to fix all the testcases for me, including my own. Nice!
Hmm...  I'm still not that happy with Hixie's spec, but I guess it's the right general idea.

I have a couple of questions about the patch:

1)  Should we be using originalTarget, rather than target?  It would seem to me
    that we should be.
2)  The hardcoding of various types of things is very fragile, and reusing
    existing interfaces for testing can lead to subtle bugs.  For example, the
    patch as written treats <fieldset> as interactive, while an <img> with an
    imagemap is not, as far as I can tell.  Although maybe the latter case is
    handled by the events being dispatched to the <area> instead?  In any case, 
    there are also inconsistencies as far as <link> elements and <a> with no
    href in the JS and the C++.
3)  <object> and <iframe> can be interactive, depending on what's loaded in
    them.

What I feel we should do is to introduce a method to ask a node whether it's interactive.  Probably put it on nsIContent and maybe also expose it on nsIDOMNSSomething (not sure what would be most appropriate) so that the XUL code can work right.  Then go through every single one of our concrete element classes and see what they should return.

Note that all of this breaks down if the page ever uses an onclick handler, but in that case they should be able to use preventDefault to prevent the default action, right?

Which raises the basic question in my mind yet again: why are some of these default actions not simply calling preventDefault?

One last thing -- I think this is the sort of patch where more than one review might be a good idea; I suggest jst or sicking.
Comment on attachment 233569 [details] [diff] [review]
Patch rev. 1

r- per comment 21
Attachment #233569 - Flags: superreview?(bzbarsky)
Attachment #233569 - Flags: superreview+
Attachment #233569 - Flags: review?(bzbarsky)
Attachment #233569 - Flags: review-
Duplicate of this bug: 303382

Updated

12 years ago
Duplicate of this bug: 386649

Updated

11 years ago
Duplicate of this bug: 400668
Flags: wanted1.9+
Whiteboard: [wanted-1.9]

Comment 26

11 years ago
Created attachment 297596 [details]
Test case 8 with multiple inputs

This test case shows that a label (1) that triggers an input (A) that is in a different label (2) will be forwarded to the input (B) that label 2 refers to (so 1/A causes B to be selected) and this can continue over several inputs/labels.  Sorry for the difficulty in explaining it; the source makes more sense.  The source demonstrates why this could easily occur by not closing the label tag as well as what happens when it is closed.  This is a core issue present in FF3.0b3pre and SeaMonkey 2.0a1pre, Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008011704 Minefield/3.0b3pre and Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008011701 SeaMonkey/2.0a1pre respectively.
Flags: wanted1.9-
Flags: wanted1.9+
Flags: wanted-next+
Duplicate of this bug: 450096
Duplicate of this bug: 296135
No longer blocks: 338303
Duplicate of this bug: 338303
Blocks: 595270
(Reporter)

Comment 30

8 years ago
Is the reasoning in comment 21 still valid today? I think that the currently implemented behavior is not sane and some kind of change should be done.

As far as I understand, the suggested patch fixes all test cases but it *could* cause regressions in some (uknown?) cases. Is there any real word example of such a page?
> Is the reasoning in comment 21 still valid today? 

Which reasoning?

> As far as I understand, the suggested patch fixes all test cases

All the ones attached to this bug.  Comment 21 mentions some test cases it breaks.

> Is there any real word example of such a page?

Are there any real world pages with imagemaps?  Probably yes.  ;)
More to the point, the idea of code review is to make sure we don't check in buggy code.  Trading one set of bugs for another may be required in some cases, but it's so not required in this case.  We should just fix this with code that doesn't introduce other bugs.
(Reporter)

Comment 33

8 years ago
(In reply to comment #31)
> > Is the reasoning in comment 21 still valid today? 
> 
> Which reasoning?

Comment 22 refers to comment 21 for the reason to fail the review. I was just wondering the the reason to fail the review is still valid with the current code base. (A lot has changed since 2006.)

> > Is there any real word example of such a page?
> 
> Are there any real world pages with imagemaps?  Probably yes.  ;)

Does the patch break imagemaps even if there is not a wrapping label? I though that the patch was supposed to fix the "interactive element inside a label" case. I would guess that there are no real world examples of imagemaps inside a label.

I do understand the idea of code review and I totally support it. I'd just prefer to see more clear reasoning for failed code review (especially what needs to be changed or what part will not be accepted). In case of open source projects, it's even more important because that failed review may be the best hint to somebody else to improve the code if the original patch author has lost the interest to improve the patch.

In case of this specific patch and comment 21 I'm not sure if the review was failed because of implementation quality or because of the logic/algorithms used. This makes a huge difference in case somebody tries to improve the patch.
> I was just wondering the the reason to fail the review is still valid with the
> current code base.

They are.

> I would guess that there are no real world examples of imagemaps inside a
> label.

I would guess you would be wrong... 

> especially what needs to be changed or what part will not be accepted

I really don't know how more clear the part of comment 21 after the 3-item list can be.  It explicitly says what needs to be changed!

> I'm not sure if the review was failed because of implementation quality or
> because of the logic/algorithms used.

I have no idea where you're drawing the distinction between those.  The main problem with that patch is that the way it determines whether an element is "interactive" is fundamentally broken.
(Reporter)

Comment 35

8 years ago
(In reply to comment #34)
> The main problem with that patch is that the way it determines
> whether an element is "interactive" is fundamentally broken.

OK. I get it now. I just hope that that sentence had been part of comment 21.
FWIWI, the Opera behavior is now specified in HTML specs. Though, I would have prefered the Webkit one.
Summary: [FIX] Unable to focus INPUT element inside a LABEL element → Unable to focus INPUT element inside a LABEL element
Duplicate of this bug: 717189

Updated

6 years ago
Duplicate of this bug: 781115

Comment 39

5 years ago
Testing this against other browser's behavior, it seems browser behaviors differ but only Firefox focuses the radio button instead of the textfield.

Against the first test case (https://bugzilla.mozilla.org/attachment.cgi?id=128314) browsers have the following behaviors when clicking on the INPUT field within the LABEL tag:

Mac/Safari 6.0.5: Radio button is selected and textfield is focused.
Mac/Chrome 28.0.1500.95: Radio button is selected and textfield is focused.
Win/IE 8/9/10: Radio button NOT selected and the textfield is focused.
Mac/Opera 12.02: Radio button NOT selected and the textfield is focused.
Mac/Firefox 22.0: Radio button selected and the textfield is NOT focused.

IMO, the Safari/Chrome behavior is the best from a UX perspective, but even the IE/Opera approach is better than what Firefox is doing currently.
Duplicate of this bug: 987627

Updated

5 years ago
Duplicate of this bug: 1034839

Comment 42

5 years ago
(In reply to Boris Zbarsky [:bz] from comment #21)
> What I feel we should do is to introduce a method to ask a node whether it's
> interactive.  Probably put it on nsIContent and maybe also expose it on
> nsIDOMNSSomething (not sure what would be most appropriate) so that the XUL
> code can work right.  Then go through every single one of our concrete
> element classes and see what they should return.

Should this be done in webidl now (rather than nsIDOMNSSomething), with [Func="IsChromeOrXBL"] (on HTMLElement.webidl? elsewhere?), if one wanted to have a go at fixing this?

Asking for a friend.
Flags: needinfo?(bzbarsky)
> Should this be done in webidl now 

Yes.

Where to put it is non-obvious, but putting it on Element or HTMLElement are the clear options.  Element seems better to me.
Flags: needinfo?(bzbarsky)
You need to log in before you can comment on or make changes to this bug.