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

dwControlKeyState behaviour changed between Windows 10/Windows 11 #16266

Closed
taviso opened this issue Nov 6, 2023 · 6 comments · Fixed by #16335
Closed

dwControlKeyState behaviour changed between Windows 10/Windows 11 #16266

taviso opened this issue Nov 6, 2023 · 6 comments · Fixed by #16335
Assignees
Labels
Area-Input Related to input processing (key presses, mouse, etc.) Impact-Correctness It be wrong. 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 Priority-1 A description (P1) Product-Conpty For console issues specifically related to conpty zInbox-Bug Ignore me!

Comments

@taviso
Copy link

taviso commented Nov 6, 2023

Windows Terminal version

1.19.231010001-preview

Windows build number

10.0.22621.0

Other Software

hiew 8.79 (inside wsl)

Steps to reproduce

I have a win32 console application (hiew, a powerful hex editor) that worked fine when invoked from within WSL in Windows 10, but no longer works in Windows 11. It still works okay when invoked from cmd or powershell, but I liked using it under WSL ☹️

Various keys no longer seem to register.

I wrote a quick c program to dump console input events, and I observe the following pattern:

  • Pressing F11 with a win32 console application under WSL on Windows 10 (conhost 10.0.19041.3393) (Works ✅):
Key event:
 bKeyDown:          1
 wRepeatCount:      1
 wVirtualKeyCode:   0x7a
    MAPVK_VK_TO_VSC  => 0x57
    MAPVK_VK_TO_CHAR => 00
 wVirtualScanCode:  0x57
    MAPVK_VSC_TO_VK => 0x7a
    MAPVK_VSC_TO_VK_EX => 0x7a
 uChar:
  UnicodeChar:      '' (0000)
  AsciiChar:        '' (00)
    VkKeyScanA  => 0x3c0
 dwControlKeyState: 0
  • Pressing F11 with a win32 console application under cmd on Windows 11 (I've tried release conhost, and current stable and preview OpenConsole, with same result ✅)
Key event:
 bKeyDown:          1
 wRepeatCount:      1
 wVirtualKeyCode:   0x7a
    MAPVK_VK_TO_VSC  => 0x57
    MAPVK_VK_TO_CHAR => 00
 wVirtualScanCode:  0x57
    MAPVK_VSC_TO_VK => 0x7a
    MAPVK_VSC_TO_VK_EX => 0x7a
 uChar:
  UnicodeChar:      ' ' (0000)
  AsciiChar:        ' ' (00)
    VkKeyScanA  => 0x3c0
 dwControlKeyState: 0
  • Pressing F11 with a win32 console application under WSL on Windows 11 (Also tried release conhost, stable and preview OpenConsole, with same result ❌ )
Key event:
 bKeyDown:          1
 wRepeatCount:      1
 wVirtualKeyCode:   0x7a
    MAPVK_VK_TO_VSC  => 0x57
    MAPVK_VK_TO_CHAR => 00
 wVirtualScanCode:  0x57
    MAPVK_VSC_TO_VK => 0x7a
    MAPVK_VSC_TO_VK_EX => 0x7a
 uChar:
  UnicodeChar:      '' (0000)
  AsciiChar:        '' (00)
    VkKeyScanA  => 0x3c0
 dwControlKeyState: 256
ENHANCED_KEY

I think the issue is that ENHANCED_KEY is now being set, and it wasn't before. I don't know why this changed, but it seems to have broken at least one app ☹️

Do you know what might be causing this?

Expected Behavior

Keys to register.

Actual Behavior

Keys are ignored, I suspect because dwControlKeyState changed.

@taviso taviso 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 Nov 6, 2023
@zadjii-msft
Copy link
Member

I'm almost certain this is probably due to #5021, though, there's probably a complicated relationship here between the fact that there's a different ConPTY that's used for Terminal (and is shipped with the Terminal), vs the one that WSL uses for interop (and is shipped with the OS)

@zadjii-msft zadjii-msft added the Area-Input Related to input processing (key presses, mouse, etc.) label Nov 6, 2023
@taviso
Copy link
Author

taviso commented Nov 6, 2023

If I replace conhost.exe with OpenConsole, will WSL automatically pick it up interop as well? (I don't know how all the pieces fit together sorry!)

I'm willing to give it a shot and see if it's already been fixed somewhere...? (I assume I'll have to takeown/icacl/etc.., I don't mind voiding my warranty 😄)

@DHowett
Copy link
Member

DHowett commented Nov 6, 2023

It absolutely will! No warranty, etc, but you know the drill. 😄

Thanks for being so down to test things with us.

@lhecker
Copy link
Member

lhecker commented Nov 6, 2023

To do it "correctly", you'd use a tool called sfpcopy.exe but it'll also work without it. Putting the original conhost.exe back will not restore Windows to its original state because conhost.exe is actually hardlinked to

C:\Windows\WinSxS\amd64_microsoft-onecore-console-host-core_31bf3856ad364e35_10.0.22621.2483_none_4d83bf9b9fc4fb79\conhost.exe

(...with a possibly different version number on your machine.)

You can probably (90% sure?) restore the original conhost.exe to its original state by running sfc /scannow. I've been running OpenConsole.exe as my conhost for about a year now and it's been fine, but I'm also running internal dev version of Windows anyways, so... 😅

@DHowett
Copy link
Member

DHowett commented Nov 6, 2023

Do note that if you damage the version in the component store (...\WinSxS) you become ineligible for future servicing updates. Replacing just the hardlink is much better in that regard. 😁

@taviso
Copy link
Author

taviso commented Nov 7, 2023

I did get the system booted with 1.19.2310.10001, but sadly my console bug is still there!

I listened to your warning and didn't bother trying to mess with the links, I assume some future update will clobber it with a link, that's okay with me 😄

I guess there are probably not many people trying to use win32 console apps in WSL, maybe I can try to bisect it at least, even if I don't know what the solution is.

@carlos-zamora carlos-zamora added Product-Conpty For console issues specifically related to conpty Priority-1 A description (P1) Impact-Correctness It be wrong. zInbox-Bug Ignore me! and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Nov 8, 2023
@carlos-zamora carlos-zamora added this to the Terminal v1.20 milestone Nov 8, 2023
@lhecker lhecker self-assigned this Nov 16, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Nov 17, 2023
lhecker added a commit that referenced this issue Nov 30, 2023
Since all VT parameters are treated to be at least 1 (and 1 if they're
absent or 0), `modifierParam > 0` was always true. This meant that
`ENHANCED_KEY` was always being set. It's unclear why `ENHANCED_KEY`
was used there, but it's likely not needed in general.

Closes #16266

## Validation Steps Performed
* Can't test this unless we fix the win32 input mode issue #16343
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Nov 30, 2023
DHowett pushed a commit that referenced this issue Dec 4, 2023
Since all VT parameters are treated to be at least 1 (and 1 if they're
absent or 0), `modifierParam > 0` was always true. This meant that
`ENHANCED_KEY` was always being set. It's unclear why `ENHANCED_KEY`
was used there, but it's likely not needed in general.

Closes #16266

## Validation Steps Performed
* Can't test this unless we fix the win32 input mode issue #16343 ❌

(cherry picked from commit be9fc20)
Service-Card-Id: 91159301
Service-Version: 1.19
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.) Impact-Correctness It be wrong. 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 Priority-1 A description (P1) Product-Conpty For console issues specifically related to conpty zInbox-Bug Ignore me!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants