From 3098fba2030ecf7c3cf7a8b5fefb3737996f2636 Mon Sep 17 00:00:00 2001 From: Leonard Hecker Date: Tue, 27 Oct 2020 20:06:29 +0100 Subject: [PATCH] Fix SendInput handling (#7900) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 d51d8dc768a9eb521d32bea3d6aea085ab92ea16) --- ...5f007265b351e140d20b3976792523ad689241.txt | 7 +++++ .github/actions/spell-check/expect/expect.txt | 28 ------------------ src/cascadia/TerminalCore/Terminal.cpp | 29 ++++++++++--------- .../UnitTests_TerminalCore/InputTest.cpp | 2 -- 4 files changed, 23 insertions(+), 43 deletions(-) create mode 100644 .github/actions/spell-check/expect/655f007265b351e140d20b3976792523ad689241.txt diff --git a/.github/actions/spell-check/expect/655f007265b351e140d20b3976792523ad689241.txt b/.github/actions/spell-check/expect/655f007265b351e140d20b3976792523ad689241.txt new file mode 100644 index 00000000000..39f6c008254 --- /dev/null +++ b/.github/actions/spell-check/expect/655f007265b351e140d20b3976792523ad689241.txt @@ -0,0 +1,7 @@ +autogenerated +CPPCORECHECK +Debian +filepath +inplace +KEYBDINPUT +WINVER diff --git a/.github/actions/spell-check/expect/expect.txt b/.github/actions/spell-check/expect/expect.txt index 8e59251ffdc..0389ca2a2f7 100644 --- a/.github/actions/spell-check/expect/expect.txt +++ b/.github/actions/spell-check/expect/expect.txt @@ -33,7 +33,6 @@ AHelper ahz AImpl AInplace -akb ALIGNRIGHT alloc allocing @@ -62,7 +61,6 @@ apiset apos APPBARDATA appconsult -appdata APPICON appium applet @@ -109,10 +107,8 @@ aumid Authenticode AUTOBUDDY AUTOCHECKBOX -Autogenerated autohide AUTOHSCROLL -autologin automagically autopositioning AUTORADIOBUTTON @@ -393,7 +389,6 @@ CPINFOEX cplinfo cplusplus cpp -cppcorecheck cppcorecheckrules cpprest cpprestsdk @@ -510,10 +505,8 @@ DDESHARE DDevice DEADCHAR dealloc -debian debolden debounce -debugbreak DECALN DECANM DECAUPSS @@ -521,11 +514,9 @@ DECAWM DECCKM DECCOLM DECEKBD -decf DECKPAM DECKPM DECKPNM -DECLL DECLRMM decls declspec @@ -551,7 +542,6 @@ DECSEL DECSET DECSLPP DECSLRM -DECSMBV DECSMKR DECSR decstandar @@ -714,7 +704,6 @@ EPres ERASEBKGND errno errorlevel -esa ETB etcoreapp ETW @@ -769,7 +758,6 @@ fgetwc fgidx FILEDESCRIPTION fileno -FILEPATH FILESUBTYPE FILESYSPATH filesystem @@ -930,7 +918,6 @@ GTP guc gui guidatom -guidgenerator GValue GWL GWLP @@ -1101,7 +1088,6 @@ Inlines INotify inout INPATHROOT -Inplace inproc Inputkeyinfo INPUTPROCESSORPROFILE @@ -1190,11 +1176,9 @@ kcud kcuf kcuu Kd -keith kernelbase kernelbasestaging keybinding -keybound keychord keydown keyevent @@ -1467,7 +1451,6 @@ namestream Namquiseratal nano natvis -naws nbsp Nc NCCALCSIZE @@ -1549,7 +1532,6 @@ NOTNULL NOTOPMOST NOTRACK NOTSUPPORTED -notypeopt nouicompat nounihan NOUPDATE @@ -1624,7 +1606,6 @@ opencon openconsole OPENIF OPENLINK -openlogo openps opensource openvt @@ -1963,7 +1944,6 @@ resheader resizable resmimetype restrictedcapabilities -restrictederrorinfo resw resx retval @@ -1990,7 +1970,6 @@ rgw rgwch rhs ri -richturn RIGHTALIGN RIGHTBUTTON riid @@ -2064,7 +2043,6 @@ SCROLLSCALE SCROLLSCREENBUFFER Scrollup Scrolluppage -Scs scursor sddl sdeleted @@ -2238,7 +2216,6 @@ subkey SUBLANG sublicensable submenu -subnegotiation subresource subspan substr @@ -2249,7 +2226,6 @@ svg swapchain swapchainpanel swappable -Switchto SWMR SWP swprintf @@ -2304,7 +2280,6 @@ technet tellp telnet telnetd -telnetpp templated terminalcore TERMINALSCROLLING @@ -2701,12 +2676,10 @@ wintelnet winternl winuser winuserp -winver wistd wixproj wline wlinestream -Wlk wmain WMSZ wnd @@ -2749,7 +2722,6 @@ WRunoff WScript wsl WSLENV -wslhome wsmatch WSpace wss diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index d1a8cf43ffa..8bfbf7d6aa0 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -471,15 +471,20 @@ bool Terminal::SendKeyEvent(const WORD vkey, _StoreKeyEvent(vkey, scanCode); - // As a Terminal we're mostly interested in getting key events from physical hardware (mouse & keyboard). - // We're thus ignoring events whose values are outside the valid range and unlikely to be generated by the current keyboard. - // It's very likely that a proper followup character event will be sent to us. - // This prominently happens using AutoHotKey's keyboard remapping feature, - // which sends input events whose vkey is 0xff and scanCode is 0. - // We need to check for this early, as _CharacterFromKeyEvent() always returns 0 for such invalid values, - // making us believe that this is an actual non-character input, while it usually isn't. + // Certain applications like AutoHotKey and its keyboard remapping feature, + // send us key events using SendInput() whose values are outside of the valid range. // GH#7064 - if (vkey == 0 || vkey >= 0xff || scanCode == 0) + if (vkey == 0 || vkey >= 0xff) + { + return false; + } + + // 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. + // --> Alternatively get the scanCode from the vkey if possible. + // GH#7495 + const auto sc = scanCode ? scanCode : _ScanCodeFromVirtualKey(vkey); + if (sc == 0) { return false; } @@ -505,7 +510,7 @@ bool Terminal::SendKeyEvent(const WORD vkey, // is the underlying ASCII character (e.g. A-Z) on the keyboard in our case. // See GH#5525/GH#6211 for more details const auto isSuppressedAltGrAlias = !_altGrAliasing && states.IsAltPressed() && states.IsCtrlPressed() && !states.IsAltGrPressed(); - const auto ch = isSuppressedAltGrAlias ? UNICODE_NULL : _CharacterFromKeyEvent(vkey, scanCode, states); + const auto ch = isSuppressedAltGrAlias ? UNICODE_NULL : _CharacterFromKeyEvent(vkey, sc, states); // Delegate it to the character event handler if this key event can be // mapped to one (see method description above). For Alt+key combinations @@ -520,7 +525,7 @@ bool Terminal::SendKeyEvent(const WORD vkey, return false; } - KeyEvent keyEv{ keyDown, 1, vkey, scanCode, ch, states.Value() }; + KeyEvent keyEv{ keyDown, 1, vkey, sc, ch, states.Value() }; return _terminalInput->HandleKey(&keyEv); } @@ -637,8 +642,6 @@ WORD Terminal::_VirtualKeyFromCharacter(const wchar_t ch) noexcept wchar_t Terminal::_CharacterFromKeyEvent(const WORD vkey, const WORD scanCode, const ControlKeyStates states) noexcept try { - const auto sc = scanCode != 0 ? scanCode : _ScanCodeFromVirtualKey(vkey); - // We might want to use GetKeyboardState() instead of building our own keyState. // The question is whether that's necessary though. For now it seems to work fine as it is. std::array keyState = {}; @@ -657,7 +660,7 @@ try // * If bit 0 is set, a menu is active. // If this flag is not specified ToUnicodeEx will send us character events on certain Alt+Key combinations (e.g. Alt+Arrow-Up). // * If bit 2 is set, keyboard state is not changed (Windows 10, version 1607 and newer) - const auto result = ToUnicodeEx(vkey, sc, keyState.data(), buffer.data(), gsl::narrow_cast(buffer.size()), 0b101, nullptr); + const auto result = ToUnicodeEx(vkey, scanCode, keyState.data(), buffer.data(), gsl::narrow_cast(buffer.size()), 0b101, nullptr); // TODO:GH#2853 We're only handling single UTF-16 code points right now, since that's the only thing KeyEvent supports. return result == 1 || result == -1 ? buffer.at(0) : 0; diff --git a/src/cascadia/UnitTests_TerminalCore/InputTest.cpp b/src/cascadia/UnitTests_TerminalCore/InputTest.cpp index 27b114793f7..131e410d0fc 100644 --- a/src/cascadia/UnitTests_TerminalCore/InputTest.cpp +++ b/src/cascadia/UnitTests_TerminalCore/InputTest.cpp @@ -71,9 +71,7 @@ namespace TerminalCoreUnitTests { // Certain applications like AutoHotKey and its keyboard remapping feature, // send us key events using SendInput() whose values are outside of the valid range. - // We don't want to handle those events as we're probably going to get a proper followup character event. VERIFY_IS_FALSE(term.SendKeyEvent(0, 123, {}, true)); VERIFY_IS_FALSE(term.SendKeyEvent(255, 123, {}, true)); - VERIFY_IS_FALSE(term.SendKeyEvent(123, 0, {}, true)); } }