[KBM Editor] fix crash when mapping left and right modifier to the combined key. (#12999)

* [KBM Editor] Don't combine keys to same key

* Avoid crashes when flyouts can't be shown yet

* Disallow mapping of left or right key to combined

* Refactor remap to combined key check

* Add log message when flyout fails to load

src/modules/keyboardmanager/KeyboardManagerEditorLibrary/BufferValidationHelpers.cpp

@@ -3,6 +3,7 @@
 
 #include <common/interop/shared_constants.h>
 #include <keyboardmanager/common/KeyboardManagerConstants.h>
+#include <keyboardmanager/common/Helpers.h>
 
 #include "KeyboardManagerEditorStrings.h"
 #include "KeyDropDownControl.h"
@@ -12,6 +13,13 @@
 
 namespace BufferValidationHelpers
 {
+    // Helper function to verify if a key is being remapped to/from its combined key
+    bool IsKeyRemappingToItsCombinedKey(DWORD keyCode1, DWORD keyCode2)
+    {
+        return (keyCode1 == Helpers::GetCombinedKey(keyCode1) || keyCode2 == Helpers::GetCombinedKey(keyCode2)) &&
+               Helpers::GetCombinedKey(keyCode1) == Helpers::GetCombinedKey(keyCode2);
+    }
+
     // Function to validate and update an element of the key remap buffer when the selection has changed
     ShortcutErrorType ValidateAndUpdateKeyBufferElement(int rowIndex, int colIndex, int selectedKeyCode, RemapBuffer& remapBuffer)
     {
@@ -23,7 +31,8 @@ namespace BufferValidationHelpers
             // Check if the value being set is the same as the other column
             if (remapBuffer[rowIndex].first[std::abs(int(colIndex) - 1)].index() == 0)
             {
-                if (std::get<DWORD>(remapBuffer[rowIndex].first[std::abs(int(colIndex) - 1)]) == selectedKeyCode)
+                DWORD otherColumnKeyCode = std::get<DWORD>(remapBuffer[rowIndex].first[std::abs(int(colIndex) - 1)]);
+                if (otherColumnKeyCode == selectedKeyCode || IsKeyRemappingToItsCombinedKey(selectedKeyCode, otherColumnKeyCode))
                 {
                     errorType = ShortcutErrorType::MapToSameKey;
                 }
@@ -251,7 +260,9 @@ namespace BufferValidationHelpers
                 // If key to key
                 if (remapBuffer[rowIndex].first[std::abs(int(colIndex) - 1)].index() == 0)
                 {
-                    if (std::get<DWORD>(remapBuffer[rowIndex].first[std::abs(int(colIndex) - 1)]) == std::get<DWORD>(tempShortcut) && std::get<DWORD>(remapBuffer[rowIndex].first[std::abs(int(colIndex) - 1)]) != NULL && std::get<DWORD>(tempShortcut) != NULL)
+                    DWORD otherColumnKeyCode = std::get<DWORD>(remapBuffer[rowIndex].first[std::abs(int(colIndex) - 1)]);
+                    DWORD shortcutKeyCode = std::get<DWORD>(tempShortcut);
+                    if ((otherColumnKeyCode == shortcutKeyCode || IsKeyRemappingToItsCombinedKey(otherColumnKeyCode, shortcutKeyCode)) && otherColumnKeyCode != NULL && shortcutKeyCode != NULL)
                     {
                         errorType = ShortcutErrorType::MapToSameKey;
                     }

src/modules/keyboardmanager/KeyboardManagerEditorLibrary/BufferValidationHelpers.h

@@ -14,6 +14,9 @@ namespace BufferValidationHelpers
         ClearUnusedDropDowns
     };
 
+    // Helper function to verify if a key is being remapped to/from its combined key
+    bool IsKeyRemappingToItsCombinedKey(DWORD keyCode1, DWORD keyCode2);
+
     // Function to validate and update an element of the key remap buffer when the selection has changed
     ShortcutErrorType ValidateAndUpdateKeyBufferElement(int rowIndex, int colIndex, int selectedKeyCode, RemapBuffer& remapBuffer);
 

src/modules/keyboardmanager/KeyboardManagerEditorLibrary/KeyDropDownControl.cpp

@@ -376,7 +376,15 @@ void KeyDropDownControl::SetDropDownError(ComboBox currentDropDown, hstring mess
 {
     currentDropDown.SelectedIndex(-1);
     warningMessage.as<TextBlock>().Text(message);
-    currentDropDown.ContextFlyout().ShowAttachedFlyout((FrameworkElement)dropDown.as<ComboBox>());
+    try
+    {
+        currentDropDown.ContextFlyout().ShowAttachedFlyout((FrameworkElement)dropDown.as<ComboBox>());
+    }
+    catch (winrt::hresult_error const&)
+    {
+        // If it's loading and some remaps are invalid from previous configs, avoid crashing when flyouts can't be showed yet.
+        Logger::error(L"Failed to show dropdown error flyout: {}", message);
+    }
 }
 
 // Function to add a shortcut to the UI control as combo boxes

src/modules/keyboardmanager/KeyboardManagerEditorLibrary/LoadingAndSavingRemappingHelper.cpp

@@ -2,6 +2,7 @@
 #include "LoadingAndSavingRemappingHelper.h"
 
 #include <set>
+#include <variant>
 #include <common/interop/shared_constants.h>
 #include <keyboardmanager/common/MappingConfiguration.h>
 
@@ -88,6 +89,11 @@ namespace LoadingAndSavingRemappingHelper
             // If they are mapped to the same key, delete those entries and set the common version
             if (table[leftKey] == table[rightKey])
             {
+                if (std::holds_alternative<DWORD>(table[leftKey]) && std::get<DWORD>(table[leftKey]) == combinedKey)
+                {
+                    // Avoid mapping a key to itself when the combined key is equal to the resulting mapping.
+                    return;
+                }
                 table[combinedKey] = table[leftKey];
                 table.erase(leftKey);
                 table.erase(rightKey);

src/modules/keyboardmanager/common/Helpers.cpp

@@ -15,6 +15,27 @@ namespace Helpers
         return (GetKeyType(key) != KeyType::Action);
     }
 
+    // Function to get the combined key for modifier keys
+    DWORD GetCombinedKey(DWORD key)
+    {
+        switch (key) {
+        case VK_LWIN:
+        case VK_RWIN:
+            return CommonSharedConstants::VK_WIN_BOTH;
+        case VK_LCONTROL:
+        case VK_RCONTROL:
+            return VK_CONTROL;
+        case VK_LMENU:
+        case VK_RMENU:
+            return VK_MENU;
+        case VK_LSHIFT:
+        case VK_RSHIFT:
+            return VK_SHIFT;
+        default:
+            return key;
+        }
+    }
+
     // Function to get the type of the key
     KeyType GetKeyType(DWORD key)
     {

src/modules/keyboardmanager/common/Helpers.h

@@ -18,6 +18,9 @@ namespace Helpers
     // Function to check if the key is a modifier key
     bool IsModifierKey(DWORD key);
 
+    // Function to get the combined key for modifier keys
+    DWORD GetCombinedKey(DWORD key);
+
     // Function to get the type of the key
     KeyType GetKeyType(DWORD key);