-
Notifications
You must be signed in to change notification settings - Fork 9.2k
Use UIA notifications for text output #12358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
91eeb59
2108d08
320b1f8
a369772
e2373d1
aa7ecb4
31762fc
551d648
768dd7c
facc78c
ffb44d7
10d3eb4
52ddfaa
8bdd809
ef06bd8
e5e5831
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2084,6 +2084,7 @@ rxvt | |
| safearray | ||
| SAFECAST | ||
| safemath | ||
| sapi | ||
| sba | ||
| SBCS | ||
| SBCSDBCS | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,43 @@ namespace XamlAutomation | |
| using winrt::Windows::UI::Xaml::Automation::Provider::ITextRangeProvider; | ||
| } | ||
|
|
||
| static constexpr wchar_t UNICODE_NEWLINE{ L'\n' }; | ||
|
|
||
| // Method Description: | ||
| // - creates a copy of the provided text with all of the control characters removed | ||
| // Arguments: | ||
| // - text: the string we're sanitizing | ||
| // Return Value: | ||
| // - a copy of "sanitized" with all of the control characters removed | ||
| static std::wstring Sanitize(std::wstring_view text) | ||
| { | ||
| std::wstring sanitized{ text }; | ||
| sanitized.erase(std::remove_if(sanitized.begin(), sanitized.end(), [](wchar_t c) { | ||
| return (c < UNICODE_SPACE && c != UNICODE_NEWLINE) || c == 0x7F /*DEL*/; | ||
| }), | ||
| sanitized.end()); | ||
| return sanitized; | ||
| } | ||
|
|
||
| // Method Description: | ||
| // - verifies if a given string has text that would be read by a screen reader. | ||
| // - a string of control characters, for example, would not be read. | ||
| // Arguments: | ||
| // - text: the string we're validating | ||
| // Return Value: | ||
| // - true, if the text is readable. false, otherwise. | ||
| static constexpr bool IsReadable(std::wstring_view text) | ||
| { | ||
| for (const auto c : text) | ||
| { | ||
| if (c > UNICODE_SPACE) | ||
| { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
lhecker marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| namespace winrt::Microsoft::Terminal::Control::implementation | ||
| { | ||
| TermControlAutomationPeer::TermControlAutomationPeer(TermControl* owner, | ||
|
|
@@ -45,6 +82,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.NewOutput([this](auto&&, hstring newOutput) { NotifyNewOutput(newOutput); }); | ||
| _contentAutomationPeer.ParentProvider(*this); | ||
| }; | ||
|
|
||
|
|
@@ -68,6 +106,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
| _contentAutomationPeer.SetControlPadding(padding); | ||
| } | ||
|
|
||
| void TermControlAutomationPeer::RecordKeyEvent(const WORD vkey) | ||
| { | ||
| if (const auto charCode{ MapVirtualKey(vkey, MAPVK_VK_TO_CHAR) }) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. something tells me there will be future bugs here related to unicode input, but this is probably fine for now. My memory harkens back to
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh but there's the |
||
| { | ||
| if (const auto keyEventChar{ gsl::narrow_cast<wchar_t>(charCode) }; IsReadable({ &keyEventChar, 1 })) | ||
| { | ||
| _keyEvents.emplace_back(keyEventChar); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Method Description: | ||
| // - Signals the ui automation client that the terminal's selection has changed and should be updated | ||
| // Arguments: | ||
|
|
@@ -142,8 +191,66 @@ namespace winrt::Microsoft::Terminal::Control::implementation | |
| }); | ||
| } | ||
|
|
||
| void TermControlAutomationPeer::NotifyNewOutput(std::wstring_view newOutput) | ||
| { | ||
| // Try to suppress any events (or event data) | ||
| // that is just the keypress the user made | ||
| auto sanitized{ Sanitize(newOutput) }; | ||
| while (!_keyEvents.empty() && IsReadable(sanitized)) | ||
| { | ||
| if (til::toupper_ascii(sanitized.front()) == _keyEvents.front()) | ||
| { | ||
| // the key event's character (i.e. the "A" key) matches | ||
| // the output character (i.e. "a" or "A" text). | ||
| // We can assume that the output character resulted from | ||
| // the pressed key, so we can ignore it. | ||
| sanitized = sanitized.substr(1); | ||
| _keyEvents.pop_front(); | ||
| } | ||
| else | ||
| { | ||
| // The output doesn't match, | ||
| // so clear the input stack and | ||
| // move on to fire the event. | ||
| _keyEvents.clear(); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| // Suppress event if the remaining text is not readable | ||
| if (!IsReadable(sanitized)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| auto dispatcher{ Dispatcher() }; | ||
| if (!dispatcher) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| // IMPORTANT: | ||
| // [1] make sure the scope returns a copy of "sanitized" so that it isn't accidentally deleted | ||
| // [2] AutomationNotificationProcessing::All --> ensures it can be interrupted by keyboard events | ||
| // [3] Do not "RunAsync(...).get()". For whatever reason, this causes NVDA to just not receive "SignalTextChanged()"'s events. | ||
| dispatcher.RunAsync(Windows::UI::Core::CoreDispatcherPriority::Normal, [weakThis{ get_weak() }, sanitizedCopy{ hstring{ sanitized } }]() { | ||
| if (auto strongThis{ weakThis.get() }) | ||
| { | ||
| try | ||
| { | ||
| strongThis->RaiseNotificationEvent(AutomationNotificationKind::ActionCompleted, | ||
| AutomationNotificationProcessing::All, | ||
| sanitizedCopy, | ||
| L"TerminalTextOutput"); | ||
| } | ||
| CATCH_LOG(); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| hstring TermControlAutomationPeer::GetClassNameCore() const | ||
| { | ||
| // IMPORTANT: Do NOT change the name. Screen readers like JAWS may be dependent on this being "TermControl". | ||
| return L"TermControl"; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1002,6 +1002,11 @@ void Terminal::_WriteBuffer(const std::wstring_view& stringView) | |
| _AdjustCursorPosition(proposedCursorPosition); | ||
| } | ||
|
|
||
| // Notify UIA of new text. | ||
| // It's important to do this here instead of in TextBuffer, because here you have access to the entire line of text, | ||
| // whereas TextBuffer writes it one character at a time via the OutputCellIterator. | ||
| _buffer->GetRenderTarget().TriggerNewTextNotification(stringView); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It feels weird to reach all the way down through the renderer to tell the TermControlAutomationPeer that there was text. sequenceDiagram
TerminalControl ->> TerminalCore: Text
TerminalCore ->> Renderer: Text
Renderer ->> DX Engine: Text
Renderer ->> UIA Engine: Text
UIA Engine ->> TCAP: Text
TCAP ->> Narrator: Text
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be fair, the UIA Engine already operated this way. It just didn't know what text was written (only if text was written). |
||
|
|
||
| cursor.EndDeferDrawing(); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,3 +110,13 @@ void ScreenBufferRenderTarget::TriggerTitleChange() | |
| pRenderer->TriggerTitleChange(); | ||
| } | ||
| } | ||
|
|
||
| void ScreenBufferRenderTarget::TriggerNewTextNotification(const std::wstring_view newText) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does conhost need this?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not yet! Conhost doesn't even use a UIA engine, so this code never gets called. Bill has expressed interest that Conhost should switch over to this notification system because it would make screen readers' lives so much easier (it removes the need to diff). But it makes sense to test this out on Windows Terminal first, then update Conhost later. |
||
| { | ||
| auto* pRenderer = ServiceLocator::LocateGlobals().pRender; | ||
| const auto* pActive = &ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer().GetActiveBuffer(); | ||
| if (pRenderer != nullptr && pActive == &_owner) | ||
| { | ||
| pRenderer->TriggerNewTextNotification(newText); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,6 +143,11 @@ constexpr HRESULT vec2_narrow(U x, U y, AtlasEngine::vec2<T>& out) noexcept | |
| return S_OK; | ||
| } | ||
|
|
||
| [[nodiscard]] HRESULT AtlasEngine::NotifyNewText(const std::wstring_view newText) noexcept | ||
| { | ||
| return S_OK; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you added a base implementation that returns
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. AtlasEngine doesn't inherit from |
||
| } | ||
|
|
||
| [[nodiscard]] HRESULT AtlasEngine::UpdateFont(const FontInfoDesired& fontInfoDesired, _Out_ FontInfo& fontInfo) noexcept | ||
| { | ||
| return UpdateFont(fontInfoDesired, fontInfo, {}, {}); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.