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

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

This is invalid HTML. If a control is within a LABEL, then the "for" attribute shouldn't be used.

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?

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.

Should clicking inside the text control fire the label's onfocus event handler? What about its onclick event handler?

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

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

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

*** 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.

# 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

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?

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.

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

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."

(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

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.

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.

(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.

(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.

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.

(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.

> 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.

I can still reproduce this bug with this code ``` <button class="sc-gGBfsJ hpvoYR" width="133px" height="40px" style="position: relative; display: flex;"> <div class="sc-exAgwC iExMic" display="inline-block" verticalalign="middle"> # </div> <div class="sc-dfVpRl iwejbh" height="39px" width="1px"></div> <input class="sc-hXRMBi jcbBix sc-gwVKww eIYBQM" name="order" placeholder="Input Placeholder" type="text" height="field" font-weight="normal" value=""> </button> ``` So on save this file as HTML and opening in Chrome. I can focus in input box but on opening the same HTML in Firefox, focus on input is not working. I think we should be focusable. At least to keep the functionality similar

(In reply to Arshad Kazmi [:arshadkazmi42] from comment #44) > I can still reproduce this bug with this code > > ``` > <button class="sc-gGBfsJ hpvoYR" width="133px" height="40px" > style="position: relative; display: flex;"> > <div class="sc-exAgwC iExMic" display="inline-block" > verticalalign="middle"> > # > </div> > <div class="sc-dfVpRl iwejbh" height="39px" width="1px"></div> > <input class="sc-hXRMBi jcbBix sc-gwVKww eIYBQM" name="order" > placeholder="Input Placeholder" type="text" height="field" > font-weight="normal" value=""> > </button> > ``` > > So on save this file as HTML and opening in Chrome. I can focus in input box > but on opening the same HTML in Firefox, focus on input is not working. > I think we should be focusable. At least to keep the functionality similar Your case uses an enclosing <button>, not a <label>. Please file a separate bug for that. This bug as filed was fixed in bug 229925 for Firefox 38, 4 years ago. \o/

Gijs, Thank you for informing. I will file a new bug for this.

The button case behavior was changed in bug 1089326 a few weeks ago. I suggest testing a nightly to see the new behavior; when I try the testcase from comment 44 in a nightly, focusing the input works fine.

Fwiw, bug 1516416 was filed per comment 46; I'll follow up there.