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

Launching in Russian layout causes Ctrl+C and other signal chars to not work #17197

Open
EntityinArray opened this issue May 6, 2024 · 7 comments
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.

Comments

@EntityinArray
Copy link

EntityinArray commented May 6, 2024

Windows Terminal version

1.19.11213.0

Windows build number

10.0.19045.0

Other Software

PSVersion 7.4.2
PSEdition Core
GitCommitId 7.4.2
OS Microsoft Windows 10.0.19045
Platform Win32NT
PSCompatibleVersions {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion 2.3
SerializationVersion 1.1.0.1
WSManStackVersion 3.0

Steps to reproduce

  • Change your keyboard layout to Russian
  • Open Windows Terminal
  • Try Ctrl+C

Expected Behavior

Sends interrupt

Actual Behavior

Types C

@EntityinArray EntityinArray 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 May 6, 2024
@lhecker
Copy link
Member

lhecker commented May 6, 2024

Between which keyboard layouts are you switching? Does the issue persist even if you press Ctrl+C multiple times?

@EntityinArray EntityinArray changed the title Switching keyboard layout causes Ctrl+C and other signal chars to not work Launching in Russian layout causes Ctrl+C and other signal chars to not work May 7, 2024
@EntityinArray
Copy link
Author

Between which keyboard layouts are you switching? Does the issue persist even if you press Ctrl+C multiple times?

Sorry, i made a new discovery. The layout switching is not necessary. Bug occurs when you just start Windows Terminal in Russian layout.

I switch between English and Russian. If Windows Terminal is started with Russian layout, it doesn't accept signal chars at all until app restart, even if you switch layout back to English during runtime.

image

@j4james
Copy link
Collaborator

j4james commented May 7, 2024

I think this is just PowerShell. I can't reproduce it in a cmd shell or WSL bash. I think it's because the C key on the Russian keyboard is actually a Cyrillic с, and PowerShell can't translate that into a control character. You can see the same thing with a Hebrew keyboard layout.

I also found that it seems to be the first time you press a Ctrl key that "locks" the mapping for that tab. So if you open a pwsh tab with the Russian layout, then switch to English, press Ctrl+C, then switch back to Russian, it should work fine. But if you press Ctrl+C when the Russian layout is first active, it'll be broken for the rest of that session.

I also see the same behavior in conhost.

@carlos-zamora carlos-zamora added this to the Terminal v1.22 milestone May 8, 2024
@carlos-zamora carlos-zamora added Area-Input Related to input processing (key presses, mouse, etc.) 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 May 8, 2024
@j4james
Copy link
Collaborator

j4james commented May 9, 2024

This bug was tracked in the PSReadLine repository in PowerShell/PSReadLine#1393, and it sounds like they've got a partial fix. The Ctrl keys still won't work when your keyboard layout is Russian, but they should work if you switch to English without needing to restart the shell. You'll need the latest beta version of PSReadLine to get that fix, though.

@lhecker
Copy link
Member

lhecker commented May 9, 2024

Apropos @j4james I believe this code may not work in conhost:

const auto hkl = GetKeyboardLayout(GetWindowThreadProcessId(GetForegroundWindow(), nullptr));

If you try something like this:

#include <Windows.h>
#include <cstdio>

int main() {
    for (;;) {
        const auto hkl = GetKeyboardLayout(GetWindowThreadProcessId(GetForegroundWindow(), nullptr));
        printf("0x%p\n", hkl);
        Sleep(1000);
    }
}

you'll see that the layout never changes in conhost. I believe it's because of the window ownership lying that ntuser does where it makes it so that it appears as if cmd/pwsh/wsl owns the window. This seems to also affect GetWindowThreadProcessId which then returns the main thread ID of cmd/pwsh/wsl instead of the actual window handle. (BTW that seems like an OS bug?)

@lhecker
Copy link
Member

lhecker commented May 9, 2024

It seems I may have spoken too soon. I'm currently looking how to solve this in pwsh actually, and I'm not actually testing it in conhost. So I probably shouldn't have written the above without confirming it.

Most importantly I found this comment in GetWindowThreadProcessId:

NB: This has implications on the scenario where the window's owner gets remapped via NtUserConsoleControl / ConsoleSetWindowOwner. If called by thread that owns a window, API will return true TID/PID and not remapped one. This is how it was done historically and we are keeping this behavior.

I guess this explains why the current code may work: The function gets called by conhost itself from threads that may own a window. That's slightly bodgy however of course. 😅

@j4james
Copy link
Collaborator

j4james commented May 9, 2024

@lhecker Note that the code you referenced is only applicable to shells using VT input sequences, like the WSL bash shell, and that doesn't have any problems with the Ctrl keys when using a Russian keyboard layout. But PSReadLine isn't using VT input sequences - they're doing the keyboard translation themselves. And regardless of whether they're picking up the correct layout at runtime, they simply don't handle Ctrl keys on a Russian keyboard.

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.) Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

4 participants