Skip to content
This repository has been archived by the owner on Mar 5, 2023. It is now read-only.

KeyCode string for KeyEquivalent depends of the current keyboard layout #60

Closed
stel opened this issue Jan 27, 2015 · 8 comments
Closed

Comments

@stel
Copy link
Contributor

stel commented Jan 27, 2015

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.

stel added a commit to stel/MASShortcut that referenced this issue Jan 27, 2015
@zoul
Copy link
Collaborator

zoul commented Jan 28, 2015

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
Copy link
Contributor Author

stel commented Jan 28, 2015

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)
screen shot 2015-01-28 at 15 23 31
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
Copy link
Collaborator

zoul commented Jan 28, 2015

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:

animated screenshot of the changing system shortcuts

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

@stel
Copy link
Contributor Author

stel commented Jan 28, 2015

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
Copy link
Collaborator

zoul commented Jan 28, 2015

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.

  1. 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.

  2. 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 Shortcut validation doesn’t consider different keyboard layouts #62.

@zoul
Copy link
Collaborator

zoul commented Jan 28, 2015

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

zoul pushed a commit that referenced this issue Jan 28, 2015
See #60 for a discussion. In short, keyCodeStringForKeyEquivalent
should be now correct even with non-ASCII keyboard layouts such as
Russian.
@stel
Copy link
Contributor Author

stel commented Jan 28, 2015

Thanks 👍

@zoul
Copy link
Collaborator

zoul commented Jan 28, 2015

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

@zoul zoul closed this as completed Jan 28, 2015
stephenkwagner added a commit to TechSmith/ShortcutRecorder that referenced this issue Aug 26, 2020
TLDR Use TISCopyCurrentASCIICapableKeyboardLayoutInputSource instead of TISCopyCurrentASCIICapableKeyboardInputSource

This fixes the issues with shortcuts not appearing correctly in Japanese.  I am not sure what the difference is but:
   1.  We were using TISCopyCurrentASCIICapableKeyboardLayoutInputSource elsewhere in our code for the recorder shortcuts and that has been working.
   1.  MASSShortcut uses TISCopyCurrentASCIICapableKeyboardLayoutInputSource cocoabits/MASShortcut#60
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants