KeyCode string for KeyEquivalent depends of the current keyboard layout

[stel]

When switching to non US (non-ASCII capable) keyboard layout -keyCodeStringForKeyEquivalent returns string with non-ASCII character, if a printable character was used in shortcut. But this character couldn't be used for keyEquivalent of the NSMenuItem.

I suppose it is better to use TISCopyCurrentASCIICapableKeyboardLayoutInputSource instead of TISCopyCurrentKeyboardLayoutInputSource in -keyCodeString of MASShortcut.

[zoul]

This is an interesting challenge. As I understand the issue, the core problem is this: On different keyboard layouts (such as US and Czech), a single shortcut (a combination of physical keys) can have different “names”.

For example, the default system shortcut for toggling directly to space no. 2 is Control–2. But when you switch to the Czech keyboard layout, the physical key with the 2 label now inserts the ě character. Thus, on most keyboard layouts the shortcut for toggling to space no. 2 is ^2, but on the Czech layout it’s ^ě. (I stress that this is the same combination of hardware keys.)

This is reflected by the system: When you open the System Preferences → Keyboard → Shortcuts pane, the shortcuts displayed depend on the currently selected keyboard layout (try switching between the US and Czech keyboard layouts and reopening the preference pane).

MASShortcut seems to handle this exactly like system does: When you run the demo with different keyboard layouts, you may get different shortcut strings in the recording control (even in the current version). But when I try to assign a shortcut like ^Ř to an NSMenuItem’s keyEquivalent, the item ignores the equivalent (^Í works, though):

// Try this with the Czech keyboard layout active
MASShortcut *shortcut = [MASShortcut shortcutWithKeyCode:kVK_ANSI_5 modifierFlags:NSControlKeyMask];
[menuItem setKeyEquivalentModifierMask:[shortcut modifierFlags]];
[menuItem setKeyEquivalent:[shortcut keyCodeStringForKeyEquivalent]];

That’s a problem. On the other hand, setting a key equivalent to ^Ř works like charm in the Interface Builder and even lets me pick between alternatives (^Ř, 5). Exploring further, the ^Ř key equivalent set through Interface Builder is somehow lost when the app runs and the shortcut doesn’t do anything. ^Í is not cleared, but doesn’t work either. ^U is not cleared and works.

The proposed switch to TISCopyCurrentASCIICapableKeyboardLayoutInputSource doesn’t seem to change anything, weird. I’m not even sure yet what the desired behaviour should be here, I’ll think about it. (My current line of thought is dropping keyCodeStringForKeyEquivalent from MASShortcut entirely, leaving just keyCodeString that changes based on the current keyboard layout, and moving the key equivalent matching code to MASShortcutValidator.)

[stel]

Can't reproduce the behaviour you describe. In my case even if I change the system language (and keyboard layout too of course) to Czech I have US letters in shortcuts (the only exception is a space character – " " – byt it is localized by the system) So in my case no matter the layout or even system language I use I always have an US (A-Z) characters in menus etc (but I only tested in on Yosemite). That's why I suppose to use TISCopyCurrentASCIICapableKeyboardLayoutInputSource which always returns the ASCII layout which gives the A-Z characters.

[zoul]

You have to watch a keyboard shortcut with a key that’s different on the two keyboard layouts. Here’s is an animated screenshot – one frame with the US keyboard layout, the other one with the CS layout:

All that’s needed is to switch to the previous tab (Text), switch keyboard layout and switch back to the Shortcuts tab.

[stel]

Interesting.. got it with 1,2 etc, but what about A-Z?

Nevertheless TISCopyCurrentASCIICapableKeyboardLayoutInputSource gives + (works in menu item) and ě (doesn't) for 1 and 2 (try my pull request).

In my case the main problem is A-Z characters with different layout, in Russian layout for example ^U becomes ^Г with TISCopyCurrentKeyboardLayoutInputSource but not with TISCopyCurrentASCIICapableKeyboardLayoutInputSource. In system preferences there is always ^U.

[zoul]

Current summary:

1) Shortcut recording and watching works correctly, even for problematic shortcuts such as ^Ř. This seems to be mostly a presentation problem.

2) The value returned by keyCodeString and keyCodeStringForKeyEquivalent depends on the current keyboard layout. This is probably the desired behaviour (even system does it like that), but it’s a nasty surprise in the API. The least we could do is to document it (done in 38fe428). The “correct” design could be to separate the rendering of shortcuts into a separate class and make the dependency on current keyboard layout explicit? Something like:

MASShortcutFormatter *formatter =  [MASShortcutFormatter new];
[formatter setKeyboardLayout:[MASKeyboardLayout currentLayout]];
NSString *keyCodeString = [formatter keyCodeStringForShortcut:someShortcut];

This would place all the code responsible for formatting shortcuts in one class and if we could pass an explicit keyboard layout (like US or CS), we could add automatic tests for shortcut rendering (which is not currently possible since the code depends on the current keyboard layout). [MASShortcut keyCodeString] could be deprecated or use a shared default formatter. Reported as #63.

3) Setting the shortcut as a key equivalent of an NSMenuItem is not always possible, see the ^Ř case. This doesn’t seem to be our fault, it appears to be a limitation of NSMenuItem.

4) Given the points above, I’m not sure if the code that checks for shortcuts already used by system is entirely correct. In particular, the isShortcut:alreadyTakenInMenu:explanation: method in the MASShortcutValidator class uses key equivalent string to compare shortcuts, which is not always correct. Test case: Assign the ^2 shortcut to a menu item and switch to the Czech keyboard layout. Now trying to record a ^ě shortcut will pass the validation, although the shortcuts have the same key code and modifiers and are in conflict. Reported as #62.

[zoul]

I’ve just read your last comment, @stel. Thank you, that’s a clear argument in favour of the “ASCII-capable” API call.

[stel]

Thanks :+1:

[zoul]

I have created standalone issues #62 and #63 for the related problems, will close this.