[Keyboard Manager] Fixed app-specific shortcut causing app to lose focus scenario (#4902)

* Fixed focus issue and added tests

* Changed key names

* Use constant instead of hardcoded empty string

src/modules/keyboardmanager/common/KeyboardManagerConstants.h

@@ -96,4 +96,7 @@ namespace KeyboardManagerConstants
 
     // String constant for the default app name in Remap shortcuts
     inline const std::wstring DefaultAppName = L"All Apps";
+    
+    // String constant to represent no activated application in app-specific shortcuts
+    inline const std::wstring NoActivatedApp = L"";
 }

src/modules/keyboardmanager/common/KeyboardManagerState.cpp

@@ -528,4 +528,16 @@ std::wstring KeyboardManagerState::GetCurrentConfigName()
 {
     std::lock_guard<std::mutex> lock(currentConfig_mutex);
     return currentConfig;
-}
+}
+
+// Sets the activated target application in app-specfic shortcut
+void KeyboardManagerState::SetActivatedApp(const std::wstring& appName)
+{
+    activatedAppSpecificShortcutTarget = appName;
+}
+
+// Gets the activated target application in app-specfic shortcut
+std::wstring KeyboardManagerState::GetActivatedApp()
+{
+    return activatedAppSpecificShortcutTarget;
+}

src/modules/keyboardmanager/common/KeyboardManagerState.h

@@ -71,6 +71,9 @@ private:
     std::map<DWORD, std::unique_ptr<KeyDelay>> keyDelays;
     std::mutex keyDelays_mutex;
 
+    // Stores the activated target application in app-specfic shortcut
+    std::wstring activatedAppSpecificShortcutTarget;
+
     // Display a key by appending a border Control as a child of the panel.
     void AddKeyToLayout(const StackPanel& panel, const winrt::hstring& key);
 
@@ -190,4 +193,10 @@ public:
 
     // Gets the Current Active Configuration Name.
     std::wstring GetCurrentConfigName();
+
+    // Sets the activated target application in app-specfic shortcut
+    void SetActivatedApp(const std::wstring& appName);
+
+    // Gets the activated target application in app-specfic shortcut
+    std::wstring GetActivatedApp();
 };

src/modules/keyboardmanager/dll/KeyboardEventHandlers.cpp

@@ -114,7 +114,7 @@ namespace KeyboardEventHandlers
     }
 
     // Function to a handle a shortcut remap
-    __declspec(dllexport) intptr_t HandleShortcutRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, std::map<Shortcut, RemapShortcut>& reMap, std::mutex& map_mutex) noexcept
+    __declspec(dllexport) intptr_t HandleShortcutRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, std::map<Shortcut, RemapShortcut>& reMap, std::mutex& map_mutex, KeyboardManagerState& keyboardManagerState, const std::wstring& activatedApp) noexcept
     {
         // The mutex should be unlocked before SendInput is called to avoid re-entry into the same mutex. More details can be found at https://github.com/microsoft/PowerToys/pull/1789#issuecomment-607555837
         std::unique_lock<std::mutex> lock(map_mutex);
@@ -260,6 +260,11 @@ namespace KeyboardEventHandlers
                     }
 
                     it.second.isShortcutInvoked = true;
+                    // If app specific shortcut is invoked, store the target application
+                    if (activatedApp != KeyboardManagerConstants::NoActivatedApp)
+                    {
+                        keyboardManagerState.SetActivatedApp(activatedApp);
+                    }
                     lock.unlock();
                     UINT res = ii.SendVirtualInput((UINT)key_count, keyEventList, sizeof(INPUT));
                     delete[] keyEventList;
@@ -359,6 +364,11 @@ namespace KeyboardEventHandlers
 
                     it.second.isShortcutInvoked = false;
                     it.second.winKeyInvoked = ModifierKey::Disabled;
+                    // If app specific shortcut has finished invoking, reset the target application
+                    if (activatedApp != KeyboardManagerConstants::NoActivatedApp)
+                    {
+                        keyboardManagerState.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp);
+                    }
                     lock.unlock();
 
                     // key count can be 0 if both shortcuts have same modifiers and the action key is not held down. delete will throw an error if keyEventList is empty
@@ -559,6 +569,11 @@ namespace KeyboardEventHandlers
 
                         it.second.isShortcutInvoked = false;
                         it.second.winKeyInvoked = ModifierKey::Disabled;
+                        // If app specific shortcut has finished invoking, reset the target application
+                        if (activatedApp != KeyboardManagerConstants::NoActivatedApp)
+                        {
+                            keyboardManagerState.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp);
+                        }
                         lock.unlock();
                         UINT res = ii.SendVirtualInput((UINT)key_count, keyEventList, sizeof(INPUT));
                         delete[] keyEventList;
@@ -571,6 +586,11 @@ namespace KeyboardEventHandlers
                 // If it was in isShortcutInvoked state and none of the above cases occur, then reset the flags
                 it.second.isShortcutInvoked = false;
                 it.second.winKeyInvoked = ModifierKey::Disabled;
+                // If app specific shortcut has finished invoking, reset the target application
+                if (activatedApp != KeyboardManagerConstants::NoActivatedApp)
+                {
+                    keyboardManagerState.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp);
+                }
             }
         }
 
@@ -583,7 +603,7 @@ namespace KeyboardEventHandlers
         // Check if the key event was generated by KeyboardManager to avoid remapping events generated by us.
         if (data->lParam->dwExtraInfo != KeyboardManagerConstants::KEYBOARDMANAGER_SHORTCUT_FLAG)
         {
-            bool result = HandleShortcutRemapEvent(ii, data, keyboardManagerState.osLevelShortcutReMap, keyboardManagerState.osLevelShortcutReMap_mutex);
+            bool result = HandleShortcutRemapEvent(ii, data, keyboardManagerState.osLevelShortcutReMap, keyboardManagerState.osLevelShortcutReMap_mutex, keyboardManagerState);
             return result;
         }
 
@@ -614,22 +634,34 @@ namespace KeyboardEventHandlers
             std::transform(process_name.begin(), process_name.end(), process_name.begin(), towlower);
 
             std::unique_lock<std::mutex> lock(keyboardManagerState.appSpecificShortcutReMap_mutex);
-            std::wstring query_string = process_name;
+            std::wstring query_string;
-            auto it = keyboardManagerState.appSpecificShortcutReMap.find(query_string);
 
-            // If no entry is found, search for the process name without it's file extension
+            std::map<std::wstring, std::map<Shortcut, RemapShortcut>>::iterator it;
-            if (it == keyboardManagerState.appSpecificShortcutReMap.end())
+            // Check if an app-specific shortcut is already activated
+            if (keyboardManagerState.GetActivatedApp() == KeyboardManagerConstants::NoActivatedApp)
             {
-                // Find index of the file extension
+                query_string = process_name;
-                size_t extensionIndex = process_name.find_last_of(L".");
+                it = keyboardManagerState.appSpecificShortcutReMap.find(query_string);
-                query_string = process_name.substr(0, extensionIndex);
+
+                // If no entry is found, search for the process name without it's file extension
+                if (it == keyboardManagerState.appSpecificShortcutReMap.end())
+                {
+                    // Find index of the file extension
+                    size_t extensionIndex = process_name.find_last_of(L".");
+                    query_string = process_name.substr(0, extensionIndex);
+                    it = keyboardManagerState.appSpecificShortcutReMap.find(query_string);
+                }
+            }
+            else
+            {
+                query_string = keyboardManagerState.GetActivatedApp();
                 it = keyboardManagerState.appSpecificShortcutReMap.find(query_string);
             }
 
             if (it != keyboardManagerState.appSpecificShortcutReMap.end())
             {
                 lock.unlock();
-                bool result = HandleShortcutRemapEvent(ii, data, keyboardManagerState.appSpecificShortcutReMap[query_string], keyboardManagerState.appSpecificShortcutReMap_mutex);
+                bool result = HandleShortcutRemapEvent(ii, data, keyboardManagerState.appSpecificShortcutReMap[query_string], keyboardManagerState.appSpecificShortcutReMap_mutex, keyboardManagerState, query_string);
                 return result;
             }
         }

src/modules/keyboardmanager/dll/KeyboardEventHandlers.h

@@ -12,7 +12,7 @@ namespace KeyboardEventHandlers
     __declspec(dllexport) intptr_t HandleSingleKeyToggleToModEvent(InputInterface& ii, LowlevelKeyboardEvent* data, KeyboardManagerState& keyboardManagerState) noexcept;
 
     // Function to a handle a shortcut remap
-    __declspec(dllexport) intptr_t HandleShortcutRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, std::map<Shortcut, RemapShortcut>& reMap, std::mutex& map_mutex) noexcept;
+    __declspec(dllexport) intptr_t HandleShortcutRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, std::map<Shortcut, RemapShortcut>& reMap, std::mutex& map_mutex, KeyboardManagerState& keyboardManagerState, const std::wstring& activatedApp = KeyboardManagerConstants::NoActivatedApp) noexcept;
 
     // Function to a handle an os-level shortcut remap
     __declspec(dllexport) intptr_t HandleOSLevelShortcutRemapEvent(InputInterface& ii, LowlevelKeyboardEvent* data, KeyboardManagerState& keyboardManagerState) noexcept;

src/modules/keyboardmanager/test/AppSpecificShortcutRemappingTests.cpp

@@ -15,8 +15,8 @@ namespace RemappingLogicTests
     private:
         MockedInput mockedInputHandler;
         KeyboardManagerState testState;
-        std::wstring testApp1 = L"TestProcess1.exe";
+        std::wstring testApp1 = L"testtrocess1.exe";
-        std::wstring testApp2 = L"TestProcess2.exe";
+        std::wstring testApp2 = L"testprocess2.exe";
 
     public:
         TEST_METHOD_INITIALIZE(InitializeTestEnv)
@@ -92,5 +92,92 @@ namespace RemappingLogicTests
             Assert::AreEqual(mockedInputHandler.GetVirtualKeyState(VK_MENU), false);
             Assert::AreEqual(mockedInputHandler.GetVirtualKeyState(0x56), false);
         }
+
+        // Test if the the keyboard manager state's activated app is correctly set after an app specific remap takes place
+        TEST_METHOD (AppSpecificShortcut_ShouldSetCorrectActivatedApp_WhenRemapOccurs)
+        {
+            // Remap Ctrl+A to Alt+V
+            Shortcut src;
+            src.SetKey(VK_CONTROL);
+            src.SetKey(0x41);
+            Shortcut dest;
+            dest.SetKey(VK_MENU);
+            dest.SetKey(0x56);
+            testState.AddAppSpecificShortcut(testApp1, src, dest);
+
+            // Set the testApp as the foreground process
+            mockedInputHandler.SetForegroundProcess(testApp1);
+
+            const int nInputs = 2;
+            INPUT input[nInputs] = {};
+            input[0].type = INPUT_KEYBOARD;
+            input[0].ki.wVk = VK_CONTROL;
+            input[1].type = INPUT_KEYBOARD;
+            input[1].ki.wVk = 0x41;
+
+            // Send Ctrl+A keydown
+            mockedInputHandler.SendVirtualInput(nInputs, input, sizeof(INPUT));
+
+            // Activated app should be testApp1
+            Assert::AreEqual(testApp1, testState.GetActivatedApp());
+
+            input[0].type = INPUT_KEYBOARD;
+            input[0].ki.wVk = 0x41;
+            input[0].ki.dwFlags = KEYEVENTF_KEYUP;
+            input[1].type = INPUT_KEYBOARD;
+            input[1].ki.wVk = VK_CONTROL;
+            input[1].ki.dwFlags = KEYEVENTF_KEYUP;
+
+            // Release A then Ctrl
+            mockedInputHandler.SendVirtualInput(nInputs, input, sizeof(INPUT));
+
+            // Activated app should be empty string
+            Assert::AreEqual(std::wstring(KeyboardManagerConstants::NoActivatedApp), testState.GetActivatedApp());
+        }
+        // Test if the key states get cleared if foreground app changes after app-specific shortcut is invoked and then released
+        TEST_METHOD (AppSpecificShortcut_ShouldClearKeyStates_WhenForegroundAppChangesAfterShortcutIsPressedOnRelease)
+        {
+            // Remap Ctrl+A to Alt+Tab
+            Shortcut src;
+            src.SetKey(VK_CONTROL);
+            src.SetKey(0x41);
+            Shortcut dest;
+            dest.SetKey(VK_MENU);
+            dest.SetKey(VK_TAB);
+            testState.AddAppSpecificShortcut(testApp1, src, dest);
+
+            // Set the testApp as the foreground process
+            mockedInputHandler.SetForegroundProcess(testApp1);
+
+            const int nInputs = 2;
+            INPUT input[nInputs] = {};
+            input[0].type = INPUT_KEYBOARD;
+            input[0].ki.wVk = VK_CONTROL;
+            input[1].type = INPUT_KEYBOARD;
+            input[1].ki.wVk = 0x41;
+
+            // Send Ctrl+A keydown
+            mockedInputHandler.SendVirtualInput(nInputs, input, sizeof(INPUT));
+
+            // Set the testApp as the foreground process
+            mockedInputHandler.SetForegroundProcess(testApp2);
+
+            input[0].type = INPUT_KEYBOARD;
+            input[0].ki.wVk = 0x41;
+            input[0].ki.dwFlags = KEYEVENTF_KEYUP;
+            input[1].type = INPUT_KEYBOARD;
+            input[1].ki.wVk = VK_CONTROL;
+            input[1].ki.dwFlags = KEYEVENTF_KEYUP;
+
+            // Release A then Ctrl
+            mockedInputHandler.SendVirtualInput(nInputs, input, sizeof(INPUT));
+
+
+            // Ctrl, A, Alt and Tab should all be false
+            Assert::AreEqual(mockedInputHandler.GetVirtualKeyState(VK_CONTROL), false);
+            Assert::AreEqual(mockedInputHandler.GetVirtualKeyState(0x41), false);
+            Assert::AreEqual(mockedInputHandler.GetVirtualKeyState(VK_MENU), false);
+            Assert::AreEqual(mockedInputHandler.GetVirtualKeyState(VK_TAB), false);
+        }
     };
 }

src/modules/keyboardmanager/test/TestHelpers.cpp

@@ -12,5 +12,12 @@ namespace TestHelpers
         input.SetForegroundProcess(L"");
         state.ClearSingleKeyRemaps();
         state.ClearOSLevelShortcuts();
+        state.ClearAppSpecificShortcuts();
+
+        // Allocate memory for the keyboardManagerState activatedApp member to avoid CRT assert errors
+        std::wstring maxLengthString;
+        maxLengthString.resize(MAX_PATH);
+        state.SetActivatedApp(maxLengthString);
+        state.SetActivatedApp(KeyboardManagerConstants::NoActivatedApp);
     }
 }