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

AltGr/DeadKey + Space was broken - FIXED IN 1.20.10572.0 #16641

Closed
BrtCst opened this issue Feb 1, 2024 · 8 comments · Fixed by #16645
Closed

AltGr/DeadKey + Space was broken - FIXED IN 1.20.10572.0 #16641

BrtCst opened this issue Feb 1, 2024 · 8 comments · Fixed by #16645
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-VT Virtual Terminal sequence support In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.

Comments

@BrtCst
Copy link

BrtCst commented Feb 1, 2024

Windows Terminal version

1.20.10303.0

Windows build number

10.0.22635.3130

Other Software

WSL 2.1.1.0

Steps to reproduce

  1. Using FR bepo keyboard layout
  2. Typing AltGr + Space in a WSL tab

Expected Behavior

It should type an underscore, since AltGr + Space is the mapping for _ in FR bepo layout.

Actual Behavior

Rings the system bell. Worked fine in Terminal 1.18

@BrtCst BrtCst added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Feb 1, 2024
@zadjii-msft
Copy link
Member

@zadjii-msft zadjii-msft added this to the Terminal v1.21 milestone Feb 1, 2024
@zadjii-msft zadjii-msft added Area-Input Related to input processing (key presses, mouse, etc.) Area-VT Virtual Terminal sequence support Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Feb 1, 2024
@j4james
Copy link
Collaborator

j4james commented Feb 1, 2024

Yeah, sorry. That's almost certainly a result of PR #16511. We're treating Space as a "functional" key, which means it gets hardcoded VT mappings which take precedence over the keyboard layout. It's not modal, though, so it's possible it could be handled as a regular "graphic" key. However, I have a sneaking suspicion that could break another keyboard layout, and that's why I put it with "functional" keys in the first place. But that may have been an old issue that was resolved in one of my many rewrites. I'll need to do some testing to double check.

@j4james
Copy link
Collaborator

j4james commented Feb 1, 2024

Unfortunately it's not a simple fix. The reason I chose to handle it as a "functional" key was because the mapping for Ctrl+Space on a number of keyboard layouts is just SP, while it's expected by VT applications to be NUL. This includes the US layout. So if we handle it as a "graphic" key, that's going to break.

Not that that's impossible to fix, but it's likely to be messy.

@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Feb 1, 2024
@lhecker lhecker changed the title AltGr + Space not working in WSL with fr-bepo keyboard layout AltGr + Space / Deadkey + Space does not work correctly Feb 1, 2024
@lhecker lhecker pinned this issue Feb 1, 2024
@ilharp
Copy link

ilharp commented Feb 2, 2024

All my keypad keys don't work in v1.20, including Enter. I would like to ask if this bug is the same as this issue?

image

@juliencombattelli
Copy link

juliencombattelli commented Feb 2, 2024

Hello, I just noticed Windows Terminal Preview has the experimental profile option experimental.connection.passthroughMode that fixes those dead-keys and numpad-keys issues when set to true (at least on my side). But it is causing other strange behaviors with the arrow keys and CTRL-C as far as I can tell...

@mveroone
Copy link

mveroone commented Feb 2, 2024

All my keypad keys don't work in v1.20, including Enter. I would like to ask if this bug is the same as this issue?

image

Hi.
Same issue this morning and I think it's related to Zsh.
I've fixed it this way : https://superuser.com/a/742193/239282

@j4james
Copy link
Collaborator

j4james commented Feb 2, 2024

Yeah, if you're using zsh, I believe it sets the terminfo smkx capability, which enables Keypad Application Mode, and that's what makes the keypad keys generate unique escape sequences. Previous versions of Windows Terminal didn't support that mode, which is why you wouldn't have noticed before. It's not related to this Deadkey+Space issue.

Other than manually rebinding the keys, as @mveroone suggested, another way you might work around the issue is by patching the zle-line-init function to disable the Keypad Application Mode after they've enabled it. Something like this might do the trick:

functions -c zle-line-init zle-line-init-orig
zle-line-init () { zle-line-init-orig; echo -en "\e>" }

I know almost nothing about zsh, though, so I have no idea what other issues that might cause, but in my brief testing it seemed to work.

I also noticed that ohmyzsh has an open PR to ditch their smkx usage (ohmyzsh/ohmyzsh#5113), and that should theoretically fix the problem, but that PR is now 8 years old, so it doesn't look like it's happening anytime soon.

DHowett pushed a commit that referenced this issue Feb 6, 2024
This fixes two issues where the `Space` key wasn't being handled
correctly:

* Keyboards with an `AltGr`+`Space` mapping were not generating the
  expected character.
* Pressing a dead key followed by `Space` is supposed to generate the
  accent character associated with that key, but it wasn't doing so.

## References and Relevant Issues

These were both regressions from the keyboard refactor in PR #16511.

## Detailed Description of the Pull Request / Additional comments

The problem was that we were treating `VK_SPACE` as a "functional" key,
which means it gets hardcoded VT mappings which take precedence over
whatever is in the keyboard layout. This was deemed necessary to deal
with the fact that many keyboards incorrectly map `Ctrl`+`Space` as a
`SP` character, when it's expected to be `NUL`.

I've now dropped `VK_SPACE` from the functional mapping table and allow
it be handled by the default mapping algorithm for "graphic" keys.
However, I've also introduced a special case check for `Ctrl`+`Space`
(and other modifier variants), so we can bypass any incorrect keyboard
layouts for those combinations.

## Validation Steps Performed

I couldn't test with a French-BEPO keyboard layout directly, because the
MS Keyboard Layout Creator wouldn't accept a `Space` key mapping that
wasn't whitespace. However, if I remapped the `AltGr`+`Space` combo to
`LF`, I could confirm that we are now generating that correctly.

I've also tested the dead key `Space` combination on various keyboard
layouts and confirmed that that is now working correctly, and checked
that the `Ctrl`+`Space` combinations are still working too.

Closes #16641
Closes #16642
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Feb 6, 2024
@SchizoDuckie
Copy link

SchizoDuckie commented Feb 21, 2024

Did this stuff get released yet? I feel like a damned idiot every time a switch to a terminal!
[edit]
Nevermind, I was apparently running the Preview build still.
Guess It's time to switch back to the stable build. That solves the issue for me.

DHowett pushed a commit that referenced this issue Feb 21, 2024
This fixes two issues where the `Space` key wasn't being handled
correctly:

* Keyboards with an `AltGr`+`Space` mapping were not generating the
  expected character.
* Pressing a dead key followed by `Space` is supposed to generate the
  accent character associated with that key, but it wasn't doing so.

## References and Relevant Issues

These were both regressions from the keyboard refactor in PR #16511.

## Detailed Description of the Pull Request / Additional comments

The problem was that we were treating `VK_SPACE` as a "functional" key,
which means it gets hardcoded VT mappings which take precedence over
whatever is in the keyboard layout. This was deemed necessary to deal
with the fact that many keyboards incorrectly map `Ctrl`+`Space` as a
`SP` character, when it's expected to be `NUL`.

I've now dropped `VK_SPACE` from the functional mapping table and allow
it be handled by the default mapping algorithm for "graphic" keys.
However, I've also introduced a special case check for `Ctrl`+`Space`
(and other modifier variants), so we can bypass any incorrect keyboard
layouts for those combinations.

## Validation Steps Performed

I couldn't test with a French-BEPO keyboard layout directly, because the
MS Keyboard Layout Creator wouldn't accept a `Space` key mapping that
wasn't whitespace. However, if I remapped the `AltGr`+`Space` combo to
`LF`, I could confirm that we are now generating that correctly.

I've also tested the dead key `Space` combination on various keyboard
layouts and confirmed that that is now working correctly, and checked
that the `Ctrl`+`Space` combinations are still working too.

Closes #16641
Closes #16642

(cherry picked from commit ec91be5)
Service-Card-Id: 91738880
Service-Version: 1.20
@DHowett DHowett changed the title AltGr + Space / Deadkey + Space does not work correctly AltGr + Space / Deadkey + Space does not work correctly - FIXED IN 1.20.10572.0 Mar 25, 2024
@DHowett DHowett changed the title AltGr + Space / Deadkey + Space does not work correctly - FIXED IN 1.20.10572.0 AltGr/DeadKey + Space was broken - FIXED IN 1.20.10572.0 Mar 25, 2024
@DHowett DHowett unpinned this issue Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Area-VT Virtual Terminal sequence support In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants