Geoff Lankow (:darktrojan) Assignee
	Description • 15 years ago

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-GB; rv:1.9.1) Gecko/20090624 Firefox/3.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-GB; rv:1.9.1) Gecko/20090624 Firefox/3.5

If the button part of a file input control is clicked (or enter is pressed), the DOMActivate event is fired twice (presumably once for the child button control, and again for the parent input control). This does not occur if the text field is clicked on.

Reproducible: Always

Steps to Reproduce:
1. Load the testcase
2. Click the browse button
3. Choose a file
Actual Results:  
Two alerts boxes are posted, indicating the event is fired twice.

Expected Results:  
One alert box.

	Boris Zbarsky [:bzbarsky]
	Comment 2 • 15 years ago

smaug, any idea what's up here?

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 3 • 15 years ago

    We have two nested input elements (file and button) and
    nsHTMLInputElement::PostHandleEvent doesn't handle that case too well.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 5 • 15 years ago

Comment on attachment 404490 [details] [diff] [review]
proposed patch

>diff --git a/content/html/content/src/nsHTMLInputElement.cpp b/content/html/content/src/nsHTMLInputElement.cpp
>--- a/content/html/content/src/nsHTMLInputElement.cpp
>+++ b/content/html/content/src/nsHTMLInputElement.cpp
>@@ -1709,16 +1709,27 @@ nsHTMLInputElement::PostHandleEvent(nsEv
>       // If activate is cancelled, we must do the same as when click is
>       // cancelled (revert the checkbox to its original value).
>       if (status == nsEventStatus_eConsumeNoDefault)
>         aVisitor.mEventStatus = status;
>     }
>   }
> 
>   if (outerActivateEvent) {
>+    // ignore the activate event fired by the "Browse..." button
>+    // (file input controls fire their own) (bug 500885)
>+    if (mType == NS_FORM_INPUT_FILE) {
>+      nsCOMPtr<nsIContent> maybeButton =
>+        do_QueryInterface(aVisitor.mEvent->originalTarget);
>+      if (maybeButton && maybeButton->AttrValueIs(kNameSpaceID_None,
>+                                                  nsGkAtoms::type,
>+                                                  nsGkAtoms::button,
>+                                                  eCaseMatters))
>+        return NS_OK;
>+    }
Add a check that maybeButton is in native anonymous content.

	Geoff Lankow (:darktrojan) Assignee
	Comment 6 • 15 years ago

I can't seem to make my patch do what it's supposed to do any more. Does it work for anybody else?

	Geoff Lankow (:darktrojan) Assignee
	Comment 7 • 15 years ago

I've moved this further up the function and outside the |if (outerActivateEvent)|, and it works again. Still don't know why the original patch stopped working but I'm sure there's a logical explanation.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 8 • 15 years ago

Comment on attachment 413583 [details] [diff] [review]
patch v2

Could you write mochitest for this.
The test should check that one and only one DOMActivate is dispatched to <input type="file">

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 9 • 15 years ago

And sorry, I should have asked a testcase already earlier.

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 10 • 15 years ago

Or please inform me if you don't have time to write the testcase, then I'll
do it.

	Geoff Lankow (:darktrojan) Assignee
	Comment 11 • 15 years ago

I was getting there! Really wish I'd decided that I didn't have time though... this took WAY too long :(

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 12 • 15 years ago

Comment on attachment 414414 [details] [diff] [review]
patch with tests

Def want smaugs review on this though.

	Phil Ringnalda (:philor)
	Comment 13 • 15 years ago

http://hg.mozilla.org/mozilla-central/rev/0db0a2034733