From 02ac246807993e57dd5b6061bee1385f186ae20d Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 13 Oct 2021 16:01:43 -0700 Subject: [PATCH] Properly initialize XamlUiaTextRange with ProviderFromPeer (#11501) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request As a part of the Interactivity split, `TermControlAutomationPeer` had to be split into `TermControlAutomationPeer` (TCAP) and `InteractivityAutomationPeer` (IAP). Just about all of the functions in `InterativityAutomationPeer` operate by calling the non-XAML UIA Provider then wrapping the resulting `UIATextRange` into a XAML format (a `XamlUiaTextRange` [XUTR]). As a part of that XUTR constructor, we need a reference to the parent provider. We generally get that via `ProviderFromPeer()`, but IAP's `ProviderFromPeer()` returned null (presumably because IAP isn't in the UI tree, whereas TCAP is directly registered as the automation peer for the `TermControl`). It looks like some screen readers didn't care (like NVDA, though there may be a chance we just didn't encounter an issue just yet), but Narrator definitely did. The fix was to provide XUTR constructors the `ProviderFromPeer` from TCAP, _not_ IAP. To accomplish this, IAP now holds a weak reference to TCAP, and provides the `ProviderFromPeer` when needed. We can't cache this result because there is no guarantee that it won't change. Some miscellaneous changes include: - `TermControl::OnCreateAutomationPeer` now returns the existing auto peer instead of always creating a new one - `TCAP::WrapArrayOfTextRangeProviders` was removed as it was unused (normally, this would be directly affected by the main `ProviderFromPeer` change here) - `XUTR::GetEnclosingElement` is now hooked up to trace logging for debugging purposes ## References Introduced in #10051 Closes #11488 ## Validation Steps Performed ✅ Narrator scan mode now works (verified with character, word, and line navigation) ✅ NVDA movement still works (verified with word and line navigation) --- .../InteractivityAutomationPeer.cpp | 40 ++++++++++++------- .../InteractivityAutomationPeer.h | 4 ++ .../InteractivityAutomationPeer.idl | 2 +- .../TermControlAutomationPeer.cpp | 27 +------------ .../TermControlAutomationPeer.h | 2 - .../TerminalControl/XamlUiaTextRange.cpp | 2 + 6 files changed, 33 insertions(+), 44 deletions(-) diff --git a/src/cascadia/TerminalControl/InteractivityAutomationPeer.cpp b/src/cascadia/TerminalControl/InteractivityAutomationPeer.cpp index 9d028b484..e34811e73 100644 --- a/src/cascadia/TerminalControl/InteractivityAutomationPeer.cpp +++ b/src/cascadia/TerminalControl/InteractivityAutomationPeer.cpp @@ -43,6 +43,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation { _controlPadding = padding; } + void InteractivityAutomationPeer::ParentProvider(AutomationPeer parentProvider) + { + _parentProvider = parentProvider; + } // Method Description: // - Signals the ui automation client that the terminal's selection has @@ -110,30 +114,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation // ScreenInfoUiaProvider doesn't actually use parameter, so just pass in nullptr THROW_IF_FAILED(_uiaProvider->RangeFromChild(/* IRawElementProviderSimple */ nullptr, &returnVal)); - - const auto parentProvider = this->ProviderFromPeer(*this); - const auto xutr = winrt::make_self(returnVal, parentProvider); - return xutr.as(); + return _CreateXamlUiaTextRange(returnVal); } XamlAutomation::ITextRangeProvider InteractivityAutomationPeer::RangeFromPoint(Windows::Foundation::Point screenLocation) { UIA::ITextRangeProvider* returnVal; THROW_IF_FAILED(_uiaProvider->RangeFromPoint({ screenLocation.X, screenLocation.Y }, &returnVal)); - - const auto parentProvider = this->ProviderFromPeer(*this); - const auto xutr = winrt::make_self(returnVal, parentProvider); - return xutr.as(); + return _CreateXamlUiaTextRange(returnVal); } XamlAutomation::ITextRangeProvider InteractivityAutomationPeer::DocumentRange() { UIA::ITextRangeProvider* returnVal; THROW_IF_FAILED(_uiaProvider->get_DocumentRange(&returnVal)); - - const auto parentProvider = this->ProviderFromPeer(*this); - const auto xutr = winrt::make_self(returnVal, parentProvider); - return xutr.as(); + return _CreateXamlUiaTextRange(returnVal); } XamlAutomation::SupportedTextSelection InteractivityAutomationPeer::SupportedTextSelection() @@ -180,6 +175,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation } #pragma endregion + XamlAutomation::ITextRangeProvider InteractivityAutomationPeer::_CreateXamlUiaTextRange(UIA::ITextRangeProvider* returnVal) const + { + // LOAD-BEARING: use _parentProvider->ProviderFromPeer(_parentProvider) instead of this->ProviderFromPeer(*this). + // Since we split the automation peer into TermControlAutomationPeer and InteractivityAutomationPeer, + // using "this" returns null. This can cause issues with some UIA Client scenarios like any navigation in Narrator. + const auto parent{ _parentProvider.get() }; + if (!parent) + { + return nullptr; + } + const auto xutr = winrt::make_self(returnVal, parent.ProviderFromPeer(parent)); + return xutr.as(); + }; + // Method Description: // - extracts the UiaTextRanges from the SAFEARRAY and converts them to Xaml ITextRangeProviders // Arguments: @@ -194,11 +203,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation std::vector vec; vec.reserve(count); - auto parentProvider = this->ProviderFromPeer(*this); for (int i = 0; i < count; i++) { - auto xutr = make_self(providers[i].detach(), parentProvider); - vec.emplace_back(xutr.as()); + if (auto xutr = _CreateXamlUiaTextRange(providers[i].detach())) + { + vec.emplace_back(std::move(xutr)); + } } com_array result{ vec }; diff --git a/src/cascadia/TerminalControl/InteractivityAutomationPeer.h b/src/cascadia/TerminalControl/InteractivityAutomationPeer.h index 893478e91..144ea20ef 100644 --- a/src/cascadia/TerminalControl/InteractivityAutomationPeer.h +++ b/src/cascadia/TerminalControl/InteractivityAutomationPeer.h @@ -43,6 +43,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void SetControlBounds(const Windows::Foundation::Rect bounds); void SetControlPadding(const Core::Padding padding); + void ParentProvider(Windows::UI::Xaml::Automation::Peers::AutomationPeer parentProvider); #pragma region IUiaEventDispatcher void SignalSelectionChanged() override; @@ -74,8 +75,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation TYPED_EVENT(CursorChanged, IInspectable, IInspectable); private: + Windows::UI::Xaml::Automation::Provider::ITextRangeProvider _CreateXamlUiaTextRange(::ITextRangeProvider* returnVal) const; + ::Microsoft::WRL::ComPtr<::Microsoft::Terminal::TermControlUiaProvider> _uiaProvider; winrt::Microsoft::Terminal::Control::implementation::ControlInteractivity* _interactivity; + weak_ref _parentProvider; til::rectangle _controlBounds{}; til::rectangle _controlPadding{}; diff --git a/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl b/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl index 4c9039c5c..43d4bd35e 100644 --- a/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl +++ b/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl @@ -1,6 +1,5 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. - namespace Microsoft.Terminal.Control { [default_interface] runtimeclass InteractivityAutomationPeer : @@ -10,6 +9,7 @@ namespace Microsoft.Terminal.Control void SetControlBounds(Windows.Foundation.Rect bounds); void SetControlPadding(Microsoft.Terminal.Core.Padding padding); + void ParentProvider(Windows.UI.Xaml.Automation.Peers.AutomationPeer parentProvider); event Windows.Foundation.TypedEventHandler SelectionChanged; event Windows.Foundation.TypedEventHandler TextChanged; diff --git a/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp b/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp index 3cfae1cce..d910b70bb 100644 --- a/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp +++ b/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp @@ -45,6 +45,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _contentAutomationPeer.SelectionChanged([this](auto&&, auto&&) { SignalSelectionChanged(); }); _contentAutomationPeer.TextChanged([this](auto&&, auto&&) { SignalTextChanged(); }); _contentAutomationPeer.CursorChanged([this](auto&&, auto&&) { SignalCursorChanged(); }); + _contentAutomationPeer.ParentProvider(*this); }; // Method Description: @@ -226,30 +227,4 @@ namespace winrt::Microsoft::Terminal::Control::implementation } #pragma endregion - - // Method Description: - // - extracts the UiaTextRanges from the SAFEARRAY and converts them to Xaml ITextRangeProviders - // Arguments: - // - SAFEARRAY of UIA::UiaTextRange (ITextRangeProviders) - // Return Value: - // - com_array of Xaml Wrapped UiaTextRange (ITextRangeProviders) - com_array TermControlAutomationPeer::WrapArrayOfTextRangeProviders(SAFEARRAY* textRanges) - { - // transfer ownership of UiaTextRanges to this new vector - auto providers = SafeArrayToOwningVector<::Microsoft::Terminal::TermControlUiaTextRange>(textRanges); - int count = gsl::narrow(providers.size()); - - std::vector vec; - vec.reserve(count); - auto parentProvider = this->ProviderFromPeer(*this); - for (int i = 0; i < count; i++) - { - auto xutr = make_self(providers[i].detach(), parentProvider); - vec.emplace_back(xutr.as()); - } - - com_array result{ vec }; - - return result; - } } diff --git a/src/cascadia/TerminalControl/TermControlAutomationPeer.h b/src/cascadia/TerminalControl/TermControlAutomationPeer.h index e5a2cc7ff..d912a1dec 100644 --- a/src/cascadia/TerminalControl/TermControlAutomationPeer.h +++ b/src/cascadia/TerminalControl/TermControlAutomationPeer.h @@ -78,7 +78,5 @@ namespace winrt::Microsoft::Terminal::Control::implementation private: winrt::Microsoft::Terminal::Control::implementation::TermControl* _termControl; Control::InteractivityAutomationPeer _contentAutomationPeer; - - winrt::com_array WrapArrayOfTextRangeProviders(SAFEARRAY* textRanges); }; } diff --git a/src/cascadia/TerminalControl/XamlUiaTextRange.cpp b/src/cascadia/TerminalControl/XamlUiaTextRange.cpp index 7e5dbcf67..58d8aa934 100644 --- a/src/cascadia/TerminalControl/XamlUiaTextRange.cpp +++ b/src/cascadia/TerminalControl/XamlUiaTextRange.cpp @@ -6,6 +6,7 @@ #include "../types/TermControlUiaTextRange.hpp" #include #include +#include "../types/UiaTracing.h" // the same as COR_E_NOTSUPPORTED // we don't want to import the CLR headers to get it @@ -182,6 +183,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation XamlAutomation::IRawElementProviderSimple XamlUiaTextRange::GetEnclosingElement() { + ::Microsoft::Console::Types::UiaTracing::TextRange::GetEnclosingElement(*static_cast<::Microsoft::Console::Types::UiaTextRangeBase*>(_uiaProvider.get())); return _parentProvider; }