From cdc85cf96466f5a972d98e548d9af6892124a581 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 24 May 2023 09:41:11 -0500 Subject: [PATCH 1/3] This should fix #15412, though I should make sure I can repro it in the first place --- src/cascadia/TerminalApp/TerminalTab.cpp | 111 +++++++++++++---------- src/cascadia/TerminalApp/TerminalTab.h | 6 ++ 2 files changed, 71 insertions(+), 46 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 28f9152794b..6d4c16271ba 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -911,60 +911,79 @@ namespace winrt::TerminalApp::implementation auto dispatcher = TabViewItem().Dispatcher(); ControlEventTokens events{}; - events.titleToken = control.TitleChanged([dispatcher, weakThis](auto&&, auto&&) -> winrt::fire_and_forget { - co_await wil::resume_foreground(dispatcher); - // Check if Tab's lifetime has expired - if (auto tab{ weakThis.get() }) - { - // The title of the control changed, but not necessarily the title of the tab. - // Set the tab's text to the active panes' text. - tab->UpdateTitle(); - } - }); + events.titleToken = control.TitleChanged({ weakThis, &TerminalTab::_controlTitleChanged }); - events.colorToken = control.TabColorChanged([dispatcher, weakThis](auto&&, auto&&) -> winrt::fire_and_forget { - co_await wil::resume_foreground(dispatcher); - if (auto tab{ weakThis.get() }) - { - // The control's tabColor changed, but it is not necessarily the - // active control in this tab. We'll just recalculate the - // current color anyways. - tab->_RecalculateAndApplyTabColor(); - } - }); + events.colorToken = control.TabColorChanged({ weakThis, &TerminalTab::_controlTabColorChanged }); - events.taskbarToken = control.SetTaskbarProgress([dispatcher, weakThis](auto&&, auto&&) -> winrt::fire_and_forget { - co_await wil::resume_foreground(dispatcher); - // Check if Tab's lifetime has expired - if (auto tab{ weakThis.get() }) - { - tab->_UpdateProgressState(); - } - }); + events.taskbarToken = control.SetTaskbarProgress({ weakThis, &TerminalTab::_controlSetTaskbarProgress }); - events.readOnlyToken = control.ReadOnlyChanged([dispatcher, weakThis](auto&&, auto&&) -> winrt::fire_and_forget { - co_await wil::resume_foreground(dispatcher); - if (auto tab{ weakThis.get() }) - { - tab->_RecalculateAndApplyReadOnly(); - } - }); + events.readOnlyToken = control.ReadOnlyChanged({ weakThis, &TerminalTab::_controlReadOnlyChanged }); - events.focusToken = control.FocusFollowMouseRequested([dispatcher, weakThis](auto&& sender, auto&&) -> winrt::fire_and_forget { - co_await wil::resume_foreground(dispatcher); - if (const auto tab{ weakThis.get() }) + events.focusToken = control.FocusFollowMouseRequested({ weakThis, &TerminalTab::_controlFocusFollowMouseRequested }); + + _controlEvents[paneId] = events; + } + + winrt::fire_and_forget TerminalTab::_controlTitleChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e) + { + auto weakThis{ get_weak() }; + auto dispatcher = TabViewItem().Dispatcher(); + co_await wil::resume_foreground(dispatcher); + if (auto tab{ weakThis.get() }) + { + // The title of the control changed, but not necessarily the title of the tab. + // Set the tab's text to the active panes' text. + tab->UpdateTitle(); + } + } + winrt::fire_and_forget TerminalTab::_controlTabColorChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e) + { + auto weakThis{ get_weak() }; + auto dispatcher = TabViewItem().Dispatcher(); + co_await wil::resume_foreground(dispatcher); + if (auto tab{ weakThis.get() }) + { + // The control's tabColor changed, but it is not necessarily the + // active control in this tab. We'll just recalculate the + // current color anyways. + tab->_RecalculateAndApplyTabColor(); + } + } + winrt::fire_and_forget TerminalTab::_controlSetTaskbarProgress(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e) + { + auto weakThis{ get_weak() }; + auto dispatcher = TabViewItem().Dispatcher(); + co_await wil::resume_foreground(dispatcher); + if (auto tab{ weakThis.get() }) + { + tab->_UpdateProgressState(); + } + } + winrt::fire_and_forget TerminalTab::_controlReadOnlyChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e) + { + auto weakThis{ get_weak() }; + auto dispatcher = TabViewItem().Dispatcher(); + co_await wil::resume_foreground(dispatcher); + if (auto tab{ weakThis.get() }) + { + tab->_RecalculateAndApplyReadOnly(); + } + } + winrt::fire_and_forget TerminalTab::_controlFocusFollowMouseRequested(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e) + { + auto weakThis{ get_weak() }; + auto dispatcher = TabViewItem().Dispatcher(); + co_await wil::resume_foreground(dispatcher); + if (auto tab{ weakThis.get() }) + { + if (tab->_focusState != FocusState::Unfocused) { - if (tab->_focusState != FocusState::Unfocused) + if (const auto termControl{ sender.try_as() }) { - if (const auto termControl{ sender.try_as() }) - { - termControl.Focus(FocusState::Pointer); - } + termControl.Focus(FocusState::Pointer); } } - }); - - _controlEvents[paneId] = events; + } } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index a4cc9aa8eca..87b0857f3df 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -166,6 +166,12 @@ namespace winrt::TerminalApp::implementation void _AttachEventHandlersToControl(const uint32_t paneId, const winrt::Microsoft::Terminal::Control::TermControl& control); void _AttachEventHandlersToPane(std::shared_ptr pane); + winrt::fire_and_forget _controlTitleChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); + winrt::fire_and_forget _controlTabColorChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); + winrt::fire_and_forget _controlSetTaskbarProgress(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); + winrt::fire_and_forget _controlReadOnlyChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); + winrt::fire_and_forget _controlFocusFollowMouseRequested(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); + void _UpdateActivePane(std::shared_ptr pane); winrt::hstring _GetActiveTitle() const; From d9b5e157fd9d3350d255d029f44260b3709219e3 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 24 May 2023 10:12:31 -0500 Subject: [PATCH 2/3] I guess building the code is heplful --- src/cascadia/TerminalApp/TerminalTab.cpp | 18 ++++++------------ src/cascadia/TerminalApp/TerminalTab.h | 2 +- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 6d4c16271ba..97e4a331cc5 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -907,24 +907,18 @@ namespace winrt::TerminalApp::implementation // - void TerminalTab::_AttachEventHandlersToControl(const uint32_t paneId, const TermControl& control) { - auto weakThis{ get_weak() }; - auto dispatcher = TabViewItem().Dispatcher(); ControlEventTokens events{}; - events.titleToken = control.TitleChanged({ weakThis, &TerminalTab::_controlTitleChanged }); - - events.colorToken = control.TabColorChanged({ weakThis, &TerminalTab::_controlTabColorChanged }); - - events.taskbarToken = control.SetTaskbarProgress({ weakThis, &TerminalTab::_controlSetTaskbarProgress }); - - events.readOnlyToken = control.ReadOnlyChanged({ weakThis, &TerminalTab::_controlReadOnlyChanged }); - - events.focusToken = control.FocusFollowMouseRequested({ weakThis, &TerminalTab::_controlFocusFollowMouseRequested }); + events.titleToken = control.TitleChanged({ get_weak(), &TerminalTab::_controlTitleChanged }); + events.colorToken = control.TabColorChanged({ get_weak(), &TerminalTab::_controlTabColorChanged }); + events.taskbarToken = control.SetTaskbarProgress({ get_weak(), &TerminalTab::_controlSetTaskbarProgress }); + events.readOnlyToken = control.ReadOnlyChanged({ get_weak(), &TerminalTab::_controlReadOnlyChanged }); + events.focusToken = control.FocusFollowMouseRequested({ get_weak(), &TerminalTab::_controlFocusFollowMouseRequested }); _controlEvents[paneId] = events; } - winrt::fire_and_forget TerminalTab::_controlTitleChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e) + winrt::fire_and_forget TerminalTab::_controlTitleChanged(Windows::Foundation::IInspectable sender, Microsoft::Terminal::Control::TitleChangedEventArgs e) { auto weakThis{ get_weak() }; auto dispatcher = TabViewItem().Dispatcher(); diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index 87b0857f3df..f7efa1b27f4 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -166,7 +166,7 @@ namespace winrt::TerminalApp::implementation void _AttachEventHandlersToControl(const uint32_t paneId, const winrt::Microsoft::Terminal::Control::TermControl& control); void _AttachEventHandlersToPane(std::shared_ptr pane); - winrt::fire_and_forget _controlTitleChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); + winrt::fire_and_forget _controlTitleChanged(Windows::Foundation::IInspectable sender, Microsoft::Terminal::Control::TitleChangedEventArgs e); winrt::fire_and_forget _controlTabColorChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); winrt::fire_and_forget _controlSetTaskbarProgress(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); winrt::fire_and_forget _controlReadOnlyChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); From e19d7404c02ce174734758d599f76a9b435a4cb2 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 25 May 2023 13:16:13 -0500 Subject: [PATCH 3/3] as minimal as possible --- src/cascadia/TerminalApp/TerminalTab.cpp | 113 ++++++++++------------- src/cascadia/TerminalApp/TerminalTab.h | 6 -- 2 files changed, 50 insertions(+), 69 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 97e4a331cc5..5a5bfb65f03 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -907,77 +907,64 @@ namespace winrt::TerminalApp::implementation // - void TerminalTab::_AttachEventHandlersToControl(const uint32_t paneId, const TermControl& control) { + auto weakThis{ get_weak() }; + auto dispatcher = TabViewItem().Dispatcher(); ControlEventTokens events{}; - events.titleToken = control.TitleChanged({ get_weak(), &TerminalTab::_controlTitleChanged }); - events.colorToken = control.TabColorChanged({ get_weak(), &TerminalTab::_controlTabColorChanged }); - events.taskbarToken = control.SetTaskbarProgress({ get_weak(), &TerminalTab::_controlSetTaskbarProgress }); - events.readOnlyToken = control.ReadOnlyChanged({ get_weak(), &TerminalTab::_controlReadOnlyChanged }); - events.focusToken = control.FocusFollowMouseRequested({ get_weak(), &TerminalTab::_controlFocusFollowMouseRequested }); + events.titleToken = control.TitleChanged([dispatcher, weakThis](auto&&, auto&&) -> winrt::fire_and_forget { + co_await wil::resume_foreground(dispatcher); + // Check if Tab's lifetime has expired + if (auto tab{ weakThis.get() }) + { + // The title of the control changed, but not necessarily the title of the tab. + // Set the tab's text to the active panes' text. + tab->UpdateTitle(); + } + }); - _controlEvents[paneId] = events; - } + events.colorToken = control.TabColorChanged([dispatcher, weakThis](auto&&, auto&&) -> winrt::fire_and_forget { + co_await wil::resume_foreground(dispatcher); + if (auto tab{ weakThis.get() }) + { + // The control's tabColor changed, but it is not necessarily the + // active control in this tab. We'll just recalculate the + // current color anyways. + tab->_RecalculateAndApplyTabColor(); + } + }); - winrt::fire_and_forget TerminalTab::_controlTitleChanged(Windows::Foundation::IInspectable sender, Microsoft::Terminal::Control::TitleChangedEventArgs e) - { - auto weakThis{ get_weak() }; - auto dispatcher = TabViewItem().Dispatcher(); - co_await wil::resume_foreground(dispatcher); - if (auto tab{ weakThis.get() }) - { - // The title of the control changed, but not necessarily the title of the tab. - // Set the tab's text to the active panes' text. - tab->UpdateTitle(); - } - } - winrt::fire_and_forget TerminalTab::_controlTabColorChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e) - { - auto weakThis{ get_weak() }; - auto dispatcher = TabViewItem().Dispatcher(); - co_await wil::resume_foreground(dispatcher); - if (auto tab{ weakThis.get() }) - { - // The control's tabColor changed, but it is not necessarily the - // active control in this tab. We'll just recalculate the - // current color anyways. - tab->_RecalculateAndApplyTabColor(); - } - } - winrt::fire_and_forget TerminalTab::_controlSetTaskbarProgress(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e) - { - auto weakThis{ get_weak() }; - auto dispatcher = TabViewItem().Dispatcher(); - co_await wil::resume_foreground(dispatcher); - if (auto tab{ weakThis.get() }) - { - tab->_UpdateProgressState(); - } - } - winrt::fire_and_forget TerminalTab::_controlReadOnlyChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e) - { - auto weakThis{ get_weak() }; - auto dispatcher = TabViewItem().Dispatcher(); - co_await wil::resume_foreground(dispatcher); - if (auto tab{ weakThis.get() }) - { - tab->_RecalculateAndApplyReadOnly(); - } - } - winrt::fire_and_forget TerminalTab::_controlFocusFollowMouseRequested(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e) - { - auto weakThis{ get_weak() }; - auto dispatcher = TabViewItem().Dispatcher(); - co_await wil::resume_foreground(dispatcher); - if (auto tab{ weakThis.get() }) - { - if (tab->_focusState != FocusState::Unfocused) + events.taskbarToken = control.SetTaskbarProgress([dispatcher, weakThis](auto&&, auto&&) -> winrt::fire_and_forget { + co_await wil::resume_foreground(dispatcher); + // Check if Tab's lifetime has expired + if (auto tab{ weakThis.get() }) + { + tab->_UpdateProgressState(); + } + }); + + events.readOnlyToken = control.ReadOnlyChanged([dispatcher, weakThis](auto&&, auto&&) -> winrt::fire_and_forget { + co_await wil::resume_foreground(dispatcher); + if (auto tab{ weakThis.get() }) + { + tab->_RecalculateAndApplyReadOnly(); + } + }); + + events.focusToken = control.FocusFollowMouseRequested([dispatcher, weakThis](auto sender, auto) -> winrt::fire_and_forget { + co_await wil::resume_foreground(dispatcher); + if (const auto tab{ weakThis.get() }) { - if (const auto termControl{ sender.try_as() }) + if (tab->_focusState != FocusState::Unfocused) { - termControl.Focus(FocusState::Pointer); + if (const auto termControl{ sender.try_as() }) + { + termControl.Focus(FocusState::Pointer); + } } } - } + }); + + _controlEvents[paneId] = events; } // Method Description: diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index f7efa1b27f4..a4cc9aa8eca 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -166,12 +166,6 @@ namespace winrt::TerminalApp::implementation void _AttachEventHandlersToControl(const uint32_t paneId, const winrt::Microsoft::Terminal::Control::TermControl& control); void _AttachEventHandlersToPane(std::shared_ptr pane); - winrt::fire_and_forget _controlTitleChanged(Windows::Foundation::IInspectable sender, Microsoft::Terminal::Control::TitleChangedEventArgs e); - winrt::fire_and_forget _controlTabColorChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); - winrt::fire_and_forget _controlSetTaskbarProgress(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); - winrt::fire_and_forget _controlReadOnlyChanged(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); - winrt::fire_and_forget _controlFocusFollowMouseRequested(Windows::Foundation::IInspectable sender, Windows::Foundation::IInspectable e); - void _UpdateActivePane(std::shared_ptr pane); winrt::hstring _GetActiveTitle() const;