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.

Sign up for GitHub

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

Jump to bottom

Windows Terminal 1.1 breaks remapped CapsLock key inside WSL #7120

Closed
christianfosli opened this issue Jul 29, 2020 · 5 comments · Fixed by #7145
Closed

Windows Terminal 1.1 breaks remapped CapsLock key inside WSL #7120

christianfosli opened this issue Jul 29, 2020 · 5 comments · Fixed by #7145
Labels
Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@christianfosli
Copy link

christianfosli commented Jul 29, 2020 •
edited
Loading

Environment

Windows build number: Microsoft Windows [Version 10.0.18363.959] (1909)
Windows Terminal version (if applicable): 1.1.2021.0

Any other software? PowerToys v0.19.2, Ubuntu 20.04 WSL fresh install from Microsoft Store with no custom config

Steps to reproduce

  • In Windows PowerToys, map "Caps Lock" key to "Ctrl" (left or right doesn't matter)
  • Open Ubuntu WSL in Windows Terminal
  • Open Vim or Neovim, go into Insert mode
  • Press "Caps Lock" (which is mapped to "Ctrl")

Expected behavior

"Caps Lock" should be treated as "Ctrl". No visible result.

Actual behavior

"Caps Lock" is treated as "Control-Space", and tries insert any previously inserted text again.

Work-Around

The issue goes away by adding this to settings.json

"experimental.input.forceVT": true,

Therefore it is likely caused by #6309

More Details, Debugging info

I originally thought it was a bug in PowerToys, but since the work-around works, I figure the bug must be here.
It's a bit annoying, because I like to escape out of normal mode in vim by pressing <CapsLock>-[ (which should be treated as <Ctrl>-[, which is again treated as <esc>). Now I instead insert the text i wrote twice.

If i input <Ctrl-V><Caps-Lock> in Vim insert mode, vim inserts <C-Space>. This is how I determined the remapped key is interpreted as <C-Space>.

  • This happens only inside Windows-Subsystem for Linux (WSL1) inside windows terminal
  • I tested with a new installation of Ubuntu from the Microsoft Store, with no customizations to Ubuntu, Vim or wslconf
  • I tested inside and outside of tmux, which did not make any difference
  • The issue is not present running WSL from CMD or Powershell, i.e. not using windows terminal.
  • The issue is not present running Neovim directly from Powershell Core inside windows terminal
  • The issue is not present running vim in Linux (tested Alpine) inside windows terminal as a Docker container
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jul 29, 2020
@christianfosli christianfosli mentioned this issue Jul 29, 2020
Closed
@DHowett
Copy link
Member

DHowett commented Jul 29, 2020 •
edited
Loading

So, I actually can reproduce this in WSL outside of the Terminal.

Here's the result of pressing my remapped caps lock under showkey -a to dump what the tty driver is generating:

image

This is an unfortunate casualty of the better (I know, it's ironic) keyboard support offered by #6309. There's two issues here:

  1. PowerToys' remapped key doesn't generate a "scancode" like the normal control key does
  2. WSL treats any key it receives with no character (Ctrl generates no character) plus no scancode as valid input for \0 (which roughly de-maps to Ctrl+Space, but is also the encoding for Ctrl+Shift+2)

This worked before #6309 because the remote app (here, WSL) was never made aware that somebody pressed Ctrl without a scancode.

WSL can't remove the check in (2) because it would break one of the few valid ways to insert \0. It's best fixed in PowerToys, as it's generating the key event. Once the event comes into Terminal we have no way of determining whether the user actually pressed a scancode-less key and truly wanted a null byte or whether they pressed this somewhat broken remapped Ctrl. ☹️

debugging information

real ctrl (left) key

␛[17;29;0;1;8;1_

remapped

␛[17;0;0;1;8;1_

second field is 29 in real and 0 in the remap

@christianfosli
Copy link
Author

Thanks, @DHowett, for finding the root cause and describing in an easy to understand way!
In this case I will close this and create an issue in PowerToys instead.

@christianfosli
Copy link
Author

debugging information
real ctrl (left) key

␛[17;29;0;1;8;1_

remapped

␛[17;0;0;1;8;1_

second field is 29 in real and 0 in the remap

How do you find these?
If I open a new bug in PowerToys titled "Capslock -> Control map is missing "scancode".", I should probably include how to debug it properly.

@lhecker lhecker mentioned this issue Aug 6, 2020
Merged
@ghost ghost added the In-PR This issue has a related PR label Aug 6, 2020
@DHowett
Copy link
Member

DHowett commented Aug 6, 2020

So, somebody else reported a similar bug and @lhecker investigated it pretty quickly. We think we can fix this on our side 😄 without requiring any powertoys changes.

ghost pushed a commit that referenced this issue Aug 6, 2020
@lhecker
Fix #7064: Ignore key events without scan code (#7145)
b617c43 
Up until #4999 we deferred all key events to the character event handler
for which `ToUnicodeEx` returned a valid character and alternatively
those who aren't a special key combination as listed in
`TerminalInput`'s implementation.

Since #4999 we started acknowledging/handling all key events no matter
whether they're actually a known key combination. Given non-ASCII inputs
the Win32 `SendInput()` method generates certain sequences that aren't
recognizable combinations though and if they're handled by the key event
handler no follow up character event is sent containing the unicode
character.

This PR adds another condition and defers all key events without scan
code (i.e. those not representable by the current keyboard layout) to
the character event handler.

I'm absolutely not certain that this PR doesn't have a negative effect
on other kinds of inputs.

Is it common for key events to not contain a scan code? I personally
haven't seen it happen before AutoHotKey/SendInput.

Before this PR is merged it'd be nice to have a good testing plan in
place in order to ensure nothing breaks.

## Validation Steps Performed

Remapped `AltGr+8` to `»` using AutoHotKey using `<^>!8::SendInput {Raw}»`.
Ensured `»` is printed if `AltGr+8` is pressed.

Closes #7064
Closes #7120
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Aug 6, 2020
@TFKls TFKls mentioned this issue Aug 10, 2020
Closed
DHowett pushed a commit that referenced this issue Aug 11, 2020
@lhecker @DHowett
Fix #7064: Ignore key events without scan code (#7145)
5d74557 
Up until #4999 we deferred all key events to the character event handler
for which `ToUnicodeEx` returned a valid character and alternatively
those who aren't a special key combination as listed in
`TerminalInput`'s implementation.

Since #4999 we started acknowledging/handling all key events no matter
whether they're actually a known key combination. Given non-ASCII inputs
the Win32 `SendInput()` method generates certain sequences that aren't
recognizable combinations though and if they're handled by the key event
handler no follow up character event is sent containing the unicode
character.

This PR adds another condition and defers all key events without scan
code (i.e. those not representable by the current keyboard layout) to
the character event handler.

I'm absolutely not certain that this PR doesn't have a negative effect
on other kinds of inputs.

Is it common for key events to not contain a scan code? I personally
haven't seen it happen before AutoHotKey/SendInput.

Before this PR is merged it'd be nice to have a good testing plan in
place in order to ensure nothing breaks.

## Validation Steps Performed

Remapped `AltGr+8` to `»` using AutoHotKey using `<^>!8::SendInput {Raw}»`.
Ensured `»` is printed if `AltGr+8` is pressed.

Closes #7064
Closes #7120

(cherry picked from commit b617c43)
@ghost
Copy link

ghost commented Aug 13, 2020

🎉This issue was addressed in #7145, which has now been successfully released as Windows Terminal Preview v1.2.2234.0.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Needs-Tag-Fix Doesn't match tag requirements Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #7064: Ignore key events without scan code
2 participants
@DHowett @christianfosli