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

RESOLVED DUPLICATE of bug 418091

Status

()

Core
Widget: Cocoa
--
major
RESOLVED DUPLICATE of bug 418091
13 years ago
6 years ago

People

(Reporter: mano, Unassigned)

Tracking

(Blocks: 2 bugs)

Trunk
All
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

The native mac menus code under widget/src/mac should honor the keycode
attribute (if there's no "key" attribute) of the <key> element.
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla1.9alpha
mano: Could you provide a reproducible example of this?
Priority: P2 → --
Whiteboard: [mano:cocoa-retarget]
Target Milestone: mozilla1.9alpha → ---

Comment 2

12 years ago
Dupe of bug 44259?
Assignee: mano → joshmoz
Status: ASSIGNED → NEW
Component: Widget: Mac → Widget: Cocoa
QA Contact: mac → cocoa
Whiteboard: [mano:cocoa-retarget]

Updated

11 years ago
Summary: Mac menus implementation doesn't honnor the "keycode" attribute → Mac menus implementation doesn't honor the "keycode" attribute
Duplicate of this bug: 482586
Hardware: PowerPC → All
Created attachment 366784 [details] [diff] [review]
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

Updated

9 years ago
Assignee: joshmoz → nobody
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.
Attachment #366784 - Flags: feedback?(joshmoz)

Comment 6

8 years ago
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.
Attachment #366784 - Flags: feedback?(joshmoz)
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.
(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.
(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
Duplicate of this bug: 669572
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!
Duplicate of this bug: 282091
Correct me if I'm wrong, but based on comment #4 I think this dupes to bug 418091.
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 418091
You need to log in before you can comment on or make changes to this bug.