623291 - Firefox 4: change/blur event are not dispatched to input upon button click

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Alessandro Vernet]

User-Agent:       Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7) Gecko/20100101 Firefox/4.0b7
Build Identifier: 4b8

-

Reproducible: Always

Steps to Reproduce:
1. Click on the input field, and enter some text
2. Click on the "nothing" button
Actual Results:  
On Firefox 4b8, the change and blur events are not dispatched.
(When the change/blur listener are called, alerts are shown.)
It is called on Firefox 3.6, Safari 5, and just any other browser.

Expected Results:  
The change/blur events should be dispatched.

Note this is related by not different than Bug 577316.
The issue here is not that the button doesn't get the focus,
but that the change event is not dispatched to input field.

Also related, at the UX level, on Safari 5 the input looses the
focus when clicking on the button, even if the button doesn't
acquire the focus. This is not the case on Firefox 4b8.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

A testcase would be great.

We need to fix this for FF4.

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

I wonder if we end up switching accessibility.mouse_focuses_formcontrol
on OSX for FF4 to get the old behavior.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

(In reply to comment #1)
> A testcase would be great.

A *minimal* testcase, attached using "Add an attachment"

Comment 4

[Neil Deakin]

Could you be more specific? The link you specified doesn't demonstrate any bug. 

Not firing the change or blur events when clicking the button is correct behaviour, as the focus does not change when clicking buttons on Mac.

Comment 5

[Alessandro Vernet]

(In reply to comment #1)
> A testcase would be great.

To just run the example (not see the source in jsFiddle), you can use: http://fiddle.jshell.net/avernet/Zm7Fx/show/light/

Comment 6

[Alessandro Vernet]

(In reply to comment #4)
> Could you be more specific? The link you specified doesn't demonstrate any bug. 
> 
> Not firing the change or blur events when clicking the button is correct
> behaviour, as the focus does not change when clicking buttons on Mac.

Try the same example on Safari 5. You'll notice that the button doesn't get the focus, but that the input looses the focus, and that the blur/change events are dispatched to the input (as they are in any other browser).

Comment 7

[Neil Deakin]

OK, so what you're actually asking for is for the focus to be removed from existing elements when buttons are clicked.

Comment 8

[Alessandro Vernet]

(In reply to comment #7)
> OK, so what you're actually asking for is for the focus to be removed from
> existing elements when buttons are clicked.

That's right, and consequently for the blur/change events to be dispatched accordingly.

(This would be consistent with the event behavior in Safari, which seems to be the direction Firefox is going to on OS X, at least as far as the default behavior is concerned.)

(This would also be "more compatible" with what happens in Firefox 3.6 and other browsers. Imagine that you have JavaScript that validates form fields on "change": with 4b8 that validation code won't run if users change a field and click on a button, while it was running in 3.6, and is running with Safari 5 and other browsers.)

Comment 9

[Neil Deakin]

OK this is because at http://mxr.mozilla.org/mozilla/source/content/html/content/src/nsHTMLButtonElement.cpp#417 , the event prevents default handling.

This has always been the case for <button>, but, oddly, not for <input type="button">. A similar test using <input type="button"> works as expected here. 

Olli, do you think it makes sense to just remove the event canceling line linked to above?

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

Attachment: nested buttons test

Comment 11

[Olli Pettay [:smaug][[e-mail]]]

Need to test still <a><button>foo</button></a>

Comment 12

[Olli Pettay [:smaug][[e-mail]]]

Copying from IRC:

I think http://mxr.mozilla.org/mozilla/source/content/html/content/src/nsHTMLButtonElement.cpp#417 could be replaced with
aVisitor.mEvent->flags |= NS_EVENT_FLAG_PREVENT_ANCHOR_ACTIONS

Comment 13

[Boris Zbarsky [:bzbarsky]]

Sounds related to bug 618414, for what it's worth.  <a> and <button> both have the weird hackery there, but <input type="button"> does not.

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

I'll upload a minimal testcase.

Comment 15

[Olli Pettay [:smaug][[e-mail]]]

Attachment: NS_EVENT_FLAG_PREVENT_ANCHOR_ACTIONS

Uploaded to tryserver

Comment 16

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

I guess we should do this for FF4.

Comment 17

[Neil Deakin]

Comment on attachment 502814 [details] [diff] [review]
patch

OK, but you should be able to put all the code in runTests without the need to use separate 'next' and 'done' methods.

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

Well, next() and done() helped with debugging, and I don't see reason not
to use them.

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

http://hg.mozilla.org/mozilla-central/rev/313df704acf7

Comment 20

[Alessandro Vernet]

Olli, awesome, thank you!