Fixed Ctrl+Alt shortcuts conflicting with AltGr (#2235)

This moves the detection of AltGr keypresses in front of the shortcut
handling. This allows one to have Ctrl+Alt shortcuts, while
simultaneously being able to use the AltGr key for special characters.

src/cascadia/TerminalControl/TermControl.cpp

@@ -619,35 +619,41 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         }
 
         const auto modifiers = _GetPressedModifierKeys();
-        const auto vkey = static_cast<WORD>(e.OriginalKey());
 
+        // AltGr key combinations don't always contain any meaningful,
+        // pretranslated unicode character during WM_KEYDOWN.
+        // E.g. on a German keyboard AltGr+Q should result in a "@" character,
+        // but actually results in "Q" with Alt and Ctrl modifier states.
+        // By returning false though, we can abort handling this WM_KEYDOWN
+        // event and let the WM_CHAR handler kick in, which will be
+        // provided with an appropriate unicode character.
+        //
+        // GH#2235: Make sure to handle AltGr before trying keybindings,
+        // so Ctrl+Alt keybindings won't eat an AltGr keypress.
+        if (modifiers.IsAltGrPressed())
+        {
+            _HandleVoidKeyEvent();
+            e.Handled(false);
+            return;
+        }
+
+        const auto vkey = static_cast<WORD>(e.OriginalKey());
         bool handled = false;
+
         auto bindings = _settings.KeyBindings();
         if (bindings)
         {
-            KeyChord chord(
+            handled = bindings.TryKeyChord({
                 modifiers.IsCtrlPressed(),
                 modifiers.IsAltPressed(),
                 modifiers.IsShiftPressed(),
-                vkey);
-            handled = bindings.TryKeyChord(chord);
+                vkey,
+            });
         }
 
         if (!handled)
         {
-            _terminal->ClearSelection();
-            // If the terminal translated the key, mark the event as handled.
-            // This will prevent the system from trying to get the character out
-            // of it and sending us a CharacterRecieved event.
-            handled = _terminal->SendKeyEvent(vkey, modifiers);
-
-            if (_cursorTimer.has_value())
-            {
-                // Manually show the cursor when a key is pressed. Restarting
-                // the timer prevents flickering.
-                _terminal->SetCursorVisible(true);
-                _cursorTimer.value().Start();
-            }
+            handled = _TrySendKeyEvent(vkey, modifiers);
         }
 
         // Manually prevent keyboard navigation with tab. We want to send tab to
@@ -661,6 +667,45 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         e.Handled(handled);
     }
 
+    // Method Description:
+    // - Some key events cannot be handled (e.g. AltGr combinations) and are
+    //   delegated to the character handler. Just like with _TrySendKeyEvent(),
+    //   the character handler counts on us though to:
+    // - Clears the current selection.
+    // - Makes the cursor briefly visible during typing.
+    void TermControl::_HandleVoidKeyEvent()
+    {
+        _TrySendKeyEvent(0, {});
+    }
+
+    // Method Description:
+    // - Send this particular key event to the terminal.
+    //   See Terminal::SendKeyEvent for more information.
+    // - Clears the current selection.
+    // - Makes the cursor briefly visible during typing.
+    // Arguments:
+    // - vkey: The vkey of the key pressed.
+    // - states: The Microsoft::Terminal::Core::ControlKeyStates representing the modifier key states.
+    bool TermControl::_TrySendKeyEvent(WORD vkey, const ControlKeyStates modifiers)
+    {
+        _terminal->ClearSelection();
+
+        // If the terminal translated the key, mark the event as handled.
+        // This will prevent the system from trying to get the character out
+        // of it and sending us a CharacterRecieved event.
+        const auto handled = vkey ? _terminal->SendKeyEvent(vkey, modifiers) : true;
+
+        if (_cursorTimer.has_value())
+        {
+            // Manually show the cursor when a key is pressed. Restarting
+            // the timer prevents flickering.
+            _terminal->SetCursorVisible(true);
+            _cursorTimer.value().Start();
+        }
+
+        return handled;
+    }
+
     // Method Description:
     // - handle a mouse click event. Begin selection process.
     // Arguments:

src/cascadia/TerminalControl/TermControl.h

@@ -164,6 +164,8 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
         static Windows::UI::Xaml::Thickness _ParseThicknessFromPadding(const hstring padding);
 
         ::Microsoft::Terminal::Core::ControlKeyStates _GetPressedModifierKeys() const;
+        void _HandleVoidKeyEvent();
+        bool _TrySendKeyEvent(WORD vkey, ::Microsoft::Terminal::Core::ControlKeyStates modifiers);
 
         const COORD _GetTerminalPosition(winrt::Windows::Foundation::Point cursorPosition);
         const unsigned int _NumberOfClicks(winrt::Windows::Foundation::Point clickPos, Timestamp clickTime);

src/cascadia/TerminalCore/Terminal.cpp

@@ -210,18 +210,6 @@ bool Terminal::SendKeyEvent(const WORD vkey, const ControlKeyStates states)
         _NotifyScrollEvent();
     }
 
-    // AltGr key combinations don't always contain any meaningful,
-    // pretranslated unicode character during WM_KEYDOWN.
-    // E.g. on a German keyboard AltGr+Q should result in a "@" character,
-    // but actually results in "Q" with Alt and Ctrl modifier states.
-    // By returning false though, we can abort handling this WM_KEYDOWN
-    // event and let the WM_CHAR handler kick in, which will be
-    // provided with an appropriate unicode character.
-    if (states.IsAltGrPressed())
-    {
-        return false;
-    }
-
     // Alt key sequences _require_ the char to be in the keyevent. If alt is
     // pressed, manually get the character that's being typed, and put it in the
     // KeyEvent.