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

Keybinding for "Toggle Integrated Terminal" keeps resetting to Ctrl+^ #26506

Closed
bpasero opened this issue May 12, 2017 · 4 comments
Closed

Keybinding for "Toggle Integrated Terminal" keeps resetting to Ctrl+^ #26506

bpasero opened this issue May 12, 2017 · 4 comments
Assignees
Labels
info-needed Issue requires more information from poster upstream Issue identified as 'upstream' component related (exists outside of VS Code)
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented May 12, 2017

I am on macOS with a german keyboard layout and I want to open the integrated terminal with Ctrl+<the key above Alt key>. For that I am assigning that key and the output I see is Ctrl+[Backquote]. When I set this it seems to work until I reload the window where it then stops to work.

@bpasero bpasero changed the title Keybinding for "Toggle Integrated Terminal" keeps resetting to Ctrl Keybinding for "Toggle Integrated Terminal" keeps resetting to Ctrl+^ May 12, 2017
@alexdima
Copy link
Member

@bpasero Just had the same conversation two days ago with @joaomoreno :)

Most likely, here is what is happening:

  • your laptop has an ANSI built-in keyboard
  • you use an external ISO keyboard
  • you type on the external ISO keyboard

The root cause

Chromium flips [Backquote] and [IntlBackslash] on ISO keyboards. This is the root cause of this issue and IMHO the only correct fix would be to ask the Electron folks to remove these horrible lines via a Chromium patch on their side:

https://cs.chromium.org/chromium/src/ui/events/keycodes/keyboard_code_conversion_mac.mm?type=cs&q=LMGetKbdType+package:%5Echromium$&l=809

int ISOKeyboardKeyCodeMap(int nativeKeyCode) {
  // OS X will swap 'Backquote' and 'IntlBackslash' if it's an ISO keyboard.
  // https://crbug.com/600607
  switch (nativeKeyCode) {
    case kVK_ISO_Section:
      return kVK_ANSI_Grave;
    case kVK_ANSI_Grave:
      return kVK_ISO_Section;
    default:
      return nativeKeyCode;
  }
}

DomCode DomCodeFromNSEvent(NSEvent* event) {
  if (KBGetLayoutType(LMGetKbdType()) == kKeyboardISO) {
    return ui::KeycodeConverter::NativeKeycodeToDomCode(
        ISOKeyboardKeyCodeMap([event keyCode]));
  }

  return ui::KeycodeConverter::NativeKeycodeToDomCode([event keyCode]);
}

The above Chromium code makes it that KeyboardEvent.code has the wrong value when typing on an ISO keyboard. i.e. [Backquote] and [IntlBackslash] are swapped, while keyCode, key, etc. are not. And that is the bug...


Our workaround

Our workaround is a bit complex, because we simply cannot detect when Chromium does the swap. The KeyboardEvent has no field called realCode or anything that would allow us to deduce from a singular KeyboardEvent that Chromium has done the swap.

To make matters worse, we cannot hook into Chromium and unswap the KeyboardEvent.code. i.e. we cannot patch Chromium, although Electron could do it on their side.

Our solution therefore consists of us polling on LMGetKbdType() on the main process. This value will change based on the last keyboard you have pressed a key on. We poll on this value every 3 seconds and determine if you last typed on an ISO keyboard. If you last typed on an ISO keyboard, we communicate this to our renderer processes, where we can then workaround the Chrome scan code swap.

Most likely, in your case, since the laptop has a built-in ANSI keyboard, initially, the value for LMGetKbdType() is initialized to something that would classify the keyboard as ANSI. Only after you do the first key press on the ISO keyboard, does OSX change the value of LMGetKbdType() on the main process. We will discover this in at most 3s (the polling interval) and communicate this to the renderer process so we can workaround the Chromium swapping.

All in all, when you switch from one different physical keyboard type to another, by pressing a key, it will take us ~3s to pick that up and begin working around Chromium's swapping.

IMHO we should create an Electron issue and ask them to delete the swapping inside Chromium so we don't have to do this crazy workaround. The reason Chromium swaps the scan code might make sense for a browser page, but does not make sense for a desktop application. Would be great if you want to file such an issue to Electron.

Also see #24153

@alexdima alexdima added the upstream Issue identified as 'upstream' component related (exists outside of VS Code) label May 12, 2017
@bpasero
Copy link
Member Author

bpasero commented May 13, 2017

@alexandrudima thanks, yeah I do remember this because we looked both into it last end game and I was aware of it when I filed the issue, however something else must be going wrong: This issue happens after VS Code has long been started and after I had typed already quite a bit so I would expect your polling logic to have determined everything by then. I am also not changing keyboards or so in this process.

As far as I can tell, at one point I can no longer toggle the integrated terminal via Ctrl+< and I have to assign the keybinding again via the keybindings editor to make it work. Right now I cannot reproduce this if I just restart and give it a try, but I know sometimes I end up in the situation where it does not work anymore. When this happens next time, I show you.

@alexdima alexdima added the info-needed Issue requires more information from poster label May 19, 2017
@alexdima alexdima added this to the Backlog milestone May 19, 2017
@vscodebot vscodebot bot closed this as completed May 26, 2017
@vscodebot
Copy link

vscodebot bot commented May 26, 2017

This issue has been closed automatically because it needs more information and has not had recent activity. Please refer to our guidelines for filing issues. Thank you for your contributions.

@bpasero
Copy link
Member Author

bpasero commented May 28, 2017

I will reopen this bug if it is not fixed in latest insider.

@alexdima alexdima modified the milestones: Backlog, May 2017 May 31, 2017
@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
info-needed Issue requires more information from poster upstream Issue identified as 'upstream' component related (exists outside of VS Code)
Projects
None yet
Development

No branches or pull requests

2 participants