Closed Bug 229925 Opened 20 years ago Closed 9 years ago

enclosing several form buttons in one <label> makes all buttons act like first button

Categories

(Core :: Layout: Form Controls, defect)

Product:
Core
Component:
Layout: Form Controls
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla38

People

(Reporter: shortestpath, Assigned: arai)

References

(Depends on 1 open bug,
URL
)

Details

Attachments

(5 files, 1 obsolete file)

example
137 bytes, text/html
Details
sample from alex
976 bytes, text/html
Details
radio buttons: "yes or no"
457 bytes, text/html
Details
Test case, 2 input box within a label tag
406 bytes, text/html
Details
Do not dispatch event to label target if interactive content is found between the event target and the label.
23.23 KB, patch
smaug
: review+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031216 Firebird/0.7+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.6b) Gecko/20031216 Firebird/0.7+

If a reset and/or regular <input> button is placed after an input type="submit"
button, the reset and/or regular buttons act as submit buttons. However if the
submit button is placed before all other buttons, the buttons work as normal.


Reproducible: Always

Steps to Reproduce:
1. Create a form, method="post", action to wherever you'd like.
2. Place an input type="submit" button first, then one or more regular input
buttons after this.
3. Click any one of the regular buttons (not the submit button). 

Actual Results:  
The regular buttons act as a submit button and submit the form.

Expected Results:  
The regular buttons should've acted as regular buttons, and not submit the form!
Attached file example — 
WFM, WinXP Fb 20031230.
Thanks for adding the attachment Jason. As in your sample form, I tried my own
which posted to mozilla.org; I could swear that the first few times I was able
to duplicate the bug. But after that, I wasn't able to anymore. Even now with
your sample form which posts to mozilla.org, the regular button does not act as
a submit button on my browser. I don't know why, but perhaps I my machine is not
allowed to post to mozilla.org anymore?

On my local webserver, I set up an asp page with the sample form posting to a
second asp page (in other words, not to mozilla.org), and with this I am able to
duplicate the bug every time. (Just thought I'd add more detail to this bug.)
alex, can you attach your sample page?  we don't need the asp it submits to,
just the form.
Attached file sample from alex — 

    
    

    
  
You're enclosing akk the buttons in a single <label> tag, which is not how that
tag is meant to be used. Removing the <label> tag fixes the problem.
http://www.w3.org/TR/html4/interact/forms.html#h-17.9

Still, I don't think Moz should be doing what it's doing. The specs only say
there should be one form control per <label>, not what happens if there's more.

Moving to Browser/Layout Controls, since this happens in Seamonkey too.
Assignee: blake → nobody
Component: General → Layout: Form Controls
Product: Firebird → Browser
QA Contact: core.layout.form-controls
Version: unspecified → Trunk

    
  
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: reset and regular buttons act as submit buttons → enclosing several form buttons in one <label> makes all buttons act like first button
Depends on: 127903

    
    

    
  
There is an other variant of this bug: multiple <input type="radio" ...>'s in
one <label>...</label>. This time the second radio button works as it was the
first one causing only the "Yes" can be selected. (This may change the severity
of this bug 'critical')

Here is an (almost) minimal example HTML code which produces the buggy behavioral:

------------------------------------------------------
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
</head>
<body>

<form name="DS156" method="post" action="ds156_complete.asp?pdf=DS156_Complete.pdf">
<label for="f35">
    35. Has Your U.S. Visa Ever Been Cancelled or Revoked?
    <input tabindex="83" type="radio" name="fvisarevoked" value="Yes" id="f35"> Yes
    <input tabindex="83" type="radio" name="fvisarevoked" value="No" id="f35"> No
</label>

</form>

</body>
</html>
------------------------------------------------------

You can find the real-world example at
https://evisaforms.state.gov/ds156.asp?lang=1 , see the question No. 35.

Mozilla version used: Mozilla/5.0 (X11; U; Linux i686; hu; rv:1.6) Gecko/20040113


    
    

    
  


  
(added HTML code as attachement too for simpler to try.)

    
    

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

    
    

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

    
  
URL: https://evisaforms.state.gov/ds156.as...
OS: Windows 2000 → All

    
    

    
  
Assigned to dbaron by request
Assignee: nobody → dbaron

    
    

    
  
So I was thinking for a minute that this was really easy to fix, but it's not
what I was thinking.  I think there are three options:
 * make some set of form controls call preventBubble on the event or do the
equivalent to nsEventStatus
 * make nsHTMLLabelElement::HandleDOMEvent check its nsEventStatus differently
(will this work?)
 * make nsHTMLLabelElement::HandleDOMEvent check that the event target is in any
form control rather than just the for content.  This may be easiest...

    
    

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

    
    

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

    
    

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

    
    

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

    
    

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

    
    

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

    
  
Blocks: 218788

    
    

    
  
http://www.first-steps.org/links/members/pay.html
You can not select input-box except "paypal id" by mouse click.

However, IE7, Google Chrome and Opera9.64 operate as expected.

I think this issue should be fixed asap.

    
    

    
  


  
Unable to select the second input box, click the mouse.

But,IE7, Google Chrome and Opera work as expected.

    
  
Assignee: dbaron → nobody

    
    

    
  


  
(In reply to David Baron [:dbaron] (UTC-8) (needinfo? for questions) from comment #11)
>  * make nsHTMLLabelElement::HandleDOMEvent check that the event target is in
> any
> form control rather than just the for content.  This may be easiest...

I'd like to choose this one :)

Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b98ba6892548


Then, currently ":hover" and ":active" are also applied to labeled control, even if cursor is on another form control. For example, in attachment 149761 [details], clicking 2nd radio button makes 1st radio button ":hover" and ":active". According to the sepc, this behavior is correct, but it might be nice to add some exceptional case, to make it clear which control is actually hovered or activated (especially for ":activate").
  https://html.spec.whatwg.org/multipage/scripting.html#selector-hover
Draft patch is here:
  https://hg.mozilla.org/try/rev/ea4d7214eacd

        Attachment #8550106 -
        Flags: review?(dbaron)

    
    

    
  
Does the test fail prior to the patch and pass with the patch?

    
    

    
  
Yes, at least locally, both click event and key event do not work as expected.

2 INFO TEST-PASS | dom/html/test/forms/test_radio_in_label.html | The first radio input element should be checked by clicking the element 
3 INFO TEST-UNEXPECTED-FAIL | dom/html/test/forms/test_radio_in_label.html | The second radio input element should be checked by clicking the element - expected PASS
4 INFO TEST-PASS | dom/html/test/forms/test_radio_in_label.html | The first radio input element should be checked by clicking other element 
5 INFO TEST-UNEXPECTED-FAIL | dom/html/test/forms/test_radio_in_label.html | The second radio input element should be checked by key - expected PASS
6 INFO TEST-PASS | dom/html/test/forms/test_radio_in_label.html | The first radio input element should be checked by key 

I pushed test-only try, it will be finished within an hour.
  https://treeherder.mozilla.org/#/jobs?repo=try&revision=921e4bb4bdf0

    
    

    
  
Comment on attachment 8550106 [details] [diff] [review]
Do not dispatch event to label target if labelable element is found between the event target and the label.

I think smaug is probably a better reviewer here.

(Sorry, I should have realized this last night.)

        Attachment #8550106 -
        Flags: review?(dbaron) → review?(bugs)

    
    

    
  
Comment on attachment 8550106 [details] [diff] [review]
Do not dispatch event to label target if labelable element is found between the event target and the label.

As discussed on IRC, I think it would be better to just make this follow the
spec.
https://html.spec.whatwg.org/multipage/forms.html#the-label-element:the-label-element-10
https://html.spec.whatwg.org/multipage/dom.html#interactive-content-2

But if the changes are difficult to make, feel free to ask re-review for this patch.

        Attachment #8550106 -
        Flags: review?(bugs)

    
    

    
  


  
Added Element::IsInteractiveHTMLContent method, "Interactive content" classes override it, and HTMLLabelElement::PostHandleEvent checks it.

Then, "sorting interface th elements" is also listed in "Interactive content", but I guess it's not yet implemented, right? In that case, I'd like to leave it now.

Green on try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a86aabd6b419

        Attachment #8550106 -
        Attachment is obsolete: true

        Attachment #8551514 -
        Flags: review?(bugs)

    
    

    
  
Yeah, I don't think 'sorting interface th elements' is implemented in any browser engine
(and maybe it will be removed from HTML).

    
    

    
  
Comment on attachment 8551514 [details] [diff] [review]
Do not dispatch event to label target if interactive content is found between the event target and the label.

>+++ b/dom/html/HTMLAnchorElement.h
...
>+  // Element
>+  virtual bool IsInteractiveHTMLContent() const
MOZ_OVERRIDE;

Same also elsewhere when overriding the method declared in Element.h


> {
>-  nsCOMPtr<nsIContent> c = do_QueryInterface(aEvent->target);
>-  nsIContent *content = c;
>-  while (content) {
>-    if (content == aChild) {
>+  nsIContent *content = aContent;
* goes with the type. nsIContent* content. (the old code uses wrong style)


>+  while (content && content != aStop) {
>+    Element* element = content->IsElement() ? content->AsElement() : nullptr;
>+    if (element && element->IsInteractiveHTMLContent()) {
>       return true;
>     }

Maybe

while (content && content != aStop) {
  if (aContent->IsElement() &&
      aContent->AsElement()->IsInteractiveHTMLContent()) {
    return true;
  }
...

        Attachment #8551514 -
        Flags: review?(bugs) → review+

    
    

    
  
Thank you for reviewing :)
Fixed them and landed.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e8981c280f7b

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/e8981c280f7b
Assignee: nobody → arai_a
Status: NEW → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Blocks: 943935

    
  
Depends on: 1141455

    
  
Depends on: 1167816

    
  
Depends on: 1167862

          You need to log in
          before you can comment on or make changes to this bug.