184549 - mac: space no longer activates buttons in dialogs

Status: VERIFIED FIXED

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

Description

[sairuh (rarely reading bugmail)]

i've seen this for some, but couldn't find a bug on this, so pls dup as needed.
on mac, spacebar no longer activates the focused button in xul dialogs. this is
*not* a problem on linux (eg, rh8) or win2k. tested recently on Mac OS X 10.2.2
with 2002.12.09.07 macho moz bits.

to repro:

1. open a xul dialog containing buttons, such as Find in Page or even the
Mailnews Account Setup wizard.

2. use the tab key to move focus to any of the buttons --it doesn't matter if
it's the default button either.

3. hit the space key.

expected results: the space key should activate the focused button.

actual results: nothing happens --the focused button is not activated.

workaround: hitting the Return key will activate the focused button.

Comment 1

[sairuh (rarely reading bugmail)]

this is especially strange on Mac because the spacebar is typically used
throughout OS X to activate the focused button (and other ui elements), iirc.

Comment 2

[Simon Fraser [no longer active]]

It this because we're picking up Unix keybindings?

Comment 3

[sairuh (rarely reading bugmail)]

hm, that'd be strange, since this is working fine on linux.

Comment 4

[Aaron Leventhal]

.

Comment 5

[sairuh (rarely reading bugmail)]

kathy (or, others), how feasible would it be to get this fixed for mozilla1.3?

Comment 6

[sairuh (rarely reading bugmail)]

this a bit worse than i thought --there's a case where i cannot get a dialog
(sheet) to dismiss properly:

1. open an editor window.
2. make some changes.
3. close the window via cmd-W.
4. i don't want to save the changes, just close the window, so i hit the tab key
to focus the "Don't Save" button.
5. hit Return, since hitting the spacebar won't work.

results: rather than activating the "Don't Save" button which is in focus, the
default button ("Save" in this case) is incorrectly activated. if it's a new
file, i'm prompted for a title. if it's an existing file, when i reopen it, the
changes i didn't want (step 2) are there.

Comment 7

[Samir Gehani]

adt: nsbeta1+/adt2

Comment 8

[sairuh (rarely reading bugmail)]

nominating for 1.4final --see comment 6 for reasoning.

Comment 9

[Paul Wyskoczka]

ADT: Nominating topembed

Comment 10

[sairuh (rarely reading bugmail)]

this is also a problem under nscp7.02. odd, i thought it use to work at some
point...

Comment 11

[hacker formerly known as [e-mail]]

After poking around on this some, I'm wondering if there's something wrong with
the event handlers on mac.  I put some printfs in nsButtonBoxFrame::HandleEvent
(
http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsButtonBoxFrame.cpp#96
  ). I'm noticing that function is only seeing the NS_KEY_UP event message and
never the NS_KEY_DOWN event message when space is pressed.  This is bad since
the function will only simulate a mouse click on the key up of space if it saw
the key down event.  If you press return on the dialog button, that function
sees the key press & key up messages.

Comment 12

[Dave Barrowman]

topembed- since this is XUL, not embedding.  Reassigning to bryner.

Comment 13

[hacker formerly known as [e-mail]]

Hrm.  This particular problem is manifesting itself in xul but the root of the
issue seems to be that we never get any keyDown events from WaitNextEvent() at
nsMacMessagePump::GetEvent() ,
http://lxr.mozilla.org/seamonkey/source/widget/src/mac/nsMacMessagePump.cpp#386
.  That seems odd since we're passing |everyEvent| to WaitNextEvent as well as
setting the system event mask to grab every event.

Comment 14

[Simon Fraser [no longer active]]

That's probably because we get key downs through the text input event handler
(for IME). They should funnel through to the same code.

Comment 15

[Kathleen :Brade]

Attachment: patch that fixes this bug but needs cleanup

Comment 16

[Akkana Peck]

Any idea what caused this to regress?

Comment 17

[sairuh (rarely reading bugmail)]

since this was a problem under ns7.02, i don't this is really a regression.

i'll double-check with ns7.0, but i think this is a problem with OS X builds
(which first appeared with ns7.0). iirc, this wasn't a problem with ns7.0x on
Mac OS 9.x.

Comment 18

[sairuh (rarely reading bugmail)]

hrm, my memory is off. this didn't work on OS 9.x in either ns7.0 or ns7.02.
(still didn't work on OS X in ns7.0.)

ah, but it did work in ns6.2.3 on OS 9.x! based off of mozilla 0.9.4.1, from
2002.05.08 --wow, a *really* old regression. i see a few old OS X from around
that time period (various branch and trunk builds); will see if i can get a
regression window there (it might be huge, if at all).

Comment 19

[sairuh (rarely reading bugmail)]

regression info, based on commercial builds on OS X.

a. 6.2.3-rtm [2002.05.08, based on moz-0.9.4.1]: works fine; when i hit the
spacebar, the text on the button reverses (from black text to white) and the
button is activated.

b. 7.0-pr1 [2002.05.15.17, based on moz-1.0rc2]: works, but differs from (a).
while the button is activated, i no longer see the reversing black-to-white
behavior.

c. build 2002.08.05.05 [based on moz-1.0.1]: works just like (b).

d. build 2002.08.12.05 [still based on moz-1.0.1]: BROKEN, spacebar no longer
activates focused xul buttons.

okay, those above are branch builds. here are data on [supposed] trunk builds
which i had access to:

e. build 2002.08.07.08-trunk [trunk, moz-1.1b]: works like (b).

f. build 2002.08.26.10-1.1 [maybe a trunk build? based on moz-1.1]: BROKEN

Comment 20

[Greg K.]

See bug 148130 about keydown not working on Mac.

Comment 21

[Simon Fraser [no longer active]]

Comment on attachment 123892 [details] [diff] [review]
patch that fixes this bug but needs cleanup

We simulate key down and key press, so don't we need to simulate key up as
well?

Comment 22

[Kathleen :Brade]

We don't need to simulate keyup, at least not today.  The OS sends us those via
the normal route (gecko gets keyup event sent by HandleKeyEvent method).

-->brade

Comment 23

[Brian Ryner (not reading)]

Will this result in

<key down>
<key press>
<key down>
<key press>
...

if you hold down a key?  That would be wrong.

Comment 24

[Kathleen :Brade]

Attachment: patch v2 which addresses keyrepeat and keydown events

Comment 25

[Kathleen :Brade]

Attachment: patch v2 which addresses keyrepeat and keydown events

Comment 26

[Kathleen :Brade]

Comment on attachment 124001 [details] [diff] [review]
patch v2 which addresses keyrepeat and keydown events

seeking approval for 1.4final to land this fix for generating key down events
on OSX.  It regressed long ago when unicode entry was enabled.

Comment 27

[Asa Dotzler [:asa]]

Comment on attachment 124001 [details] [diff] [review]
patch v2 which addresses keyrepeat and keydown events

a=asa (on behalf of drivers) for checkin to 1.4

Comment 28

[Kathleen :Brade]

fix checked in for 1.4final

Comment 29

[sairuh (rarely reading bugmail)]

test cases from bug 148130:

http://bugzilla.mozilla.org/attachment.cgi?id=85839&action=view

http://bugzilla.mozilla.org/attachment.cgi?id=94030&action=view

Comment 30

[sairuh (rarely reading bugmail)]

spacebar does work (both modern and classic themes) to activate the focused
button. tested with 2003.05.23.06 comm bits on 10.2.6.

remaining issues:

bug 206917: unable to arrow around in XUL droplists (dropmenus). regression or
old bug uncovered?

bug 203734: XUL buttons no longer display focus ring.

Comment 31

[sairuh (rarely reading bugmail)]

vrfy'd fixed in trunk and branch (tho' it went in pre-branch).