Fix SendInput handling #7900

lhecker · 2020-10-12T20:12:20Z

While not explicitly permitted, a wide range of software (including
Windows' own touch keyboard) sets the wScan member of the KEYBDINPUT
structure to 0, resulting in scanCode being 0 as well. In these
situations we'll now use the vkey to get a scanCode.

Validation

AutoHotkey
- Use a keyboard layout with AltGr key
- Execute the following script:
```
#NoEnv
#Warn
SendMode Input
SetWorkingDir %A_ScriptDir%
<^>!8::SendInput {Raw}»
```
- Press AltGr+8 while the Terminal is in the foreground
- Ensure » is being echoed ✔️
PowerToys
- Add a Ctrl+I -> ↑ (up arrow) keyboard shortcut
- Press Ctrl+I while the Terminal is in the foreground
- Ensure the shell history is being navigated backwards ✔️
Windows Touch Keyboard
- Right-click or tap and hold the taskbar and select "Show touch
  keyboard" button
- Open touch keyboard
- Ensure keyboard works like a regular keyboard ✔️
- Ensure unicode characters are echoed on the Terminal as well (except
  for Emojis) ✔️

Closes #7438
Closes #7495
Closes #7843

While not explicitly permitted, a wide range of software, including Windows' own touch keyboard, sets the wScan member of the KEYBDINPUT structure to 0, resulting in scanCode being 0 as well. In these situations we'll now use the vkey to get a scanCode.

lhecker · 2020-10-12T20:18:55Z

src/cascadia/UnitTests_TerminalCore/InputTest.cpp

        VERIFY_IS_FALSE(term.SendKeyEvent(0, 123, {}, true));
        VERIFY_IS_FALSE(term.SendKeyEvent(255, 123, {}, true));
-        VERIFY_IS_FALSE(term.SendKeyEvent(123, 0, {}, true));


The already minimal test coverage for this particular method is being reduced even further. 😕
Unfortunately I don't believe there's any valid vkey on "regular" computers that maps to a zero scanCode using MapVirtualKeyW.

lhecker · 2020-10-12T20:23:03Z

src/cascadia/TerminalCore/Terminal.cpp

+    // --> Alternatively get the scanCode from the vkey if possible.
+    // GH#7495
+    const auto sc = scanCode ? scanCode : _ScanCodeFromVirtualKey(vkey);
+    if (sc == 0)


I've been thinking for a while whether we should allow sc to be 0 or not in the code that follows after this line.
I've decided for the latter, because we can still remove this if-condition later on if it turns out that some vkeys cannot be mapped to scanCodes on some platforms. Until then this is something of a "defensive programming style" of sorts.

I think it's important I mention the reason we need scanCodes here again (because this topic is kinda complicated)...
When we receive a key event we must decide whether we consume the event or not. If we do consume it we'll not receive a follow-up character event no matter what. We'd like to only consume non-character key events, because the character events contain proper unicode characters and the key events don't.

In order to decide this we use ToUnicodeEx which translates a vkey and some modifier keys to one or more characters. If it spits out characters we ignore the key and wait for an appropriate character event.
Unfortunately there can be multiple meanings to a single vkey depending on the keyboard state and depending on the key in question that was pressed. Two or more keys might map to the same vkey but have different scanCodes.

Due to this ToUnicodeEx asks for a scanCode allowing it to correctly map unicode characters. Ergo we need scanCodes in the key event handler. But apparently even official applications don't set it in the KEYBDINPUT structure, if input doesn't originate from a physical keyboard (but for instance a software/touch keyboard instead).

This is an excellent description. Thanks!

lhecker · 2020-10-12T20:24:38Z

If anyone has any suggestions which corner cases I should additionally test, please let me know.
I don't like breaking stuff and I'd really like to "unbreak" all the corner cases now. 😅

DHowett

This looks great and very well-reasoned to me,

zadjii-msft

Alright, so after enough coffee, this doesn't actually look all that scary. I quite like the clever fallback of looking up the scan code from the vkey, since that's likely been set correctly.

Sorry for the delay in reviewing!

zadjii-msft · 2020-10-27T17:36:30Z

I'm gonna check to see if I can repro #8045, and see if this fixes that. Turns out my dev box is still on, despite no one being in the office for the last 8 months 😆

ghost · 2020-10-27T19:06:26Z

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

zadjii-msft · 2020-10-27T19:07:07Z

I couldn't repro #8045, so I'm just gonna merge this, and hope we figure out the root cause of that issue separately.

While not explicitly permitted, a wide range of software (including Windows' own touch keyboard) sets the `wScan` member of the `KEYBDINPUT` structure to 0, resulting in `scanCode` being 0 as well. In these situations we'll now use the `vkey` to get a `scanCode`. Validation ---------- * AutoHotkey * Use a keyboard layout with `AltGr` key * Execute the following script: ```ahk #NoEnv #Warn SendMode Input SetWorkingDir %A_ScriptDir% <^>!8::SendInput {Raw}» ``` * Press `AltGr+8` while the Terminal is in the foreground * Ensure » is being echoed ✔️ * PowerToys * Add a `Ctrl+I -> ↑ (up arrow)` keyboard shortcut * Press `Ctrl+I` while the Terminal is in the foreground * Ensure the shell history is being navigated backwards ✔️ * Windows Touch Keyboard * Right-click or tap and hold the taskbar and select "Show touch keyboard" button * Open touch keyboard * Ensure keyboard works like a regular keyboard ✔️ * Ensure unicode characters are echoed on the Terminal as well (except for Emojis) ✔️ Closes #7438 Closes #7495 Closes #7843 (cherry picked from commit d51d8dc)

ghost · 2020-11-11T18:06:29Z

🎉Windows Terminal v1.4.3141.0 has been released which incorporates this pull request.:tada:

Handy links:

ghost · 2020-11-11T18:10:23Z

🎉Windows Terminal Preview v1.5.3142.0 has been released which incorporates this pull request.:tada:

Handy links:

D3nbot · 2022-10-10T09:27:08Z

Hi,

has this been reintroduced with a later release of terminal by any chance? just I'm experiencing the same issues, enabled the service though initially it wasn't even listed, had to enable a few options withing settings accessibility for the service to show in the list of services, but even after enabling the service still now luck.

AltGr+8 seems to work, along with other AltGr+ (random keys).

I'm running this on:
Windows 11 Pro - 21H2 - 22000.978
Windows Feature Experience Pack 1000.22000.978.0

Test Terminal Version: 1.15.2713.0 and preview release 1.16.2642.0

Thanks

lhecker · 2022-10-10T14:23:22Z

@D3nbot Which service are you talking about? None of the 3 issues this PR closed mention anything related to "service". From what I can tell, this PR also still seems to work for me.

D3nbot · 2022-10-10T23:19:52Z

Hi @lhecker,

One of the many threads mentioned "Touch Keyboard and Handwriting Panel Service" once I'd rebooted after installing terminal pre, it seems to have started working in both variants, not entirely sure what caused the issue.

Sorry if I wasted anyone's time with this one, was highly frustrating at the time.

Thanks

ghost added Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Terminal The new Windows Terminal. labels Oct 12, 2020

Fix SendInput handling

ff00cbb

While not explicitly permitted, a wide range of software, including Windows' own touch keyboard, sets the wScan member of the KEYBDINPUT structure to 0, resulting in scanCode being 0 as well. In these situations we'll now use the vkey to get a scanCode.

lhecker commented Oct 12, 2020

View reviewed changes

ghost added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Priority-2 A description (P2) labels Oct 12, 2020

zadjii-msft self-assigned this Oct 15, 2020

DHowett approved these changes Oct 20, 2020

View reviewed changes

microsoft-github-updates bot changed the base branch from master to main October 22, 2020 00:27

lhecker mentioned this pull request Oct 27, 2020

Terminal not accepting keyboard input in Windows 10 #8045

Closed

zadjii-msft approved these changes Oct 27, 2020

View reviewed changes

zadjii-msft removed their assignment Oct 27, 2020

zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label Oct 27, 2020

ghost merged commit d51d8dc into microsoft:main Oct 27, 2020

DHowett added the hacktoberfest-accepted label Nov 3, 2020

lhecker deleted the fix-send-input-handling branch November 11, 2020 20:41

lhecker mentioned this pull request Nov 26, 2020

Windows Terminal 1.1 breaks WinCompose input #7089

Closed

samhocevar mentioned this pull request Nov 26, 2020

Does not work in Windows Terminal samhocevar/wincompose#348

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix SendInput handling #7900

Fix SendInput handling #7900

lhecker commented Oct 12, 2020 •

edited by DHowett

Loading

lhecker Oct 12, 2020

lhecker Oct 12, 2020 •

edited

Loading

lhecker Oct 12, 2020 •

edited

Loading

DHowett Oct 20, 2020

lhecker commented Oct 12, 2020

DHowett left a comment

zadjii-msft left a comment

zadjii-msft commented Oct 27, 2020

ghost commented Oct 27, 2020

zadjii-msft commented Oct 27, 2020

ghost commented Nov 11, 2020

ghost commented Nov 11, 2020

D3nbot commented Oct 10, 2022

lhecker commented Oct 10, 2022

D3nbot commented Oct 10, 2022

Fix SendInput handling #7900

Fix SendInput handling #7900

Conversation

lhecker commented Oct 12, 2020 • edited by DHowett Loading

Validation

lhecker Oct 12, 2020

Choose a reason for hiding this comment

lhecker Oct 12, 2020 • edited Loading

Choose a reason for hiding this comment

lhecker Oct 12, 2020 • edited Loading

Choose a reason for hiding this comment

DHowett Oct 20, 2020

Choose a reason for hiding this comment

lhecker commented Oct 12, 2020

DHowett left a comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft commented Oct 27, 2020

ghost commented Oct 27, 2020

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

zadjii-msft commented Oct 27, 2020

ghost commented Nov 11, 2020

ghost commented Nov 11, 2020

D3nbot commented Oct 10, 2022

lhecker commented Oct 10, 2022

D3nbot commented Oct 10, 2022

lhecker commented Oct 12, 2020 •

edited by DHowett

Loading

lhecker Oct 12, 2020 •

edited

Loading

lhecker Oct 12, 2020 •

edited

Loading

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.