284683 - Fix (XUL) open-menulist keyboard shortcuts on mac

Status: RESOLVED FIXED

(Core :: XUL, defect, P2)

Description

[Asaf Romano (gone)]

Here's what we do at the time of writing:
 1. Toogle the menu on F4 or alt+up/down arrow
 2. Cycle between the options (without opening the menu) on arrows.

On mac we should open (not toogle) the menu on either space or arrows, the
second behavior mentioned above doesn't exist on mac.

Comment 1

[Asaf Romano (gone)]

Attachment: patch

Comment 2

[jhp (no longer active)]

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

Looks good.  Is there a bug to fix this for web page dropdowns also?

And I noticed that if you drop down the menu, select a new item, then hit Esc,
it still selects a new item.  Don't know if that bug is cross-platform.

Comment 3

[Asaf Romano (gone)]

(In reply to comment #2)
> (From update of attachment 176190 [details] [diff] [review] [edit])
> Looks good.  Is there a bug to fix this for web page dropdowns also?

If not, I'll file
 
> And I noticed that if you drop down the menu, select a new item, then hit Esc,
> it still selects a new item.  Don't know if that bug is cross-platform.

hmm, I couldn't rero' w/ or w/o this patch.

Comment 4

[Peter Van der Beken [:peterv]]

Is this documented somewhere? I couldn't find a reference to this, and a quick
test with a couple of applications doesn't show this behaviour, instead both
arrows and spacebar do nothing on a closed menu afaict.

Comment 5

[Asaf Romano (gone)]

Peter, for me this works in any app. I'm on 10.3.

Also, please make sure the select element is actually focused...

Comment 6

[Asaf Romano (gone)]

(You need full-keyaboard-access on)

Comment 7

[Peter Van der Beken [:peterv]]

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

>+    // On mac, open menulist on either up/down arrow or unmodified space
>+    if (!IsOpen() && ((keyEvent->charCode == NS_VK_SPACE && !keyEvent->isMeta) ||

The comment and the code seem inconsistent, you only check for space without
meta.

Comment 8

[Asaf Romano (gone)]

w/ that fixed:
Checking in base/src/nsMenuFrame.cpp;
/cvsroot/mozilla/layout/xul/base/src/nsMenuFrame.cpp,v  <--  nsMenuFrame.cpp
new revision: 1.290; previous revision: 1.289
done