[Keyboard Manager] Fix stuck modifiers and dropped key-to-text remaps#48571
[Keyboard Manager] Fix stuck modifiers and dropped key-to-text remaps#48571crutkas wants to merge 7 commits into
Conversation
…text remap - SendVirtualInput now returns bool; callers pass through original key on failure - Release held modifiers before text injection in single-key-to-text remap to prevent Ctrl+text corruption Fixes: microsoft#17035, microsoft#29015, microsoft#9778 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…urns, add tests - Add VK_LWIN/VK_RWIN to modifierKeys[] in HandleSingleKeyToTextRemapEvent so Win key is released before text injection (prevents Win+h/e/l shortcuts) - Check SendVirtualInput return on modifier release — pass through original key if release fails (prevents Ctrl+text corruption on SendInput failure) - Add MockedInput::SetKeyboardState for test setup - Add ClearSingleKeyToTextRemaps to ResetTestEnv - Add 2 new tests: - HandleSingleKeyToTextRemapEvent_ShouldReleaseAndRestoreWinKey_WhenWinKeyIsHeld - HandleSingleKeyToTextRemapEvent_ShouldReleaseAndRestoreCtrl_WhenCtrlIsHeld All 94 KBM unit tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The single-key-to-text handler releases any held modifiers before injecting Unicode text (so Ctrl/Shift/etc. don't corrupt it) and previously re-pressed them afterwards. Re-pressing is unsafe: once we inject a modifier KEYUP, GetAsyncKeyState reports that key as up, so we cannot tell whether the user is still physically holding it or has let go. If they released it during injection, the re-press would leave the modifier stuck down - the exact bug this series fixes. Leave released modifiers released; the user taps the key again to re-engage it. Update the tests to assert modifiers are never re-pressed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ection Three correctness fixes for the single-key-to-text remap path plus test coverage for the stuck-key hardening: 1. Insert a dummy key event before releasing held modifiers. Releasing a lone Win or Alt key-up otherwise triggers the Start Menu / menu bar; the dummy key absorbs the modifier so the release is inert. The dummy and releases are only injected when a modifier is actually held. 2. Accept WM_SYSKEYDOWN as well as WM_KEYDOWN. While Alt is held the system sends WM_SYSKEYDOWN, so the previous WM_KEYDOWN-only guard silently dropped the remap whenever Alt was down. 3. Route Helpers::SendTextInput through the InputInterface instead of calling Win32 SendInput directly. Previously the unit tests for this handler injected real keystrokes into the OS during the test run. Text is still flushed per character to preserve the existing batching workaround. Also add a MockedInput failure seam (SetSendVirtualInputShouldFail) and tests: RemappedKey_ShouldPassOriginalKeyThrough_WhenInjectionFails verifies the handler passes the original key through when injection fails - the core stuck-key behavior introduced by this change set, previously untestable because the mock always succeeded; and HandleSingleKeyToTextRemapEvent_ShouldFireAndReleaseAlt_WhenAltIsHeld covers fix 2 by asserting the remap still fires (releasing the held Alt) when the key arrives as WM_SYSKEYDOWN. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
HandleSingleKeyRemapEvent processes key-down and key-up as separate hook invocations. When a remap key-down injection was blocked (so the original key-down passed through, leaving the key physically DOWN) but the later key-up injection succeeded, the original key-up was suppressed and the physical key was stranded DOWN. Track on State whether a remap key-down injection for a source key was passed through, and on the matching key-up pass the original key-up through as well so the key is released. Add a unit test covering this asymmetric failure (99/99 passing). Also harden Input::SendVirtualInput: return false only when SendInput injects nothing (fully blocked, so it is safe to pass the original key through). On a partial injection, log a warning and return true so the caller suppresses the original event instead of layering it on top of a half-applied remap, which could leave a key or modifier stuck. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
…ment The check-spelling CI rejects ", otherwise" (it wants "; otherwise" or ". Otherwise"). Reword the State member comment accordingly. Comment-only change; engine test project still builds and 99/99 tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, 👼 SARIF report, or 📝 job summary for details.
See ❌ Event descriptions for more information. Some files were automatically ignored 🙈These sample patterns would exclude them: You should consider adding them to: File matching is via Perl regular expressions. To check these files, more of their words need to be in the dictionary than not. You can use To update file exclusions, you could run the following commands... in a clone of the git@github.com:crutkas/autoUpgradeAttempt.git repository curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/cfb6f7e75bbfc89c71eaa30366d0c166f1bd9c8c/apply.pl' |
perl - 'https://github.com/microsoft/PowerToys/actions/runs/27455004404/attempts/1' &&
git commit -m 'Update check-spelling metadata'Forbidden patterns 🙅 (1)In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves. These forbidden patterns matched content: Should be
|
Summary
Fixes stuck modifier keys and silently-dropped remaps on Keyboard Manager's single key → text path, and adds unit coverage (including a mockable injection-failure seam).
What this changes
WM_SYSKEYDOWNas well asWM_KEYDOWN. While Alt is held the system deliversWM_SYSKEYDOWN, so the previousWM_KEYDOWN-only guard silently dropped the remap whenever Alt was down.Helpers::SendTextInputthroughInputInterfaceinstead of calling Win32SendInputdirectly. Besides making the path mockable, this stops the existing unit tests from injecting real keystrokes into the OS during a test run. Text is still flushed per character to preserve the existing batching workaround.GetAsyncKeyStatereports it as up, so re-pressing risks leaving it stuck if the user let go during injection. Leaving it released is always safe.Testing
MockedInputfailure seam (SetSendVirtualInputShouldFail).RemappedKey_ShouldPassOriginalKeyThrough_WhenInjectionFails— verifies the original key is passed through when injection fails (the core stuck-key behavior, previously untestable because the mock always succeeded).HandleSingleKeyToTextRemapEvent_ShouldFireAndReleaseAlt_WhenAltIsHeld— covers fix fix a couple typos #2 by asserting the remap still fires (and releases the held Alt) when the key arrives asWM_SYSKEYDOWN.main.This is one of a small set of related "stuck key" hardening fixes; each stands alone.