Add an openSystemMenu keybinding (#11086)

## Summary of the Pull Request
Basically undoes #10988  in favour of implementing it as described in #11018 

## PR Checklist
* [x] Closes #11018 
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [X] Tests added/passed
* [X] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [X] Schema updated.
* [x] I work here

## Validation Steps Performed

- alt+space opens the system menu by default
- when alt+space is bound, the keys do not get send to terminal
- right-click on the tab bar didn't break (still opens system menu at the location of the cursor)

doc/cascadia/profiles.schema.json

@@ -279,6 +279,7 @@
         "paste",
         "prevTab",
         "renameTab",
+        "openSystemMenu",
         "openTabRenamer",
         "quakeMode",
         "resetFontSize",

src/cascadia/TerminalApp/AppActionHandlers.cpp

@@ -882,6 +882,13 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
+    void TerminalPage::_HandleOpenSystemMenu(const IInspectable& /*sender*/,
+                                             const ActionEventArgs& args)
+    {
+        _OpenSystemMenuHandlers(*this, nullptr);
+        args.Handled(true);
+    }
+
     void TerminalPage::_HandleClearBuffer(const IInspectable& /*sender*/,
                                           const ActionEventArgs& args)
     {

src/cascadia/TerminalApp/AppLogic.h

@@ -175,6 +175,7 @@ namespace winrt::TerminalApp::implementation
         FORWARDED_TYPED_EVENT(RenameWindowRequested, Windows::Foundation::IInspectable, winrt::TerminalApp::RenameWindowRequestedArgs, _root, RenameWindowRequested);
         FORWARDED_TYPED_EVENT(IsQuakeWindowChanged, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, IsQuakeWindowChanged);
         FORWARDED_TYPED_EVENT(SummonWindowRequested, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, SummonWindowRequested);
+        FORWARDED_TYPED_EVENT(OpenSystemMenu, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, OpenSystemMenu);
         FORWARDED_TYPED_EVENT(QuitRequested, Windows::Foundation::IInspectable, Windows::Foundation::IInspectable, _root, QuitRequested);
 
 #ifdef UNIT_TESTING

src/cascadia/TerminalApp/AppLogic.idl

@@ -99,6 +99,7 @@ namespace TerminalApp
         event Windows.Foundation.TypedEventHandler<Object, Object> SettingsChanged;
         event Windows.Foundation.TypedEventHandler<Object, Object> IsQuakeWindowChanged;
         event Windows.Foundation.TypedEventHandler<Object, Object> SummonWindowRequested;
+        event Windows.Foundation.TypedEventHandler<Object, Object> OpenSystemMenu;
         event Windows.Foundation.TypedEventHandler<Object, Object> QuitRequested;
     }
 }

src/cascadia/TerminalApp/TerminalPage.h

@@ -132,6 +132,7 @@ namespace winrt::TerminalApp::implementation
         TYPED_EVENT(RenameWindowRequested, Windows::Foundation::IInspectable, winrt::TerminalApp::RenameWindowRequestedArgs);
         TYPED_EVENT(IsQuakeWindowChanged, IInspectable, IInspectable);
         TYPED_EVENT(SummonWindowRequested, IInspectable, IInspectable);
+        TYPED_EVENT(OpenSystemMenu, IInspectable, IInspectable);
         TYPED_EVENT(QuitRequested, IInspectable, IInspectable);
 
     private:

src/cascadia/TerminalApp/TerminalPage.idl

@@ -58,5 +58,6 @@ namespace TerminalApp
         event Windows.Foundation.TypedEventHandler<Object, RenameWindowRequestedArgs> RenameWindowRequested;
         event Windows.Foundation.TypedEventHandler<Object, Object> IsQuakeWindowChanged;
         event Windows.Foundation.TypedEventHandler<Object, Object> SummonWindowRequested;
+        event Windows.Foundation.TypedEventHandler<Object, Object> OpenSystemMenu;
     }
 }

src/cascadia/TerminalControl/TermControl.cpp

@@ -904,24 +904,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             return;
         }
 
-        if (vkey == VK_SPACE && modifiers.IsAltPressed())
-        {
-            if (const auto bindings = _settings.KeyBindings())
-            {
-                if (!bindings.IsKeyChordExplicitlyUnbound({ modifiers.IsCtrlPressed(), modifiers.IsAltPressed(), modifiers.IsShiftPressed(), modifiers.IsWinPressed(), vkey, scanCode }))
-                {
-                    // If we get here, it means that
-                    //      1. we do not have a command bound to alt+space
-                    //      2. alt+space was not explicitly unbound
-                    // That means that XAML handled the alt+space to open up the context menu, and
-                    // so we don't want to send anything to the terminal
-                    // TODO GH#11018: Add a new "openSystemMenu" keybinding
-                    e.Handled(true);
-                    return;
-                }
-            }
-        }
-
         if (_TrySendKeyEvent(vkey, scanCode, modifiers, keyDown))
         {
             e.Handled(true);

src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp

@@ -65,6 +65,7 @@ static constexpr std::string_view OpenWindowRenamerKey{ "openWindowRenamer" };
 static constexpr std::string_view GlobalSummonKey{ "globalSummon" };
 static constexpr std::string_view QuakeModeKey{ "quakeMode" };
 static constexpr std::string_view FocusPaneKey{ "focusPane" };
+static constexpr std::string_view OpenSystemMenuKey{ "openSystemMenu" };
 static constexpr std::string_view ClearBufferKey{ "clearBuffer" };
 static constexpr std::string_view MultipleActionsKey{ "multipleActions" };
 static constexpr std::string_view QuitKey{ "quit" };
@@ -369,6 +370,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
                 { ShortcutAction::GlobalSummon, L"" }, // Intentionally omitted, must be generated by GenerateName
                 { ShortcutAction::QuakeMode, RS_(L"QuakeModeCommandKey") },
                 { ShortcutAction::FocusPane, L"" }, // Intentionally omitted, must be generated by GenerateName
+                { ShortcutAction::OpenSystemMenu, RS_(L"OpenSystemMenuCommandKey") },
                 { ShortcutAction::ClearBuffer, L"" }, // Intentionally omitted, must be generated by GenerateName
                 { ShortcutAction::MultipleActions, L"" }, // Intentionally omitted, must be generated by GenerateName
                 { ShortcutAction::Quit, RS_(L"QuitCommandKey") },

src/cascadia/TerminalSettingsModel/AllShortcutActions.h

@@ -79,6 +79,7 @@
     ON_ALL_ACTIONS(GlobalSummon)           \
     ON_ALL_ACTIONS(QuakeMode)              \
     ON_ALL_ACTIONS(FocusPane)              \
+    ON_ALL_ACTIONS(OpenSystemMenu)         \
     ON_ALL_ACTIONS(ClearBuffer)            \
     ON_ALL_ACTIONS(MultipleActions)        \
     ON_ALL_ACTIONS(Quit)

src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw

@@ -178,6 +178,9 @@
   <data name="CloseWindowCommandKey" xml:space="preserve">
     <value>Close window</value>
   </data>
+  <data name="OpenSystemMenuCommandKey" xml:space="preserve">
+    <value>Open system menu</value>
+  </data>
   <data name="CommandPromptDisplayName" xml:space="preserve">
     <value>Command Prompt</value>
     <comment>This is the name of "Command Prompt", as localized in Windows. The localization here should match the one in the Windows product for "Command Prompt"</comment>

src/cascadia/TerminalSettingsModel/defaults.json

@@ -301,6 +301,7 @@
         { "command": "identifyWindow" },
         { "command": "openWindowRenamer" },
         { "command": "quakeMode", "keys":"win+sc(41)" },
+        { "command": "openSystemMenu", "keys": "alt+space" },
         { "command": "quit" },
 
         // Tab Management

src/cascadia/WindowsTerminal/AppHost.cpp

@@ -279,6 +279,7 @@ void AppHost::Initialize()
     _logic.SettingsChanged({ this, &AppHost::_HandleSettingsChanged });
     _logic.IsQuakeWindowChanged({ this, &AppHost::_IsQuakeWindowChanged });
     _logic.SummonWindowRequested({ this, &AppHost::_SummonWindowRequested });
+    _logic.OpenSystemMenu({ this, &AppHost::_OpenSystemMenu });
     _logic.QuitRequested({ this, &AppHost::_RequestQuitAll });
 
     _window->UpdateTitle(_logic.Title());
@@ -1062,6 +1063,12 @@ void AppHost::_SummonWindowRequested(const winrt::Windows::Foundation::IInspecta
     _HandleSummon(sender, summonArgs);
 }
 
+void AppHost::_OpenSystemMenu(const winrt::Windows::Foundation::IInspectable&,
+                              const winrt::Windows::Foundation::IInspectable&)
+{
+    _window->OpenSystemMenu(std::nullopt, std::nullopt);
+}
+
 // Method Description:
 // - Creates a Tray Icon and hooks up its handlers
 // Arguments:

src/cascadia/WindowsTerminal/AppHost.h

@@ -85,6 +85,9 @@ class AppHost
     void _SummonWindowRequested(const winrt::Windows::Foundation::IInspectable& sender,
                                 const winrt::Windows::Foundation::IInspectable& args);
 
+    void _OpenSystemMenu(const winrt::Windows::Foundation::IInspectable& sender,
+                         const winrt::Windows::Foundation::IInspectable& args);
+
     winrt::fire_and_forget _QuitRequested(const winrt::Windows::Foundation::IInspectable& sender,
                                           const winrt::Windows::Foundation::IInspectable& args);
 

src/cascadia/WindowsTerminal/IslandWindow.cpp

@@ -1645,5 +1645,63 @@ void IslandWindow::SetMinimizeToTrayBehavior(bool minimizeToTray) noexcept
     _minimizeToTray = minimizeToTray;
 }
 
+// Method Description:
+// - Opens the window's system menu.
+// - The system menu is the menu that opens when the user presses Alt+Space or
+//   right clicks on the title bar.
+// - Before updating the menu, we update the buttons like "Maximize" and
+//   "Restore" so that they are grayed out depending on the window's state.
+// Arguments:
+// - cursorX: the cursor's X position in screen coordinates
+// - cursorY: the cursor's Y position in screen coordinates
+void IslandWindow::OpenSystemMenu(const std::optional<int> mouseX, const std::optional<int> mouseY) const noexcept
+{
+    const auto systemMenu = GetSystemMenu(_window.get(), FALSE);
+
+    WINDOWPLACEMENT placement;
+    if (!GetWindowPlacement(_window.get(), &placement))
+    {
+        return;
+    }
+    const bool isMaximized = placement.showCmd == SW_SHOWMAXIMIZED;
+
+    // Update the options based on window state.
+    MENUITEMINFO mii;
+    mii.cbSize = sizeof(MENUITEMINFO);
+    mii.fMask = MIIM_STATE;
+    mii.fType = MFT_STRING;
+    auto setState = [&](UINT item, bool enabled) {
+        mii.fState = enabled ? MF_ENABLED : MF_DISABLED;
+        SetMenuItemInfo(systemMenu, item, FALSE, &mii);
+    };
+    setState(SC_RESTORE, isMaximized);
+    setState(SC_MOVE, !isMaximized);
+    setState(SC_SIZE, !isMaximized);
+    setState(SC_MINIMIZE, true);
+    setState(SC_MAXIMIZE, !isMaximized);
+    setState(SC_CLOSE, true);
+    SetMenuDefaultItem(systemMenu, UINT_MAX, FALSE);
+
+    int xPos;
+    int yPos;
+    if (mouseX && mouseY)
+    {
+        xPos = mouseX.value();
+        yPos = mouseY.value();
+    }
+    else
+    {
+        RECT windowPos;
+        ::GetWindowRect(GetHandle(), &windowPos);
+        xPos = windowPos.left;
+        yPos = windowPos.top;
+    }
+    const auto ret = TrackPopupMenu(systemMenu, TPM_RETURNCMD, xPos, yPos, 0, _window.get(), nullptr);
+    if (ret != 0)
+    {
+        PostMessage(_window.get(), WM_SYSCOMMAND, ret, 0);
+    }
+}
+
 DEFINE_EVENT(IslandWindow, DragRegionClicked, _DragRegionClickedHandlers, winrt::delegate<>);
 DEFINE_EVENT(IslandWindow, WindowCloseButtonClicked, _windowCloseButtonClickedHandler, winrt::delegate<>);

src/cascadia/WindowsTerminal/IslandWindow.h

@@ -51,6 +51,8 @@ class IslandWindow :
 
     void SetMinimizeToTrayBehavior(bool minimizeToTray) noexcept;
 
+    void OpenSystemMenu(const std::optional<int> mouseX, const std::optional<int> mouseY) const noexcept;
+
     DECLARE_EVENT(DragRegionClicked, _DragRegionClickedHandlers, winrt::delegate<>);
     DECLARE_EVENT(WindowCloseButtonClicked, _windowCloseButtonClickedHandler, winrt::delegate<>);
     WINRT_CALLBACK(MouseScrolled, winrt::delegate<void(til::point, int32_t)>);

src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp

@@ -750,7 +750,7 @@ void NonClientIslandWindow::_UpdateFrameMargins() const noexcept
         // reason so we have to do it ourselves.
         if (wParam == HTCAPTION)
         {
-            _OpenSystemMenu(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam));
+            OpenSystemMenu(GET_X_LPARAM(lParam), GET_Y_LPARAM(lParam));
         }
         break;
     }
@@ -947,47 +947,3 @@ bool NonClientIslandWindow::_IsTitlebarVisible() const
 {
     return !(_fullscreen || _borderless);
 }
-
-// Method Description:
-// - Opens the window's system menu.
-// - The system menu is the menu that opens when the user presses Alt+Space or
-//   right clicks on the title bar.
-// - Before updating the menu, we update the buttons like "Maximize" and
-//   "Restore" so that they are grayed out depending on the window's state.
-// Arguments:
-// - cursorX: the cursor's X position in screen coordinates
-// - cursorY: the cursor's Y position in screen coordinates
-void NonClientIslandWindow::_OpenSystemMenu(const int cursorX, const int cursorY) const noexcept
-{
-    const auto systemMenu = GetSystemMenu(_window.get(), FALSE);
-
-    WINDOWPLACEMENT placement;
-    if (!GetWindowPlacement(_window.get(), &placement))
-    {
-        return;
-    }
-    const bool isMaximized = placement.showCmd == SW_SHOWMAXIMIZED;
-
-    // Update the options based on window state.
-    MENUITEMINFO mii;
-    mii.cbSize = sizeof(MENUITEMINFO);
-    mii.fMask = MIIM_STATE;
-    mii.fType = MFT_STRING;
-    auto setState = [&](UINT item, bool enabled) {
-        mii.fState = enabled ? MF_ENABLED : MF_DISABLED;
-        SetMenuItemInfo(systemMenu, item, FALSE, &mii);
-    };
-    setState(SC_RESTORE, isMaximized);
-    setState(SC_MOVE, !isMaximized);
-    setState(SC_SIZE, !isMaximized);
-    setState(SC_MINIMIZE, true);
-    setState(SC_MAXIMIZE, !isMaximized);
-    setState(SC_CLOSE, true);
-    SetMenuDefaultItem(systemMenu, UINT_MAX, FALSE);
-
-    const auto ret = TrackPopupMenu(systemMenu, TPM_RETURNCMD, cursorX, cursorY, 0, _window.get(), nullptr);
-    if (ret != 0)
-    {
-        PostMessage(_window.get(), WM_SYSCOMMAND, ret, 0);
-    }
-}

src/cascadia/WindowsTerminal/NonClientIslandWindow.h

@@ -87,6 +87,4 @@ class NonClientIslandWindow : public IslandWindow
     void _UpdateFrameMargins() const noexcept;
     void _UpdateMaximizedState();
     void _UpdateIslandPosition(const UINT windowWidth, const UINT windowHeight);
-
-    void _OpenSystemMenu(const int mouseX, const int mouseY) const noexcept;
 };

src/cascadia/WindowsTerminal/main.cpp

@@ -184,16 +184,13 @@ int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int)
             }
         }
 
-        // GH#7125 = System XAML will show a system dialog on Alt Space. We might want to
-        // explicitly prevent that - for example when an action is bound to it. So similar to
-        // above, we steal the event and hand it off to the host. When the host does not process
-        // it, we will still dispatch like normal.
+        // GH#7125 = System XAML will show a system dialog on Alt Space. We want to
+        // explicitly prevent that because we handle that ourselves. So similar to
+        // above, we steal the event and hand it off to the host.
         if (_messageIsAltSpaceKeypress(message))
         {
-            if (host.OnDirectKeyEvent(VK_SPACE, LOBYTE(HIWORD(message.lParam)), true))
-            {
-                continue;
-            }
+            host.OnDirectKeyEvent(VK_SPACE, LOBYTE(HIWORD(message.lParam)), true);
+            continue;
         }
 
         TranslateMessage(&message);