diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt index ede4229f16..932611291a 100644 --- a/.github/actions/spelling/expect/expect.txt +++ b/.github/actions/spelling/expect/expect.txt @@ -1091,6 +1091,8 @@ NEXTLINE nfe NLSMODE NOACTIVATE +NOACTIVATEKEYBOARDLAYOUT +NOACTIVATETIP NOAPPLYNOW NOCLIP NOCOMM @@ -1504,7 +1506,6 @@ scrolllock scrolloffset SCROLLSCALE SCROLLSCREENBUFFER -scursor sddl SDKDDK segfault @@ -1703,9 +1704,11 @@ TEXTMETRIC TEXTMETRICW textmode texttests +TFCAT THUMBPOSITION THUMBTRACK tilunittests +TIPCAP titlebars TITLEISLINKNAME TLDP @@ -1757,6 +1760,8 @@ UIACCESS uiacore uiautomationcore uielem +UIELEMENTENABLED +UIELEMENTENABLEDONLY UINTs uld uldash diff --git a/src/cascadia/TerminalControl/HwndTerminal.cpp b/src/cascadia/TerminalControl/HwndTerminal.cpp index 58c9f34e4e..f4cfb967ae 100644 --- a/src/cascadia/TerminalControl/HwndTerminal.cpp +++ b/src/cascadia/TerminalControl/HwndTerminal.cpp @@ -462,6 +462,11 @@ void HwndTerminal::SendOutput(std::wstring_view data) _terminal->Write(data); } +void _stdcall AvoidBuggyTSFConsoleFlags() +{ + Microsoft::Console::TSF::Handle::AvoidBuggyTSFConsoleFlags(); +} + HRESULT _stdcall CreateTerminal(HWND parentHwnd, _Out_ void** hwnd, _Out_ void** terminal) { auto publicTerminal = std::make_unique(parentHwnd); diff --git a/src/cascadia/TerminalControl/HwndTerminal.hpp b/src/cascadia/TerminalControl/HwndTerminal.hpp index 4d41741952..68ffc73ec7 100644 --- a/src/cascadia/TerminalControl/HwndTerminal.hpp +++ b/src/cascadia/TerminalControl/HwndTerminal.hpp @@ -41,6 +41,7 @@ typedef struct _TerminalTheme } TerminalTheme, *LPTerminalTheme; extern "C" { +__declspec(dllexport) void _stdcall AvoidBuggyTSFConsoleFlags(); __declspec(dllexport) HRESULT _stdcall CreateTerminal(HWND parentHwnd, _Out_ void** hwnd, _Out_ void** terminal); __declspec(dllexport) void _stdcall TerminalSendOutput(void* terminal, LPCWSTR data); __declspec(dllexport) void _stdcall TerminalRegisterScrollCallback(void* terminal, void __stdcall callback(int, int, int)); diff --git a/src/cascadia/TerminalControl/dll/Microsoft.Terminal.Control.def b/src/cascadia/TerminalControl/dll/Microsoft.Terminal.Control.def index c02c0721ab..60ce76f1a9 100644 --- a/src/cascadia/TerminalControl/dll/Microsoft.Terminal.Control.def +++ b/src/cascadia/TerminalControl/dll/Microsoft.Terminal.Control.def @@ -4,6 +4,7 @@ EXPORTS DllGetActivationFactory = WINRT_GetActivationFactory PRIVATE ; Flat C ABI + AvoidBuggyTSFConsoleFlags CreateTerminal DestroyTerminal TerminalCalculateResize diff --git a/src/cascadia/TerminalCore/Terminal.cpp b/src/cascadia/TerminalCore/Terminal.cpp index b6965e3037..aec06719d7 100644 --- a/src/cascadia/TerminalCore/Terminal.cpp +++ b/src/cascadia/TerminalCore/Terminal.cpp @@ -1297,9 +1297,15 @@ void Terminal::ScrollToSearchHighlight(til::CoordType searchScrollOffset) if (_searchHighlightFocused < _searchHighlights.size()) { const auto focused = til::at(_searchHighlights, _searchHighlightFocused); - const auto adjustedStart = til::point{ focused.start.x, std::max(0, focused.start.y - searchScrollOffset) }; - const auto adjustedEnd = til::point{ focused.end.x, std::max(0, focused.end.y - searchScrollOffset) }; - _ScrollToPoints(adjustedStart, adjustedEnd); + + // Only adjust the y coordinates if "start" is in a row that would be covered by the search box + auto adjustedStart = focused.start; + const auto firstVisibleRow = _VisibleStartIndex(); + if (focused.start.y > firstVisibleRow && focused.start.y < firstVisibleRow + searchScrollOffset) + { + adjustedStart.y = std::max(0, focused.start.y - searchScrollOffset); + } + _ScrollToPoints(adjustedStart, focused.end); } } diff --git a/src/cascadia/TerminalCore/terminalrenderdata.cpp b/src/cascadia/TerminalCore/terminalrenderdata.cpp index f2a9087a90..82f539f18b 100644 --- a/src/cascadia/TerminalCore/terminalrenderdata.cpp +++ b/src/cascadia/TerminalCore/terminalrenderdata.cpp @@ -152,29 +152,14 @@ const til::point_span* Terminal::GetSearchHighlightFocused() const noexcept // - The updated scroll offset til::CoordType Terminal::_ScrollToPoints(const til::point coordStart, const til::point coordEnd) { - auto notifyScrollChange = false; if (coordStart.y < _VisibleStartIndex()) { - // recalculate the scrollOffset - _scrollOffset = ViewStartIndex() - coordStart.y; - notifyScrollChange = true; + _ScrollToPoint(coordStart); } else if (coordEnd.y > _VisibleEndIndex()) { - // recalculate the scrollOffset, note that if the found text is - // beneath the current visible viewport, it may be within the - // current mutableViewport and the scrollOffset will be smaller - // than 0 - _scrollOffset = std::max(0, ViewStartIndex() - coordStart.y); - notifyScrollChange = true; + _ScrollToPoint(coordEnd); } - - if (notifyScrollChange) - { - _activeBuffer().TriggerScroll(); - _NotifyScrollEvent(); - } - return _VisibleStartIndex(); } diff --git a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp index 4cb9f655b7..7996535c97 100644 --- a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp +++ b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.cpp @@ -159,12 +159,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation CurrentColor(nullptr); } - safe_void_coroutine NullableColorPicker::MoreColors_Clicked(const IInspectable& /*sender*/, const RoutedEventArgs& /*args*/) - { - co_await ColorPickerDialog().ShowAsync(); - } - - void NullableColorPicker::ColorPickerDialog_Opened(const IInspectable& /*sender*/, const ContentDialogOpenedEventArgs& /*args*/) + void NullableColorPicker::Flyout_Opening(const IInspectable& /*sender*/, const IInspectable& /*args*/) { // Initialize color picker with current color if (CurrentColor()) @@ -185,7 +180,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation } } - void NullableColorPicker::ColorPickerDialog_PrimaryButtonClick(const IInspectable& /*sender*/, const ContentDialogButtonClickEventArgs& /*args*/) + void NullableColorPicker::Flyout_Closing(const winrt::Windows::Foundation::IInspectable& /*sender*/, const winrt::Windows::UI::Xaml::Controls::Primitives::FlyoutBaseClosingEventArgs& /*args*/) { const auto& selectedColor = ColorPickerControl().Color(); const Microsoft::Terminal::Core::Color terminalColor{ selectedColor.R, selectedColor.G, selectedColor.B, selectedColor.A }; diff --git a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h index 876ce23d7f..04ce05aa32 100644 --- a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h +++ b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.h @@ -22,10 +22,8 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation void ColorChip_DataContextChanged(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::DataContextChangedEventArgs& args); void NullColorButton_Clicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& args); - safe_void_coroutine MoreColors_Clicked(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& args); - - void ColorPickerDialog_Opened(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Controls::ContentDialogOpenedEventArgs& args); - void ColorPickerDialog_PrimaryButtonClick(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Controls::ContentDialogButtonClickEventArgs& args); + void Flyout_Opening(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& args); + void Flyout_Closing(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Controls::Primitives::FlyoutBaseClosingEventArgs& args); DEPENDENCY_PROPERTY(Editor::ColorSchemeViewModel, ColorSchemeVM); DEPENDENCY_PROPERTY(Windows::Foundation::IReference, CurrentColor); diff --git a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml index 6755543311..941f29b5e3 100644 --- a/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml +++ b/src/cascadia/TerminalSettingsEditor/NullableColorPicker.xaml @@ -97,25 +97,6 @@ - - - - @@ -146,8 +127,14 @@ diff --git a/src/cascadia/WpfTerminalControl/NativeMethods.cs b/src/cascadia/WpfTerminalControl/NativeMethods.cs index fdd42a1450..d5839accc3 100644 --- a/src/cascadia/WpfTerminalControl/NativeMethods.cs +++ b/src/cascadia/WpfTerminalControl/NativeMethods.cs @@ -171,6 +171,9 @@ namespace Microsoft.Terminal.Wpf SWP_SHOWWINDOW = 0x0040, } + [DllImport("Microsoft.Terminal.Control.dll", CharSet = CharSet.Unicode, CallingConvention = CallingConvention.StdCall, ExactSpelling = true)] + public static extern void AvoidBuggyTSFConsoleFlags(); + [DllImport("Microsoft.Terminal.Control.dll", CharSet = CharSet.Unicode, CallingConvention = CallingConvention.StdCall, PreserveSig = false)] public static extern void CreateTerminal(IntPtr parent, out IntPtr hwnd, out IntPtr terminal); diff --git a/src/cascadia/WpfTerminalControl/TerminalContainer.cs b/src/cascadia/WpfTerminalControl/TerminalContainer.cs index 547338f909..ffe11ea231 100644 --- a/src/cascadia/WpfTerminalControl/TerminalContainer.cs +++ b/src/cascadia/WpfTerminalControl/TerminalContainer.cs @@ -11,7 +11,6 @@ namespace Microsoft.Terminal.Wpf using System.Windows.Automation.Peers; using System.Windows.Interop; using System.Windows.Media; - using System.Windows.Threading; /// /// The container class that hosts the native hwnd terminal. @@ -32,6 +31,11 @@ namespace Microsoft.Terminal.Wpf /// public TerminalContainer() { + // WPF & TSF can't deal with us setting TF_TMAE_CONSOLE on the UI thread. + // It simply crashes on Windows 10 if you use the Emoji picker. + // (On later versions of Windows it just doesn't work.) + NativeMethods.AvoidBuggyTSFConsoleFlags(); + this.MessageHook += this.TerminalContainer_MessageHook; this.GotFocus += this.TerminalContainer_GotFocus; this.Focusable = true; diff --git a/src/tsf/Handle.cpp b/src/tsf/Handle.cpp index e60e251307..b7cae1d1be 100644 --- a/src/tsf/Handle.cpp +++ b/src/tsf/Handle.cpp @@ -16,6 +16,11 @@ Handle Handle::Create() return handle; } +void Handle::AvoidBuggyTSFConsoleFlags() +{ + Implementation::AvoidBuggyTSFConsoleFlags(); +} + void Handle::SetDefaultScopeAlphanumericHalfWidth(bool enable) { Implementation::SetDefaultScopeAlphanumericHalfWidth(enable); diff --git a/src/tsf/Handle.h b/src/tsf/Handle.h index 7e2e912574..c3be18ba93 100644 --- a/src/tsf/Handle.h +++ b/src/tsf/Handle.h @@ -33,6 +33,7 @@ namespace Microsoft::Console::TSF struct Handle { static Handle Create(); + static void AvoidBuggyTSFConsoleFlags(); static void SetDefaultScopeAlphanumericHalfWidth(bool enable); Handle() = default; diff --git a/src/tsf/Implementation.cpp b/src/tsf/Implementation.cpp index 7d0e2dfd34..99ea34c01c 100644 --- a/src/tsf/Implementation.cpp +++ b/src/tsf/Implementation.cpp @@ -27,9 +27,45 @@ static void TfPropertyvalClose(TF_PROPERTYVAL* val) } using unique_tf_propertyval = wil::unique_struct; +// The flags passed to ActivateEx don't replace the flags during previous calls. +// Instead, they're additive. So, if we pass flags that some concurrently running +// TSF clients don't expect we may blow them up accidentally. +// +// Such is the case with WPF (and TSF, which is actually at fault there). +// If you pass TF_TMAE_CONSOLE it'll instantly crash on Windows 10 on first text input. +// On Windows 11 it'll at least not crash but still make emoji input completely non-functional. +// +// -------- +// +// In any case, we pass the same flags as conhost v1: +// - TF_TMAE_UIELEMENTENABLEDONLY: TSF activates only text services that are +// categorized in GUID_TFCAT_TIPCAP_UIELEMENTENABLED. +// - TF_TMAE_NOACTIVATEKEYBOARDLAYOUT: TSF does not sync the current keyboard layout +// while this method is called. The keyboard layout will be adjusted when the +// calling thread gets focus. This flag must be used with TF_TMAE_NOACTIVATETIP. +// - TF_TMAE_CONSOLE: A text service is activated for console usage. +// Some IMEs are known to use this as a hint. Particularly a Korean IME can benefit +// from this, because Korean relies on "recomposing" previously finished compositions. +// That can't work in a terminal, since we submit composed text to the shell immediately. +// +// I'm not sure what TF_TMAE_UIELEMENTENABLEDONLY does. I tried to figure it out but failed. +// +// For TF_TMAE_NOACTIVATEKEYBOARDLAYOUT, I'm 99% sure it doesn't do anything, including in +// conhost v1. This is because IMM will be initialized on WM_ACTIVATE, which calls ActivateEx(0). +// Any subsequent ActivateEx() calls will update the flags, _except_ for this one and +// TF_TMAE_NOACTIVATETIP which are explicitly filtered out. +// +// TF_TMAE_NOACTIVATETIP however is important. Without it, TIPs are immediately initialized. +static std::atomic s_activationFlags{ TF_TMAE_NOACTIVATETIP | TF_TMAE_UIELEMENTENABLEDONLY | TF_TMAE_NOACTIVATEKEYBOARDLAYOUT | TF_TMAE_CONSOLE }; +void Implementation::AvoidBuggyTSFConsoleFlags() noexcept +{ + s_activationFlags.fetch_and(~static_cast(TF_TMAE_CONSOLE), std::memory_order_relaxed); +} + +static std::atomic s_wantsAnsiInputScope{ false }; void Implementation::SetDefaultScopeAlphanumericHalfWidth(bool enable) noexcept { - _wantsAnsiInputScope.store(enable, std::memory_order_relaxed); + s_wantsAnsiInputScope.store(enable, std::memory_order_relaxed); } void Implementation::Initialize() @@ -40,7 +76,7 @@ void Implementation::Initialize() // There's no point in calling TF_GetThreadMgr. ITfThreadMgr is a per-thread singleton. _threadMgrEx = wil::CoCreateInstance(CLSID_TF_ThreadMgr, CLSCTX_INPROC_SERVER); - THROW_IF_FAILED(_threadMgrEx->ActivateEx(&_clientId, TF_TMAE_CONSOLE)); + THROW_IF_FAILED(_threadMgrEx->ActivateEx(&_clientId, s_activationFlags.load(std::memory_order_relaxed))); THROW_IF_FAILED(_threadMgrEx->CreateDocumentMgr(_documentMgr.addressof())); TfEditCookie ecTextStore; @@ -319,7 +355,7 @@ STDMETHODIMP Implementation::GetWnd(HWND* phwnd) noexcept STDMETHODIMP Implementation::GetAttribute(REFGUID rguidAttribute, VARIANT* pvarValue) noexcept { - if (_wantsAnsiInputScope.load(std::memory_order_relaxed) && IsEqualGUID(rguidAttribute, GUID_PROP_INPUTSCOPE)) + if (s_wantsAnsiInputScope.load(std::memory_order_relaxed) && IsEqualGUID(rguidAttribute, GUID_PROP_INPUTSCOPE)) { _ansiInputScope.AddRef(); pvarValue->vt = VT_UNKNOWN; diff --git a/src/tsf/Implementation.h b/src/tsf/Implementation.h index fecec35cea..82e84ab888 100644 --- a/src/tsf/Implementation.h +++ b/src/tsf/Implementation.h @@ -16,6 +16,7 @@ namespace Microsoft::Console::TSF struct Implementation : ITfContextOwner, ITfContextOwnerCompositionSink, ITfTextEditSink { + static void AvoidBuggyTSFConsoleFlags() noexcept; static void SetDefaultScopeAlphanumericHalfWidth(bool enable) noexcept; virtual ~Implementation() = default; @@ -129,6 +130,5 @@ namespace Microsoft::Console::TSF int _compositions = 0; AnsiInputScope _ansiInputScope{ this }; - inline static std::atomic _wantsAnsiInputScope{ false }; }; }