Skip to content

Commit

Permalink
Fix SendInput handling (#7900)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
lhecker authored Oct 27, 2020
1 parent 1df3182 commit d51d8dc
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 43 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
autogenerated
CPPCORECHECK
Debian
filepath
inplace
KEYBDINPUT
WINVER
28 changes: 0 additions & 28 deletions .github/actions/spell-check/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ AHelper
ahz
AImpl
AInplace
akb
ALIGNRIGHT
alloc
allocing
Expand Down Expand Up @@ -62,7 +61,6 @@ apiset
apos
APPBARDATA
appconsult
appdata
APPICON
appium
applet
Expand Down Expand Up @@ -109,10 +107,8 @@ aumid
Authenticode
AUTOBUDDY
AUTOCHECKBOX
Autogenerated
autohide
AUTOHSCROLL
autologin
automagically
autopositioning
AUTORADIOBUTTON
Expand Down Expand Up @@ -393,7 +389,6 @@ CPINFOEX
cplinfo
cplusplus
cpp
cppcorecheck
cppcorecheckrules
cpprest
cpprestsdk
Expand Down Expand Up @@ -510,22 +505,18 @@ DDESHARE
DDevice
DEADCHAR
dealloc
debian
debolden
debounce
debugbreak
DECALN
DECANM
DECAUPSS
DECAWM
DECCKM
DECCOLM
DECEKBD
decf
DECKPAM
DECKPM
DECKPNM
DECLL
DECLRMM
decls
declspec
Expand All @@ -552,7 +543,6 @@ DECSEL
DECSET
DECSLPP
DECSLRM
DECSMBV
DECSMKR
DECSR
decstandar
Expand Down Expand Up @@ -716,7 +706,6 @@ EPres
ERASEBKGND
errno
errorlevel
esa
ETB
etcoreapp
ETW
Expand Down Expand Up @@ -771,7 +760,6 @@ fgetwc
fgidx
FILEDESCRIPTION
fileno
FILEPATH
FILESUBTYPE
FILESYSPATH
filesystem
Expand Down Expand Up @@ -932,7 +920,6 @@ GTP
guc
gui
guidatom
guidgenerator
GValue
GWL
GWLP
Expand Down Expand Up @@ -1105,7 +1092,6 @@ Inlines
INotify
inout
INPATHROOT
Inplace
inproc
Inputkeyinfo
INPUTPROCESSORPROFILE
Expand Down Expand Up @@ -1194,11 +1180,9 @@ kcud
kcuf
kcuu
Kd
keith
kernelbase
kernelbasestaging
keybinding
keybound
keychord
keydown
keyevent
Expand Down Expand Up @@ -1471,7 +1455,6 @@ namestream
Namquiseratal
nano
natvis
naws
nbsp
Nc
NCCALCSIZE
Expand Down Expand Up @@ -1553,7 +1536,6 @@ NOTNULL
NOTOPMOST
NOTRACK
NOTSUPPORTED
notypeopt
nouicompat
nounihan
NOUPDATE
Expand Down Expand Up @@ -1628,7 +1610,6 @@ opencon
openconsole
OPENIF
OPENLINK
openlogo
openps
opensource
openvt
Expand Down Expand Up @@ -1968,7 +1949,6 @@ resheader
resizable
resmimetype
restrictedcapabilities
restrictederrorinfo
resw
resx
retval
Expand All @@ -1995,7 +1975,6 @@ rgw
rgwch
rhs
ri
richturn
RIGHTALIGN
RIGHTBUTTON
riid
Expand Down Expand Up @@ -2069,7 +2048,6 @@ SCROLLSCALE
SCROLLSCREENBUFFER
Scrollup
Scrolluppage
Scs
scursor
sddl
sdeleted
Expand Down Expand Up @@ -2243,7 +2221,6 @@ subkey
SUBLANG
sublicensable
submenu
subnegotiation
subresource
subspan
substr
Expand All @@ -2254,7 +2231,6 @@ svg
swapchain
swapchainpanel
swappable
Switchto
SWMR
SWP
swprintf
Expand Down Expand Up @@ -2309,7 +2285,6 @@ technet
tellp
telnet
telnetd
telnetpp
templated
terminalcore
TERMINALSCROLLING
Expand Down Expand Up @@ -2706,12 +2681,10 @@ wintelnet
winternl
winuser
winuserp
winver
wistd
wixproj
wline
wlinestream
Wlk
wmain
WMSZ
wnd
Expand Down Expand Up @@ -2754,7 +2727,6 @@ WRunoff
WScript
wsl
WSLENV
wslhome
wsmatch
WSpace
wss
Expand Down
29 changes: 16 additions & 13 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,15 +472,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;
}
Expand All @@ -506,7 +511,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
Expand All @@ -521,7 +526,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);
}

Expand Down Expand Up @@ -638,8 +643,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<BYTE, 256> keyState = {};
Expand All @@ -658,7 +661,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<int>(buffer.size()), 0b101, nullptr);
const auto result = ToUnicodeEx(vkey, scanCode, keyState.data(), buffer.data(), gsl::narrow_cast<int>(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;
Expand Down
2 changes: 0 additions & 2 deletions src/cascadia/UnitTests_TerminalCore/InputTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

0 comments on commit d51d8dc

Please sign in to comment.