From 1bcc85a60b141a1f7f9cea94ce414d078813aef7 Mon Sep 17 00:00:00 2001 From: Carlos Zamora Date: Wed, 18 Mar 2020 11:01:05 -0700 Subject: [PATCH] Fire UIA Events for Output and Cursor Movement (#4826) ## Summary of the Pull Request This notifies automation clients (i.e.: NVDA, narrator, etc...) of new output being rendered to the screen. ## References Close #2447 - Signaling for new output and cursor Close #3791 - fixed by signaling cursor changes ## Detailed Description of the Pull Request / Additional comments - Added tracing for UiaRenderer. This makes it easier to debug issues with notifying an automation client. - Fire TextChanged automation events when new content is output to the screen. ## Validation Steps Performed Verified with NVDA [1] ## Narrator Narrator works _better_, but is unable to detect new output consistently. There is no harm for narrator when this change goes in. [1] https://github.com/microsoft/terminal/issues/2447#issuecomment-595879890 --- .../TermControlAutomationPeer.cpp | 11 +++++- src/renderer/uia/UiaRenderer.cpp | 31 ++++++++++++++-- src/renderer/uia/UiaRenderer.hpp | 2 + src/types/UiaTracing.cpp | 37 +++++++++++++++++++ src/types/UiaTracing.h | 8 ++++ 5 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp b/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp index f50ba2ac6de..94f0ef0e361 100644 --- a/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp +++ b/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp @@ -9,6 +9,7 @@ #include "TermControlAutomationPeer.g.cpp" #include "XamlUiaTextRange.h" +#include "..\types\UiaTracing.h" using namespace Microsoft::Console::Types; using namespace winrt::Windows::UI::Xaml::Automation::Peers; @@ -44,6 +45,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - void TermControlAutomationPeer::SignalSelectionChanged() { + UiaTracing::Signal::SelectionChanged(); Dispatcher().RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [&]() { // The event that is raised when the text selection is modified. RaiseAutomationEvent(AutomationEvents::TextPatternOnTextSelectionChanged); @@ -58,6 +60,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - void TermControlAutomationPeer::SignalTextChanged() { + UiaTracing::Signal::TextChanged(); Dispatcher().RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [&]() { // The event that is raised when textual content is modified. RaiseAutomationEvent(AutomationEvents::TextPatternOnTextChanged); @@ -72,9 +75,15 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation // - void TermControlAutomationPeer::SignalCursorChanged() { + UiaTracing::Signal::CursorChanged(); Dispatcher().RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [&]() { // The event that is raised when the text was changed in an edit control. - RaiseAutomationEvent(AutomationEvents::TextEditTextChanged); + // Do NOT fire a TextEditTextChanged. Generally, an app on the other side + // will expect more information. Though you can dispatch that event + // on its own, it may result in a nullptr exception on the other side + // because no additional information was provided. Crashing the screen + // reader. + RaiseAutomationEvent(AutomationEvents::TextPatternOnTextSelectionChanged); }); } diff --git a/src/renderer/uia/UiaRenderer.cpp b/src/renderer/uia/UiaRenderer.cpp index 6a20a7a171b..01235ca1cfe 100644 --- a/src/renderer/uia/UiaRenderer.cpp +++ b/src/renderer/uia/UiaRenderer.cpp @@ -17,6 +17,8 @@ UiaEngine::UiaEngine(IUiaEventDispatcher* dispatcher) : _dispatcher{ THROW_HR_IF_NULL(E_INVALIDARG, dispatcher) }, _isPainting{ false }, _selectionChanged{ false }, + _textBufferChanged{ false }, + _cursorChanged{ false }, _isEnabled{ true }, _prevSelection{}, RenderEngineBase() @@ -56,7 +58,8 @@ UiaEngine::UiaEngine(IUiaEventDispatcher* dispatcher) : // - S_OK, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT UiaEngine::Invalidate(const SMALL_RECT* const /*psrRegion*/) noexcept { - return S_FALSE; + _textBufferChanged = true; + return S_OK; } // Routine Description: @@ -68,6 +71,7 @@ UiaEngine::UiaEngine(IUiaEventDispatcher* dispatcher) : // - S_FALSE [[nodiscard]] HRESULT UiaEngine::InvalidateCursor(const COORD* const /*pcoordCursor*/) noexcept { + _cursorChanged = true; return S_FALSE; } @@ -150,7 +154,8 @@ UiaEngine::UiaEngine(IUiaEventDispatcher* dispatcher) : // - S_OK, else an appropriate HRESULT for failing to allocate or write. [[nodiscard]] HRESULT UiaEngine::InvalidateAll() noexcept { - return S_FALSE; + _textBufferChanged = true; + return S_OK; } // Routine Description: @@ -192,10 +197,10 @@ UiaEngine::UiaEngine(IUiaEventDispatcher* dispatcher) : RETURN_HR_IF(S_FALSE, !_isEnabled); // add more events here - // bool somethingToDo = _selectionChanged; + const bool somethingToDo = _selectionChanged || _textBufferChanged || _cursorChanged; // If there's nothing to do, quick return - RETURN_HR_IF(S_FALSE, !_selectionChanged); + RETURN_HR_IF(S_FALSE, !somethingToDo); _isPainting = true; return S_OK; @@ -221,8 +226,26 @@ UiaEngine::UiaEngine(IUiaEventDispatcher* dispatcher) : } CATCH_LOG(); } + if (_textBufferChanged) + { + try + { + _dispatcher->SignalTextChanged(); + } + CATCH_LOG(); + } + if (_cursorChanged) + { + try + { + _dispatcher->SignalCursorChanged(); + } + CATCH_LOG(); + } _selectionChanged = false; + _textBufferChanged = false; + _cursorChanged = false; _prevSelection.clear(); _isPainting = false; diff --git a/src/renderer/uia/UiaRenderer.hpp b/src/renderer/uia/UiaRenderer.hpp index 4b0eb55b13f..bdebb01de27 100644 --- a/src/renderer/uia/UiaRenderer.hpp +++ b/src/renderer/uia/UiaRenderer.hpp @@ -82,6 +82,8 @@ namespace Microsoft::Console::Render bool _isEnabled; bool _isPainting; bool _selectionChanged; + bool _textBufferChanged; + bool _cursorChanged; Microsoft::Console::Types::IUiaEventDispatcher* _dispatcher; diff --git a/src/types/UiaTracing.cpp b/src/types/UiaTracing.cpp index d416c715fbb..34b7fbbf346 100644 --- a/src/types/UiaTracing.cpp +++ b/src/types/UiaTracing.cpp @@ -613,4 +613,41 @@ void UiaTracing::TextProvider::get_SupportedTextSelection(const ScreenInfoUiaPro TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); } } + +void UiaTracing::Signal::SelectionChanged() noexcept +{ + EnsureRegistration(); + if (TraceLoggingProviderEnabled(g_UiaProviderTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + TraceLoggingWrite( + g_UiaProviderTraceProvider, + "Signal::SelectionChanged", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } +} + +void UiaTracing::Signal::TextChanged() noexcept +{ + EnsureRegistration(); + if (TraceLoggingProviderEnabled(g_UiaProviderTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + TraceLoggingWrite( + g_UiaProviderTraceProvider, + "Signal::TextChanged", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } +} + +void UiaTracing::Signal::CursorChanged() noexcept +{ + EnsureRegistration(); + if (TraceLoggingProviderEnabled(g_UiaProviderTraceProvider, WINEVENT_LEVEL_VERBOSE, 0)) + { + TraceLoggingWrite( + g_UiaProviderTraceProvider, + "Signal::CursorChanged", + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE)); + } +} + #pragma warning(pop) diff --git a/src/types/UiaTracing.h b/src/types/UiaTracing.h index 12c353c86b8..f2f155358d0 100644 --- a/src/types/UiaTracing.h +++ b/src/types/UiaTracing.h @@ -72,6 +72,14 @@ namespace Microsoft::Console::Types static void get_SupportedTextSelection(const ScreenInfoUiaProviderBase& base, SupportedTextSelection result) noexcept; }; + class Signal final + { + public: + static void SelectionChanged() noexcept; + static void TextChanged() noexcept; + static void CursorChanged() noexcept; + }; + private: // Implement this as a singleton class. static UiaTracing& EnsureRegistration() noexcept