Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move keybinding dispatching off e.keyCode #17521

Closed
alexdima opened this issue Dec 19, 2016 · 4 comments · Fixed by #22894
Closed

Move keybinding dispatching off e.keyCode #17521

alexdima opened this issue Dec 19, 2016 · 4 comments · Fixed by #22894
Assignees
Labels
feature-request Request for new features or functionality
Milestone

Comments

@alexdima
Copy link
Member

alexdima commented Dec 19, 2016

Intro

VS Code dispatches keyboard shorcuts to commands by listening to keydown events.

At the time VS Code was first released, we shipped with Chromium version 45. It did not support KeyboardEvent.key, nor KeyboardEvent.code (see the current spec here).

It supported KeyboardEvent.keyCode, KeyboardEvent.charCode, KeyboardEvent.which and KeyboardEvent.keyIdentifier:

  • KeyboardEvent.charCode is often not set for keydown events, therefore unreliable.
  • KeyboardEvent.which was, from my testing, always equal to KeyboardEvent.keyCode.
  • KeyboardEvent.keyIdentifier was known to be severely flawed already at that time, and it was marked as deprecated (it got removed in Chromium version 54).
  • we were therefore stuck with using KeyboardEvent.keyCode for dispatching keyboard shortcuts.

What is wrong with KeyboardEvent.keyCode and why is it deprecated?

The w3c spec is dry (which is a good thing in general, maybe not in this case), and does not explain what went wrong or why it is now deprecated.
In this section, after having browsed through Chromium's source code, I will try to explain what I think is wrong with it.

To understand the shortcomings of KeyboardEvent.keyCode, we must first understand what its value means and where it comes from. The only reasonable explanation I could find is that it must be rooted in how Windows does keyboard input.

I've found a good explanation about keyboard input in Windows here.

Assigned to each key on a keyboard is a unique value called a scan code, a device-dependent identifier for the key on the keyboard. A keyboard generates two scan codes when the user types a key—one when the user presses the key and another when the user releases the key.

The keyboard device driver interprets a scan code and translates (maps) it to a virtual-key code, a device-independent value defined by the system that identifies the purpose of a key.

... and a hint to how keyboard layouts work:

A keyboard layout not only specifies the physical position of the keys on the keyboard but also determines the character values generated by pressing those keys. Each layout identifies the current input language and determines which character values are generated by which keys and key combinations.

So a keyboard layout on Windows consists of two mappings. The first one maps scan codes to virtual keys and the second one maps virtual keys and modifiers combinations to generated characters. The list of Virtual Key Codes is defined here.

Since this is pretty abstract, let's pick an example where we compare the US standard keyboard layout with the GER (Germany) keyboard layout, as it can exemplify both mappings:

Standard US GER (Germany)
image image
Physical Key Scan Code US layout GER layout Notes
Key Code Char Key Code Char
Y 21 89 (0x59) y 90 (0x5A) z The first mapping (scan code -> key code) is changed, and the second mapping (key code -> character) is identical.
Z 44 90 (0x5A) z 89 (0x59) y
7 8 55 (0x37) 7 55 (0x37) 7 The first mapping (scan code -> key code) is identical, and the second mapping (key code -> character) is changed.
Shift+7 8 55 (0x37) & 55 (0x37) /

Turns out the indirection through Virtual Key Codes is quite helpful, e.g. one can write an application and simply look for Ctrl+0x5A (i.e. Ctrl+Z) keydown events and this will work as expected on keyboards where keys are moved around, e.g. QWERTZ or AZERTY keyboards, etc.

By looking for KeyboardCodeFromNative in Chromium's sources we can confirm that under Windows, KeyboardEvent.keyCode is simply equal to the Windows Virtual Key Code:

KeyboardCode KeyboardCodeFromNative(const base::NativeEvent& native_event) {
  return KeyboardCodeForWindowsKeyCode(static_cast<WORD>(native_event.wParam));
}
// ...
KeyboardCode KeyboardCodeForWindowsKeyCode(WORD keycode) {
  return static_cast<KeyboardCode>(keycode);
}

The problem: Linux and Mac

AFAICT keyboard input and keyboard layouts on Linux and Mac do not work through this double indirection (from scan code to key code, and from key code and modifiers to character), apparently keyboard input on Linux and Mac goes straight from scan code and modifiers to characters.

This raises the question, what then is the value of KeyboardEvent.keyCode on Linux and Mac?

By looking for the Linux and Mac equivalents of KeyboardCodeForWindowsKeyCode (i.e. KeyboardCodeFromXKeyEvent and KeyboardCodeFromNSEvent) we can conclude that nobody really knows.

On Linux:

// Get an ui::KeyboardCode from an X keyevent
KeyboardCode KeyboardCodeFromXKeyEvent(const XEvent* xev) {
  // Gets correct VKEY code from XEvent is performed as the following steps:
  // 1. Gets the keysym without modifier states.
  // 2. For [a-z] & [0-9] cases, returns the VKEY code accordingly.
  // 3. Find keysym in map0.
  // 4. If not found, fallback to find keysym + hardware_code in map1.
  // 5. If not found, fallback to find keysym + keysym_shift + hardware_code
  //    in map2.
  // 6. If not found, fallback to find keysym + keysym_shift + keysym_altgr +
  //    hardware_code in map3.
  // 7. If not found, fallback to find in KeyboardCodeFromXKeysym(), which
  //    mainly for non-letter keys.
  // 8. If not found, fallback to find with the hardware code in US layout.
  // ...
}

On Mac:

KeyboardCode KeyboardCodeFromNSEvent(NSEvent* event) {
  // ...
  NSString* characters = [event characters];
  if ([characters length] > 0)
    code = KeyboardCodeFromCharCode([characters characterAtIndex:0]);
  // ...
  characters = [event charactersIgnoringModifiers];
  if ([characters length] > 0)
    code = KeyboardCodeFromCharCode([characters characterAtIndex:0]);
  // ...
}

// Translates from character code to keyboard code.
KeyboardCode KeyboardCodeFromCharCode(unichar charCode) {
  switch (charCode) {
    // ...
    // U.S. Specific mappings.  Mileage may vary.
    case ';': case ':': return VKEY_OEM_1;
    case '=': case '+': return VKEY_OEM_PLUS;
    case ',': case '<': return VKEY_OEM_COMMA;
    case '-': case '_': return VKEY_OEM_MINUS;
    case '.': case '>': return VKEY_OEM_PERIOD;
    case '/': case '?': return VKEY_OEM_2;
    case '`': case '~': return VKEY_OEM_3;
    case '[': case '{': return VKEY_OEM_4;
    case '\\': case '|': return VKEY_OEM_5;
    case ']': case '}': return VKEY_OEM_6;
    case '\'': case '"': return VKEY_OEM_7;
  }

  return VKEY_UNKNOWN;
}

The way KeyboardEvent.keyCode is computed leads to some weird situations, such as the one documented in issue #1302:

Given the German (Swiss) keyboard layout, ... it is very difficult to figure out what actual physical key was pressed. E.g.:

  • when pressing 7, the keydown event we receive has keyCode:55
  • this is OK
  • when pressing Shift+7, the keydown we receive has shiftKey:true and keyCode: 191
  • this is unexpected, we should receive shiftKey:true and keyCode:55

Here are the values observed in KeyboardEvent:

Physical Key US layout Win US layout Mac GER layout Win GER layout Mac
Y
shift: false
keyCode: 89
shift: false
keyCode: 89
shift: false
keyCode: 90
shift: false
keyCode: 90
Z
shift: false
keyCode: 90
shift: false
keyCode: 90
shift: false
keyCode: 89
shift: false
keyCode: 89
7
shift: false
keyCode: 55
shift: false
keyCode: 55
shift: false
keyCode: 55
shift: false
keyCode: 55
Shift+7
shift: true
keyCode: 55
shift: true
keyCode: 55
shift: true
keyCode: 55
shift: true
keyCode: 191

This is probably the case from above where "mileage may vary". We have tried to workaround this on our side, but the workaround is limited and only functions for a subset of these key codes (i.e. does not work, perhaps it is even making things worse for DVORAK).

Issues that possibly all share the same root cause (the fuzzyness of keyCode on Linux and Mac and/or our workaround not working or making things worse):

The path forward

We can now use KeyboardEvent.code and KeyboardEvent.key (see the current spec here). The simplified explanation is that KeyboardEvent.code is a string representation of the scan code, and KeyboardEvent.key is the produced character (skipping entirely the Virtual Keys hop on Windows). This has a chance to work correctly because all Operating Systems have these concepts.

Physical Key US layout Win US layout Mac GER layout Win GER layout Mac
Y
shift: false
code: "KeyY"
key: "y"
shift: false
code: "KeyY"
key: "y"
shift: false
code: "KeyY"
key: "z"
shift: false
code: "KeyY"
key: "z"
Z
shift: false
code: "KeyZ"
key: "z"
shift: false
code: "KeyZ"
key: "z"
shift: false
code: "KeyZ"
key: "y"
shift: false
code: "KeyZ"
key: "y"
7
shift: false
code: "Digit7"
key: "7"
shift: false
code: "Digit7"
key: "7"
shift: false
code: "Digit7"
key: "7"
shift: false
code: "Digit7"
key: "7"
Shift+7
shift: true
code: "Digit7"
key: "&"
shift: true
code: "Digit7"
key: "&"
shift: true
code: "Digit7"
key: "/"
shift: true
code: "Digit7"
key: "/"
@alexdima alexdima added the feature-request Request for new features or functionality label Dec 19, 2016
@alexdima alexdima added this to the January 2017 milestone Dec 19, 2016
@alexdima alexdima self-assigned this Dec 19, 2016
@alexdima
Copy link
Member Author

alexdima commented Dec 19, 2016

[This comment will be updated to reflect the status of this issue]

Additional constraints

We cannot dispatch based on e.key. Even if we can overcome the missmatch between modifier flags and the value of e.key, the real problem lies within Chromium's handling of dead keys (i.e. keys producing combining accents). All keydown events for such keys result in e.key = "Dead". This would mean that these key combinations would be completely un-mappable in VS Code, which is unacceptable.

e.g. for the dead key problem on the GER Windows layout:

Physical Key US layout Win GER layout Win
Ctrl+Equal ctrl: true, code: "Equal", key: "=" ctrl: true, code: "Equal", key: "Dead"

After examining multiple applications (e.g. Visual Studio) on Windows, we have concluded that VS Code's keybinding dispatching on Windows should continue using e.keyCode (i.e. virtual keys). That is consistent with all other Windows Desktop applications.

[23.03.2017]

What changed

on Windows

  • keybinding dispatching remains on e.keyCode (i.e. virtual key codes).
  • we now detect keyboard layout changes and react appropiately.
  • all in all, there should be no observable change on Windows.

on OSX and Linux

  • keybinding dispatching moves to e.code (i.e. scan codes).
  • we now detect keyboard layout changes on OSX, no reasonable solution was found on Linux.
  • core actions and extensions continue to define keybindings on virtual key codes.
  • existing keybindings in user keybindings.json, along with core actions and extensions will be mapped at runtime, using heuristics, and depending on the current keyboard layout, to scan codes.
  • user keybindings can now be defined using a scan code format.
  • US standard keyboard layout users should observe no change.

A note on how the changes are implemented

The common type that can encapsulate all these cases is ResolvedKeybinding. It is an object that is constructed taking into account the current keyboard layout, and it should never be stored by consumers.

All keybindings go through an IKeyboardMapper, which is the only authorative producer of ResolvedKeybinding:

  • keybindings defined in core actions and keybindings defined by extensions go through IKeyboardMapper.resolveKeybinding, which provides a 1:N mapping (N>=0).
  • keyboard events go through IKeyboardMapper.resolveKeyboardEvent, which provides a 1:1 mapping.
  • user defined keybindings go through IKeyboardMapper.resolveUserBinding, which provides a 1:N mapping (N>=0).

A note on mapping heuristics

Here is an example for how the scan code Digit7 is interpreted under the GER (Swiss) keyboard layout on Linux.

These are the raw mappings read by native-keymap:

Digit7: {
    value: '7',
    withShift: '/',
    withAltGr: '|',
    withShiftAltGr: '⅞'
}
Scan Code combination Key KeyCode combination Alternative KeyCode combination
Digit7 7 7
Ctrl+Digit7 7 Ctrl+7
Shift+Digit7 / Shift+7 /
Ctrl+Shift+Digit7 / Ctrl+Shift+7 Ctrl+/
Alt+Digit7 7 Alt+7
Ctrl+Alt+Digit7 | Ctrl+Alt+7 Shift+\
Shift+Alt+Digit7 / Shift+Alt+7 Alt+/
Ctrl+Shift+Alt+Digit7 Ctrl+Shift+Alt+7 Ctrl+Alt+/

Explanation:

  • because Shift+Digit7 produces /, which is a character of interest (/):
    • Shift+Digit7 => /
    • Ctrl+Shift+Digit7 => ctrl+/
    • Shift+Alt+Digit7 => alt+/
    • Ctrl+Shift+Alt+Digit7 => ctrl+alt+/
  • because Ctrl+Alt+Digit7 produces |, which is a character of interest (shift+\\):
    • Ctrl+Alt+Digit7 => shift+\\

E.g. This effectively causes the Toggle Line Comment (bound to Ctrl+/ in core) to be bound to Ctrl+Shift+Digit7 under this keyboard layout.


A note on the new user keybindings.json scan code syntax

To distinguish from the existing KeyCode bindings, scan code bindings have the following shape. e.g.:

  • ctrl+[KeyY]
  • ctrl+[Digit1]
  • ctrl+shift+[Slash]
  • the full list of scan codes can be found here, but not all are supported in VS Code.

A note on troubleshooting

There is a new action: Developer: Inspect Key Mappings that will print all the generated tables based on the raw mappings read from the OS.

@ottonascarella
Copy link

using visual code with a japanese layout has been madness for me.
i would LOVE to see that change, thanks!

@Gozala
Copy link

Gozala commented Feb 23, 2017

Any chance a setting to use new keybinding dispatch could be either released as an add-on or a setting until the support for multiple shortcut syntax can be implemented and shipped. With non US layout on non-windows editor just behaves extremely odd. For example on dvorak cmd+, closes the the editor instead of opening preferences.

@Kobold
Copy link

Kobold commented Mar 6, 2017

Dvorak on mac is rough, I edited the raw js to disable the custom mac keycode handler. Cheering y'all on to get this resolved!

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants