294437 - Mac menus implementation doesn't honor the "keycode" attribute

Status: RESOLVED DUPLICATE of bug 418091

(Core :: Widget: Cocoa, defect)

Description

[Asaf Romano (gone)]

The native mac menus code under widget/src/mac should honor the keycode
attribute (if there's no "key" attribute) of the <key> element.

Comment 1

[jhp (no longer active)]

mano: Could you provide a reproducible example of this?

Comment 2

[Karsten Düsterloh]

Dupe of bug 44259?

Comment 4

[Mark Banner (:standard8)]

Attachment: Investigation patch

I think what this bug is saying, is that on Mac we don't set the key equivalent for key elements that don't have a key set, but have a keycode set.

So for example we have the Delete menu item (under Edit) set up for VK_DELETE (or VK_BACK in TB), but the delete icon doesn't display as a short cut for that menu item, even though it is.

This hurts discovery of short cuts, especially for some actions (e.g. send) in Thunderbird.

I did some investigation and found where we set up the existing keys in nsMenuItemX.mm. A little more work and I found I could retrieve the keycode that was set on the element.

Here comes the problem, the keycode is a string like "VK_DELETE" but we need an actual character code. So for purposes of demonstration I did a set of if (keycode = "VK_DELETE") etc.

The only place I can see in the code there is a map of strings to char codes is nsXBLPrototypeHandler.cpp [1] which obviously we can't get at.

Therefore, anyone have any thoughts about how we can do this efficiently? Or indeed is there an existing way I haven't found translate the code?

I'd prefer not to do this manually or copy code, so does anyone have any ideas?

[1] http://mxr.mozilla.org/comm-central/source/mozilla/content/xbl/src/nsXBLPrototypeHandler.cpp#649

Comment 5

[Mark Banner (:standard8)]

Comment on attachment 366784 [details] [diff] [review]
Investigation patch

This patch (amazingly) still applies, though I've not actually tested it on the latest builds.

I'd still like to get feedback on suggestions for converting the strings that I mentioned in the previous comment.

Comment 6

[Josh Aas]

Comment on attachment 366784 [details] [diff] [review]
Investigation patch

I'm not opposed to doing this sort of thing so long as it isn't a major compatibility problem. And if it is, maybe we can just do this in mozilla-central.

We have the reverse of the type of mapping you want in Cocoa widgets. See:

static PRUint32 GetGeckoKeyCodeFromChar(PRUnichar aChar)

If necessary you could move that to a utility file and add the type of mapping you need. I don't know the best thing to do about this though.

Comment 7

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

This patch is not working for me unless at some point there is a

  [mNativeMenuItem setKeyEquivalentModifierMask:NSFunctionKeyMask];

that is currently missing. If it's not here, NS_VK_F2 is for instance
shown as Q...

And I have the feeling this patch should not directly translate from keycode
if there is a keytext attribute present. I may try something here.

Comment 8

[Mark Banner (:standard8)]

(In reply to comment #7)
> This patch is not working for me unless at some point there is a
> 
>   [mNativeMenuItem setKeyEquivalentModifierMask:NSFunctionKeyMask];
> 
> that is currently missing. If it's not here, NS_VK_F2 is for instance
> shown as Q...

I guess that may have changed then, or may be needed on newer Mac OS X (I originally did the patch on 10.5).

> And I have the feeling this patch should not directly translate from keycode
> if there is a keytext attribute present. I may try something here.

Note also that it may now be possible to share the code in a sensible manner from nsXBLPrototypeHandler.cpp - now that everything is libxul based, we don't have the shared library boundaries that we did before.

I'd really love to see this fixed, just unfortunately I don't have time to work on it at the moment.

Comment 9

[Daniel Glazman (:glazou) (not active in Mozilla any more)]

(In reply to comment #8)

> > And I have the feeling this patch should not directly translate from keycode
> > if there is a keytext attribute present. I may try something here.
> 
> Note also that it may now be possible to share the code in a sensible manner
> from nsXBLPrototypeHandler.cpp - now that everything is libxul based, we
> don't have the shared library boundaries that we did before.
> 
> I'd really love to see this fixed, just unfortunately I don't have time to
> work on it at the moment.

Ok, OS X only deals with one char for the shortcut. So keytext is pointless
on Mac unless we take only its first char. Given the fact the Unicode
equivalents for all special keys are available [1], it could make sense.
In that case, XUL authors could use the keytext attribute with a preprocessor
#ifdef XP_MACOSX... And if there is no keytext, default to an automatic
translation of keycode.

[1] http://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSEvent_Class/Reference/Reference.html#//apple_ref/doc/uid/20000016-SW136

Comment 11

[Rob Campbell [:rc] (:robcee)]

copied from bug 669572:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:7.0a1) Gecko/20110705 Firefox/7.0a1

Bug 663746 changed Scratchpad's shortcut key from F4 to Shift+F4 for all platforms. On Mac OS X the shortcut key for Scratchpad is not listed in the Web Developer Menu near the item.

Reproducible: always

(note, the keyboard shortcut of F4 never showed on the Mac's menus either).

Invisible shortcuts!

Comment 13

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Correct me if I'm wrong, but based on comment #4 I think this dupes to bug 418091.