Merge branch 'dev/migrie/oop/3/akallabeth' into dev/migrie/oop/3/an-u…

…nexpected-party

doc/cascadia/profiles.schema.json

@@ -1822,7 +1822,7 @@
         "name": {
           "type": "string",
           "description": "The name of the theme. This will be displayed in the settings UI.",
-          "not": { 
+          "not": {
             "enum": [ "light", "dark", "system" ]
           }
         },
@@ -2092,6 +2092,11 @@
           "description": "When set to true, the terminal will focus the pane on mouse hover.",
           "type": "boolean"
         },
+        "compatibility.isolatedMode": {
+          "default": false,
+          "description": "When set to true, Terminal windows will not be able to interact with each other (including global hotkeys, tab drag/drop, running commandlines in existing windows, etc.). This is a compatibility escape hatch for users who are running into certain windowing issues.",
+          "type": "boolean"
+        },
         "copyFormatting": {
           "default": true,
           "description": "When set to `true`, the color and font formatting of selected text is also copied to your clipboard. When set to `false`, only plain text is copied to your clipboard. An array of specific formats can also be used. Supported array values include `html` and `rtf`. Plain text is always copied.",

src/cascadia/Remoting/Monarch.cpp

@@ -688,6 +688,11 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
             case WindowingBehaviorUseName:
                 windowID = _lookupPeasantIdForName(targetWindowName);
                 break;
+            case WindowingBehaviorUseNone:
+                // This should be impossible. The if statement above should have
+                // prevented WindowingBehaviorUseNone from falling in here.
+                // Explode, because this is a programming error.
+                THROW_HR(E_UNEXPECTED);
             default:
                 windowID = ::base::saturated_cast<uint64_t>(targetWindow);
                 break;

src/cascadia/Remoting/WindowManager.cpp

@@ -106,18 +106,15 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
             // in isolated mode.
 
             shouldCreateWindow = false;
-            std::optional<uint64_t> givenID;
-            winrt::hstring givenName{};
-
             // Send the commandline over to the monarch process
-            if (_proposeToMonarch(args, givenID, givenName))
+            if (_proposeToMonarch(args))
             {
                 // If that succeeded, then we don't need to make a new window.
                 // Our job is done. Either the monarch is going to run the
                 // commandline in an existing window, or a new one, but either way,
                 // this process doesn't need to make a new window.
 
-                return *winrt::make_self<ProposeCommandlineResult>(shouldCreateWindow);
+                return winrt::make<ProposeCommandlineResult>(shouldCreateWindow);
             }
             // Otherwise, we'll try to handle this ourselves.
         }
@@ -153,7 +150,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
                 // either.
                 shouldCreateWindow = false;
 
-                return *winrt::make_self<ProposeCommandlineResult>(shouldCreateWindow);
+                return winrt::make<ProposeCommandlineResult>(shouldCreateWindow);
             }
             else
             {
@@ -214,9 +211,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
     // - Helper attempting to call to the monarch multiple times. If the monarch
     //   fails to respond, or we encounter any sort of error, we'll try again
     //   until we find one, or decisively determine there isn't one.
-    bool WindowManager::_proposeToMonarch(const Remoting::CommandlineArgs& args,
-                                          std::optional<uint64_t>& /*givenID*/,
-                                          winrt::hstring& /*givenName*/)
+    bool WindowManager::_proposeToMonarch(const Remoting::CommandlineArgs& args)
     {
         // these two errors are Win32 errors, convert them to HRESULTS so we can actually compare below.
         static constexpr auto RPC_SERVER_UNAVAILABLE_HR = HRESULT_FROM_WIN32(RPC_S_SERVER_UNAVAILABLE);

src/cascadia/Remoting/WindowManager.h

@@ -66,9 +66,7 @@ namespace winrt::Microsoft::Terminal::Remoting::implementation
         void _createMonarch();
         void _registerAsMonarch();
 
-        bool _proposeToMonarch(const Remoting::CommandlineArgs& args,
-                               std::optional<uint64_t>& givenID,
-                               winrt::hstring& givenName);
+        bool _proposeToMonarch(const Remoting::CommandlineArgs& args);
 
         void _createCallbacks();
         void _raiseFindTargetWindowRequested(const winrt::Windows::Foundation::IInspectable& sender,

src/cascadia/TerminalApp/AppLogic.cpp

@@ -45,6 +45,17 @@ static const std::array settingsLoadErrorsLabels{
 };
 static_assert(settingsLoadErrorsLabels.size() == static_cast<size_t>(SettingsLoadErrors::ERRORS_SIZE));
 
+// Function Description:
+// - General-purpose helper for looking up a localized string for a
+//   warning/error. First will look for the given key in the provided map of
+//   keys->strings, where the values in the map are ResourceKeys. If it finds
+//   one, it will lookup the localized string from that ResourceKey.
+// - If it does not find a key, it'll return an empty string
+// Arguments:
+// - key: the value to use to look for a resource key in the given map
+// - map: A map of keys->Resource keys.
+// Return Value:
+// - the localized string for the given type, if it exists.
 template<typename T>
 winrt::hstring _GetMessageText(uint32_t index, const T& keys)
 {
@@ -66,7 +77,6 @@ static winrt::hstring _GetErrorText(SettingsLoadErrors error)
 {
     return _GetMessageText(static_cast<uint32_t>(error), settingsLoadErrorsLabels);
 }
-////////////////////////////////////////////////////////////////////////////////
 
 static constexpr std::wstring_view StartupTaskName = L"StartTerminalOnLoginTask";
 
@@ -118,9 +128,6 @@ namespace winrt::TerminalApp::implementation
         // cause you to chase down the rabbit hole of "why is App not
         // registered?" when it definitely is.
 
-        // The TerminalPage has to be constructed during our construction, to
-        // make sure that there's a terminal page for callers of
-        // SetTitleBarContent
         _isElevated = ::Microsoft::Console::Utils::IsElevated();
 
         _reloadSettings = std::make_shared<ThrottledFuncTrailing<>>(winrt::Windows::System::DispatcherQueue::GetForCurrentThread(), std::chrono::milliseconds(100), [weakSelf = get_weak()]() {

src/cascadia/TerminalApp/Pane.cpp

@@ -80,7 +80,7 @@ Pane::Pane(std::shared_ptr<Pane> first,
 
     // Use the unfocused border color as the pane background, so an actual color
     // appears behind panes as we animate them sliding in.
-    _root.Background(_unfocusedBorderBrush);
+    _root.Background(_themeResources.unfocusedBorderBrush);
 
     _root.Children().Append(_borderFirst);
     _root.Children().Append(_borderSecond);
@@ -1437,8 +1437,8 @@ void Pane::UpdateVisuals()
     {
         _UpdateBorders();
     }
-    _borderFirst.BorderBrush(_lastActive ? _focusedBorderBrush : _unfocusedBorderBrush);
-    _borderSecond.BorderBrush(_lastActive ? _focusedBorderBrush : _unfocusedBorderBrush);
+    _borderFirst.BorderBrush(_lastActive ? _themeResources.focusedBorderBrush : _themeResources.unfocusedBorderBrush);
+    _borderSecond.BorderBrush(_lastActive ? _themeResources.focusedBorderBrush : _themeResources.unfocusedBorderBrush);
 }
 
 // Method Description:
@@ -1890,7 +1890,7 @@ winrt::fire_and_forget Pane::_CloseChildRoutine(const bool closeFirst)
         Controls::Grid dummyGrid;
         // GH#603 - we can safely add a BG here, as the control is gone right
         // away, to fill the space as the rest of the pane expands.
-        dummyGrid.Background(_unfocusedBorderBrush);
+        dummyGrid.Background(_themeResources.unfocusedBorderBrush);
         // It should be the size of the closed pane.
         dummyGrid.Width(removedOriginalSize.Width);
         dummyGrid.Height(removedOriginalSize.Height);
@@ -2168,7 +2168,7 @@ void Pane::_SetupEntranceAnimation()
     // * If we give the parent (us) root BG a color, then a transparent pane
     //   will flash opaque during the animation, then back to transparent, which
     //   looks bad.
-    _secondChild->_root.Background(_unfocusedBorderBrush);
+    _secondChild->_root.Background(_themeResources.unfocusedBorderBrush);
 
     const auto [firstSize, secondSize] = _CalcChildrenSizes(::base::saturated_cast<float>(totalSize));
 
@@ -3133,51 +3133,20 @@ float Pane::_ClampSplitPosition(const bool widthOrHeight, const float requestedV
     return std::clamp(requestedValue, minSplitPosition, maxSplitPosition);
 }
 
-// Function Description:
-// - Attempts to load some XAML resources that the Pane will need. This includes:
-//   * The Color we'll use for active Panes's borders - SystemAccentColor
-//   * The Brush we'll use for inactive Panes - TabViewBackground (to match the
-//     color of the titlebar)
-// Arguments:
-// - requestedTheme: this should be the currently active Theme for the app
-// Return Value:
-// - <none>
-void Pane::SetupResources(const winrt::Windows::UI::Xaml::ElementTheme& requestedTheme)
+// Method Description:
+// - Update our stored brushes for the current theme. This will also recursively
+//   update all our children.
+// - TerminalPage creates these brushes and ultimately owns them. Effectively,
+//   we're just storing a smart pointer to the page's brushes.
+void Pane::UpdateResources(const PaneResources& resources)
 {
-    const auto res = Application::Current().Resources();
-    const auto accentColorKey = winrt::box_value(L"SystemAccentColor");
-    if (res.HasKey(accentColorKey))
-    {
-        const auto colorFromResources = ThemeLookup(res, requestedTheme, accentColorKey);
-        // If SystemAccentColor is _not_ a Color for some reason, use
-        // Transparent as the color, so we don't do this process again on
-        // the next pane (by leaving s_focusedBorderBrush nullptr)
-        auto actualColor = winrt::unbox_value_or<Color>(colorFromResources, Colors::Black());
-        _focusedBorderBrush = SolidColorBrush(actualColor);
-    }
-    else
-    {
-        // DON'T use Transparent here - if it's "Transparent", then it won't
-        // be able to hittest for clicks, and then clicking on the border
-        // will eat focus.
-        _focusedBorderBrush = SolidColorBrush{ Colors::Black() };
-    }
+    _themeResources = resources;
+    UpdateVisuals();
 
-    const auto unfocusedBorderBrushKey = winrt::box_value(L"UnfocusedBorderBrush");
-    if (res.HasKey(unfocusedBorderBrushKey))
-    {
-        // MAKE SURE TO USE ThemeLookup, so that we get the correct resource for
-        // the requestedTheme, not just the value from the resources (which
-        // might not respect the settings' requested theme)
-        auto obj = ThemeLookup(res, requestedTheme, unfocusedBorderBrushKey);
-        _unfocusedBorderBrush = obj.try_as<winrt::Windows::UI::Xaml::Media::SolidColorBrush>();
-    }
-    else
+    if (!_IsLeaf())
     {
-        // DON'T use Transparent here - if it's "Transparent", then it won't
-        // be able to hittest for clicks, and then clicking on the border
-        // will eat focus.
-        _unfocusedBorderBrush = SolidColorBrush{ Colors::Black() };
+        _firstChild->UpdateResources(resources);
+        _secondChild->UpdateResources(resources);
     }
 }
 

src/cascadia/TerminalApp/Pane.h

@@ -51,6 +51,12 @@ enum class SplitState : int
     Vertical = 2
 };
 
+struct PaneResources
+{
+    winrt::Windows::UI::Xaml::Media::SolidColorBrush focusedBorderBrush{ nullptr };
+    winrt::Windows::UI::Xaml::Media::SolidColorBrush unfocusedBorderBrush{ nullptr };
+};
+
 class Pane : public std::enable_shared_from_this<Pane>
 {
 public:
@@ -136,7 +142,7 @@ class Pane : public std::enable_shared_from_this<Pane>
 
     bool ContainsReadOnly() const;
 
-    void SetupResources(const winrt::Windows::UI::Xaml::ElementTheme& requestedTheme);
+    void UpdateResources(const PaneResources& resources);
 
     // Method Description:
     // - A helper method for ad-hoc recursion on a pane tree. Walks the pane
@@ -217,8 +223,8 @@ class Pane : public std::enable_shared_from_this<Pane>
     winrt::Windows::UI::Xaml::Controls::Grid _root{};
     winrt::Windows::UI::Xaml::Controls::Border _borderFirst{};
     winrt::Windows::UI::Xaml::Controls::Border _borderSecond{};
-    winrt::Windows::UI::Xaml::Media::SolidColorBrush _focusedBorderBrush;
-    winrt::Windows::UI::Xaml::Media::SolidColorBrush _unfocusedBorderBrush;
+
+    PaneResources _themeResources;
 
 #pragma region Properties that need to be transferred between child / parent panes upon splitting / closing
     std::shared_ptr<Pane> _firstChild{ nullptr };

src/cascadia/TerminalApp/TerminalAppLib.vcxproj

@@ -144,6 +144,9 @@
     <ClInclude Include="TerminalWindow.h">
       <DependentUpon>TerminalWindow.idl</DependentUpon>
     </ClInclude>
+    <ClInclude Include="SettingsLoadEventArgs.h">
+      <DependentUpon>TerminalWindow.idl</DependentUpon>
+    </ClInclude>
     <ClInclude Include="Toast.h" />
   </ItemGroup>
   <!-- ========================= Cpp Files ======================== -->