300227 - enter always fires default button

Status: VERIFIED FIXED

(Core :: XUL, defect)

Description

[Andrew Schultz]

bug 298894 made [enter] fire the default button on mac, regardless of focus, but
it also changed all platforms to that behavior.  It is the wrong behavior for
all other platforms.

Comment 1

[[e-mail]]

Attachment: Proposed patch

I first wrote this patch to fix bug 98070, and I've been running with it ever
since; what I don't know is why I failed to attach it to that bug back then.

Comment 2

[Ed Lee :Mardak]

I was about to file a bug reporting pressing enter on the password dialog
defaults to yes, but I see there already are comments in Bug 263532. I went
looking for other dialog boxes to test out the "default button on enter" issue,
and it does indeed fire the default.

Should dataloss be added to the keywords? Pressing enter while the cancel button
is selected for sanitize triggers the default action (sanitizing).

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050728 Firefox/1.0+

Comment 3

[Ed Lee :Mardak]

It seems like the default button is *not* fired when pressing enter.. even on a mac.

This picture of the new password dialog [1] shows that the default button is
indeed "Not Now" (the blue shaded button) as well as the default selected button
is "Not Now" (the blue outline).

The default button is set correctly to "Not Now" (or at least rendered
correctly), but pressing enter triggers the rightmost (position 0) button.

[1] http://www.squarefree.com/burningedge/password-after.png

Comment 4

[Asaf Romano (gone)]

filed bug 304879 on comment 3.

Comment 5

[Johnny Stenback (:jst)]

Comment on attachment 188829 [details] [diff] [review]
Proposed patch

r=jst

Comment 6

[Boris Zbarsky [:bzbarsky]]

I won't be able to really look for at least a week...  I think bryner's sr
should be quite sufficient to land this with.  ;)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 188829 [details] [diff] [review]
Proposed patch

So the idea here is that dispatch to the frame should be an at-target handler
in the system event group, right?  And that we can't quite do that yet, but
this is close?

Please add a comment to that effect before checking in...

Get this in on trunk and request 1.8b4 approval, please?

Comment 8

[Pablo Greco]

cc=[e-mail] (because of 304453)

Comment 9

[[e-mail]]

Fix checked in to the trunk; waiting for it to bake before requesting approval.

Comment 10

[Brian Ryner (not reading)]

Comment on attachment 188829 [details] [diff] [review]
Proposed patch

r=me for what it's worth

Comment 11

[Serge Gautherie (:sgautherie)]

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

Comment 12

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050901 SeaMonkey/1.1a]
(nightly) (W98SE)

V.Fixed. (I checked with bug 302571 steps)

Comment 13

[[e-mail]]

Comment on attachment 188829 [details] [diff] [review]
Proposed patch

No new regressions introduced by this regression fix :-)