Mirrors/PowerToys
1
0
Fork 0
mirror of https://github.com/microsoft/PowerToys.git synced 2026-07-09 12:00:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

[Keyboard Manager] Fix stuck modifiers and dropped key-to-text remaps (#48571)

Browse Source 
## 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
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 it so the release is inert. The dummy +
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 delivers `WM_SYSKEYDOWN`, so the previous `WM_KEYDOWN`-only
guard silently dropped the remap whenever Alt was down.
3. **Route `Helpers::SendTextInput` through `InputInterface`** instead
of calling Win32 `SendInput` directly. 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.
4. **Never re-press released modifiers.** Once a modifier key-up is
injected, `GetAsyncKeyState` reports it as up, so re-pressing risks
leaving it stuck if the user let go during injection. Leaving it
released is always safe.

## Testing
- New `MockedInput` failure 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 #2 by asserting the remap still fires (and releases the
held Alt) when the key arrives as `WM_SYSKEYDOWN`.
- Full Keyboard Manager engine suite: **98/98 passing**, Release x64,
against current `main`.

This is one of a small set of related "stuck key" hardening fixes; each
stands alone.

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Clint Rutkas
2026-06-24 20:10:35 -07:00
committed by GitHub
parent 3331bdf02a
commit a864d421fc
11 changed files with 433 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

118
src/modules/keyboardmanager/KeyboardManagerEngineLibrary/KeyboardEventHandlers.cpp
View File

@@ -122,6 +122,21 @@ namespace KeyboardEventHandlers
key_count = std::get<Shortcut>(it->second).Size();
}
const DWORD sourceKey = data->lParam->vkCode;
const bool isKeyUp = (data->wParam == WM_KEYUP || data->wParam == WM_SYSKEYUP);
// If the matching key-down injection was blocked earlier, we passed the
// original key-down through to the foreground app to keep the key alive.
// The corresponding key-up must be passed through as well; otherwise the
// physical key is stranded DOWN (its down reached the app, but its up would
// be swallowed by the remap). Key-down and key-up arrive as separate hook
// events, so this is the cross-invocation counterpart of the key-down
// passthrough handled below.
if (isKeyUp && state.ConsumeSingleKeyRemapInjectionFailed(sourceKey))
{
return 0;
}
std::vector<INPUT> keyEventList;
// Handle remaps to VK_WIN_BOTH
@@ -177,7 +192,25 @@ namespace KeyboardEventHandlers
}
}
ii.SendVirtualInput(keyEventList);
if (!ii.SendVirtualInput(keyEventList))
{
// Injection was blocked (e.g. by UIPI). Return 0 so the ORIGINAL key is
// passed through instead of being swallowed, leaving no dead key. For a
// key-down, remember that we passed it through so the matching key-up is
// passed through too (handled above), preventing a key stranded DOWN.
if (!isKeyUp)
{
state.SetSingleKeyRemapInjectionFailed(sourceKey, true);
}
return 0;
}
// Injection succeeded; drop any stale passthrough marker for this key so its
// key-up follows the normal (suppressed) path.
if (!isKeyUp)
{
state.SetSingleKeyRemapInjectionFailed(sourceKey, false);
}
if (data->wParam == WM_KEYDOWN || data->wParam == WM_SYSKEYDOWN)
{
@@ -552,9 +585,12 @@ namespace KeyboardEventHandlers
// Send modifier release events first, then send text directly
// (SendTextInput handles multiline by flushing between chunks)
ii.SendVirtualInput(keyEventList);
if (!ii.SendVirtualInput(keyEventList))
{
return 0;
}
keyEventList.clear();
Helpers::SendTextInput(remapping);
Helpers::SendTextInput(remapping, ii);
}
it->second.isShortcutInvoked = true;
@@ -566,7 +602,10 @@ namespace KeyboardEventHandlers
Logger::trace(L"ChordKeyboardHandler:keyEventList.size:{}", keyEventList.size());
ii.SendVirtualInput(keyEventList);
if (!ii.SendVirtualInput(keyEventList))
{
return 0;
}
if (activatedApp.has_value())
{
if (remapToKey)
@@ -705,7 +744,10 @@ namespace KeyboardEventHandlers
state.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp);
}
ii.SendVirtualInput(keyEventList);
if (!ii.SendVirtualInput(keyEventList))
{
return 0;
}
return 1;
}
@@ -735,12 +777,14 @@ namespace KeyboardEventHandlers
else if (remapToText)
{
auto& remapping = std::get<std::wstring>(it->second.targetShortcut);
ii.SendVirtualInput(keyEventList);
Helpers::SendTextInput(remapping);
Helpers::SendTextInput(remapping, ii);
return 1;
}
ii.SendVirtualInput(keyEventList);
if (!ii.SendVirtualInput(keyEventList))
{
return 0;
}
return 1;
}
@@ -827,7 +871,10 @@ namespace KeyboardEventHandlers
}
}
ii.SendVirtualInput(keyEventList);
if (!ii.SendVirtualInput(keyEventList))
{
return 0;
}
return 1;
}
@@ -952,7 +999,10 @@ namespace KeyboardEventHandlers
state.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp);
}
ii.SendVirtualInput(keyEventList);
if (!ii.SendVirtualInput(keyEventList))
{
return 0;
}
return 1;
}
else
@@ -1021,7 +1071,10 @@ namespace KeyboardEventHandlers
state.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp);
}
ii.SendVirtualInput(keyEventList);
if (!ii.SendVirtualInput(keyEventList))
{
return 0;
}
return 1;
}
else
@@ -1799,8 +1852,9 @@ namespace KeyboardEventHandlers
return 0;
}
// Only send the text on keydown event
if (data->wParam != WM_KEYDOWN)
// Only send the text on key-down events. WM_SYSKEYDOWN is sent instead of
// WM_KEYDOWN while Alt is held, so accept it too or the remap silently drops.
if (data->wParam != WM_KEYDOWN && data->wParam != WM_SYSKEYDOWN)
{
return 0;
}
@@ -1811,7 +1865,43 @@ namespace KeyboardEventHandlers
return 0;
}
Helpers::SendTextInput(*remapping);
// Release held modifiers before text injection to prevent Ctrl+text corruption
constexpr int modifierKeys[] = { VK_LCONTROL, VK_RCONTROL, VK_LSHIFT, VK_RSHIFT, VK_LMENU, VK_RMENU, VK_LWIN, VK_RWIN };
std::vector<INPUT> releaseEvents;
// A dummy key event must precede the modifier releases so that releasing a
// held Win (Start Menu) or Alt (menu bar) does not trigger its lone-press
// action when we inject the modifier key-up.
Helpers::SetDummyKeyEvent(releaseEvents, KeyboardManagerConstants::KEYBOARDMANAGER_SHORTCUT_FLAG);
bool anyModifierHeld = false;
for (int vk : modifierKeys)
{
if (ii.GetVirtualKeyState(vk))
{
Helpers::SetKeyEvent(releaseEvents, INPUT_KEYBOARD, static_cast<WORD>(vk), KEYEVENTF_KEYUP, KeyboardManagerConstants::KEYBOARDMANAGER_SHORTCUT_FLAG);
anyModifierHeld = true;
}
}
// Only inject the dummy + modifier releases when a modifier was actually held.
if (anyModifierHeld)
{
if (!ii.SendVirtualInput(releaseEvents))
{
return 0;
}
}
Helpers::SendTextInput(*remapping, ii);
// Intentionally do NOT re-press the released modifiers. Once we inject a
// KEYUP for a modifier, GetAsyncKeyState (and therefore GetVirtualKeyState)
// reports it as up, so there is no reliable way to tell whether the user is
// still physically holding the key or has released it. Re-pressing
// unconditionally would risk leaving a modifier stuck down if the user let
// go during injection — the exact failure this change set prevents. Leaving
// the modifier released is always safe: the user taps it again to re-engage.
return 1;
}