Skip to content

Commit 0bb92cb

Browse files
crutkasCopilot
andcommitted
fix(kbm): Add Win key to modifier release, check SendVirtualInput returns, 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>
1 parent 70addf9 commit 0bb92cb

5 files changed

Lines changed: 78 additions & 2 deletions

File tree

src/modules/keyboardmanager/KeyboardManagerEngineLibrary/KeyboardEventHandlers.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1824,7 +1824,7 @@ namespace KeyboardEventHandlers
18241824
}
18251825

18261826
// Release held modifiers before text injection to prevent Ctrl+text corruption
1827-
constexpr int modifierKeys[] = { VK_LCONTROL, VK_RCONTROL, VK_LSHIFT, VK_RSHIFT, VK_LMENU, VK_RMENU };
1827+
constexpr int modifierKeys[] = { VK_LCONTROL, VK_RCONTROL, VK_LSHIFT, VK_RSHIFT, VK_LMENU, VK_RMENU, VK_LWIN, VK_RWIN };
18281828
std::vector<INPUT> releaseEvents;
18291829
std::vector<int> releasedKeys;
18301830

@@ -1839,7 +1839,10 @@ namespace KeyboardEventHandlers
18391839

18401840
if (!releaseEvents.empty())
18411841
{
1842-
ii.SendVirtualInput(releaseEvents);
1842+
if (!ii.SendVirtualInput(releaseEvents))
1843+
{
1844+
return 0;
1845+
}
18431846
}
18441847

18451848
Helpers::SendTextInput(*remapping);
@@ -1853,6 +1856,9 @@ namespace KeyboardEventHandlers
18531856
Helpers::SetKeyEvent(restoreEvents, INPUT_KEYBOARD, static_cast<WORD>(vk), 0, KeyboardManagerConstants::KEYBOARDMANAGER_SHORTCUT_FLAG);
18541857
}
18551858
ii.SendVirtualInput(restoreEvents);
1859+
// Note: not checking return here — failing to restore modifiers is
1860+
// less harmful than failing to release them (text already sent correctly).
1861+
// The user can tap the modifier to resync state.
18561862
}
18571863

18581864
return 1;

src/modules/keyboardmanager/KeyboardManagerEngineTest/MockedInput.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,12 @@ bool MockedInput::GetVirtualKeyState(int key)
130130
return keyboardState[key];
131131
}
132132

133+
// Function to set the state of a particular key for test setup
134+
void MockedInput::SetKeyboardState(int key, bool state)
135+
{
136+
keyboardState[key] = state;
137+
}
138+
133139
// Function to reset the mocked keyboard state
134140
void MockedInput::ResetKeyboardState()
135141
{

src/modules/keyboardmanager/KeyboardManagerEngineTest/MockedInput.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ namespace KeyboardManagerInput
4242
// Function to get the state of a particular key
4343
bool GetVirtualKeyState(int key);
4444

45+
// Function to set the state of a particular key for test setup
46+
void SetKeyboardState(int key, bool state);
47+
4548
// Function to reset the mocked keyboard state
4649
void ResetKeyboardState();
4750

src/modules/keyboardmanager/KeyboardManagerEngineTest/SingleKeyRemappingTests.cpp

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,4 +329,64 @@ namespace RemappingLogicTests
329329
Assert::AreEqual(mockedInputHandler.GetVirtualKeyState(0x56), false);
330330
}
331331
};
332+
333+
// Tests for single key to text remap modifier release logic
334+
TEST_CLASS (SingleKeyToTextRemapModifierTests)
335+
{
336+
private:
337+
KeyboardManagerInput::MockedInput mockedInputHandler;
338+
State testState;
339+
340+
public:
341+
TEST_METHOD_INITIALIZE(InitializeTestEnv)
342+
{
343+
TestHelpers::ResetTestEnv(mockedInputHandler, testState);
344+
345+
// Set HandleSingleKeyToTextRemapEvent as the hook procedure
346+
std::function<intptr_t(LowlevelKeyboardEvent*)> currentHookProc = std::bind(&KeyboardEventHandlers::HandleSingleKeyToTextRemapEvent, std::ref(mockedInputHandler), std::placeholders::_1, std::ref(testState));
347+
mockedInputHandler.SetHookProc(currentHookProc);
348+
}
349+
350+
// Test if Win key is released and restored when held during text remap
351+
TEST_METHOD (HandleSingleKeyToTextRemapEvent_ShouldReleaseAndRestoreWinKey_WhenWinKeyIsHeld)
352+
{
353+
// Remap X to text "hello"
354+
testState.AddSingleKeyToTextRemap(0x58, L"hello");
355+
356+
// Simulate LWin being held down
357+
mockedInputHandler.SetKeyboardState(VK_LWIN, true);
358+
Assert::AreEqual(true, mockedInputHandler.GetVirtualKeyState(VK_LWIN));
359+
360+
std::vector<INPUT> inputs{
361+
{ .type = INPUT_KEYBOARD, .ki = { .wVk = 0x58 } },
362+
};
363+
364+
// Send X keydown — handler should release LWin before text and restore after
365+
mockedInputHandler.SendVirtualInput(inputs);
366+
367+
// After the handler completes, LWin should be restored (re-pressed)
368+
Assert::AreEqual(true, mockedInputHandler.GetVirtualKeyState(VK_LWIN));
369+
}
370+
371+
// Test if Ctrl key is released and restored when held during text remap
372+
TEST_METHOD (HandleSingleKeyToTextRemapEvent_ShouldReleaseAndRestoreCtrl_WhenCtrlIsHeld)
373+
{
374+
// Remap X to text "hello"
375+
testState.AddSingleKeyToTextRemap(0x58, L"hello");
376+
377+
// Simulate LCtrl being held down
378+
mockedInputHandler.SetKeyboardState(VK_LCONTROL, true);
379+
Assert::AreEqual(true, mockedInputHandler.GetVirtualKeyState(VK_LCONTROL));
380+
381+
std::vector<INPUT> inputs{
382+
{ .type = INPUT_KEYBOARD, .ki = { .wVk = 0x58 } },
383+
};
384+
385+
// Send X keydown
386+
mockedInputHandler.SendVirtualInput(inputs);
387+
388+
// After the handler completes, LCtrl should be restored
389+
Assert::AreEqual(true, mockedInputHandler.GetVirtualKeyState(VK_LCONTROL));
390+
}
391+
};
332392
}

src/modules/keyboardmanager/KeyboardManagerEngineTest/TestHelpers.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ namespace TestHelpers
1515
state.ClearSingleKeyRemaps();
1616
state.ClearOSLevelShortcuts();
1717
state.ClearAppSpecificShortcuts();
18+
state.ClearSingleKeyToTextRemaps();
1819

1920
// Allocate memory for the keyboardManagerState activatedApp member to avoid CRT assert errors
2021
std::wstring maxLengthString;

0 commit comments

Comments
 (0)