Skip to content

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

Merged
16 commits merged into from
Mar 11, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/actions/spelling/expect/expect.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2075,6 +2075,7 @@ rxvt
safearray
SAFECAST
safemath
sapi
sba
SBCS
SBCSDBCS
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalControl/InteractivityAutomationPeer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
_CursorChangedHandlers(*this, nullptr);
}

void InteractivityAutomationPeer::NotifyNewOutput(std::wstring_view newOutput)
{
_NewOutputHandlers(*this, hstring{ newOutput });
}

#pragma region ITextProvider
com_array<XamlAutomation::ITextRangeProvider> InteractivityAutomationPeer::GetSelection()
{
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalControl/InteractivityAutomationPeer.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void SignalSelectionChanged() override;
void SignalTextChanged() override;
void SignalCursorChanged() override;
void NotifyNewOutput(std::wstring_view newOutput) override;
#pragma endregion

#pragma region ITextProvider Pattern
Expand All @@ -73,6 +74,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
TYPED_EVENT(SelectionChanged, IInspectable, IInspectable);
TYPED_EVENT(TextChanged, IInspectable, IInspectable);
TYPED_EVENT(CursorChanged, IInspectable, IInspectable);
TYPED_EVENT(NewOutput, IInspectable, hstring);

private:
Windows::UI::Xaml::Automation::Provider::ITextRangeProvider _CreateXamlUiaTextRange(::ITextRangeProvider* returnVal) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,6 @@ namespace Microsoft.Terminal.Control
event Windows.Foundation.TypedEventHandler<Object, Object> SelectionChanged;
event Windows.Foundation.TypedEventHandler<Object, Object> TextChanged;
event Windows.Foundation.TypedEventHandler<Object, Object> CursorChanged;
event Windows.Foundation.TypedEventHandler<Object, String> NewOutput;
}
}
5 changes: 5 additions & 0 deletions src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation
keyDown) :
true;

if (vkey && keyDown && _automationPeer)
{
get_self<TermControlAutomationPeer>(_automationPeer)->RecordKeyEvent(vkey);
}

if (_cursorTimer)
{
// Manually show the cursor when a key is pressed. Restarting
Expand Down
107 changes: 107 additions & 0 deletions src/cascadia/TerminalControl/TermControlAutomationPeer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

namespace winrt::Microsoft::Terminal::Control::implementation
{
TermControlAutomationPeer::TermControlAutomationPeer(TermControl* owner,
Expand All @@ -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);
};

Expand All @@ -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) })
Copy link
Member

Choose a reason for hiding this comment

The 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 Terminal::_CharacterFromKeyEvent

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh but there's the towupper thing below... That might be more complicated than the Terminal version. Still, probably good enough for now.

{
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:
Expand Down Expand Up @@ -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";
}

Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalControl/TermControlAutomationPeer.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation

void UpdateControlBounds();
void SetControlPadding(const Core::Padding padding);
void RecordKeyEvent(const WORD vkey);

#pragma region FrameworkElementAutomationPeer
hstring GetClassNameCore() const;
Expand All @@ -64,6 +65,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
void SignalSelectionChanged() override;
void SignalTextChanged() override;
void SignalCursorChanged() override;
void NotifyNewOutput(std::wstring_view newOutput) override;
#pragma endregion

#pragma region ITextProvider Pattern
Expand All @@ -78,5 +80,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
private:
winrt::Microsoft::Terminal::Control::implementation::TermControl* _termControl;
Control::InteractivityAutomationPeer _contentAutomationPeer;
std::deque<wchar_t> _keyEvents;
};
}
5 changes: 5 additions & 0 deletions src/cascadia/TerminalCore/Terminal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The 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
Loading

Copy link
Member Author

Choose a reason for hiding this comment

The 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();
}

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/UnitTests_TerminalCore/ScrollTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ namespace
};
virtual void TriggerCircling(){};
void TriggerTitleChange(){};
void TriggerNewTextNotification(const std::wstring_view){};

private:
std::optional<COORD> _triggerScrollDelta;
Expand Down
10 changes: 10 additions & 0 deletions src/host/ScreenBufferRenderTarget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,3 +110,13 @@ void ScreenBufferRenderTarget::TriggerTitleChange()
pRenderer->TriggerTitleChange();
}
}

void ScreenBufferRenderTarget::TriggerNewTextNotification(const std::wstring_view newText)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does conhost need this?

Copy link
Member Author

Choose a reason for hiding this comment

The 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);
}
}
1 change: 1 addition & 0 deletions src/host/ScreenBufferRenderTarget.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class ScreenBufferRenderTarget final : public Microsoft::Console::Render::IRende
void TriggerScroll(const COORD* const pcoordDelta) override;
void TriggerCircling() override;
void TriggerTitleChange() override;
void TriggerNewTextNotification(const std::wstring_view newText) override;

private:
SCREEN_INFORMATION& _owner;
Expand Down
5 changes: 5 additions & 0 deletions src/renderer/atlas/AtlasEngine.api.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you added a base implementation that returns S_FALSE; does atlas need an S_OK version?

Copy link
Member

@lhecker lhecker Mar 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AtlasEngine doesn't inherit from RenderEngineBase.
(I did this so I can control the implementation as much as possible.)

}

[[nodiscard]] HRESULT AtlasEngine::UpdateFont(const FontInfoDesired& fontInfoDesired, _Out_ FontInfo& fontInfo) noexcept
{
return UpdateFont(fontInfoDesired, fontInfo, {}, {});
Expand Down
1 change: 1 addition & 0 deletions src/renderer/atlas/AtlasEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace Microsoft::Console::Render
[[nodiscard]] HRESULT InvalidateAll() noexcept override;
[[nodiscard]] HRESULT InvalidateCircling(_Out_ bool* pForcePaint) noexcept override;
[[nodiscard]] HRESULT InvalidateTitle(std::wstring_view proposedTitle) noexcept override;
[[nodiscard]] HRESULT NotifyNewText(const std::wstring_view newText) noexcept override;
[[nodiscard]] HRESULT PrepareRenderInfo(const RenderFrameInfo& info) noexcept override;
[[nodiscard]] HRESULT ResetLineTransform() noexcept override;
[[nodiscard]] HRESULT PrepareLineTransform(LineRendition lineRendition, size_t targetRow, size_t viewportLeft) noexcept override;
Expand Down
5 changes: 5 additions & 0 deletions src/renderer/base/RenderEngineBase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ HRESULT RenderEngineBase::UpdateTitle(const std::wstring_view newTitle) noexcept
return hr;
}

HRESULT RenderEngineBase::NotifyNewText(const std::wstring_view /*newText*/) noexcept
{
return S_FALSE;
}

HRESULT RenderEngineBase::UpdateSoftFont(const gsl::span<const uint16_t> /*bitPattern*/,
const SIZE /*cellSize*/,
const size_t /*centeringHint*/) noexcept
Expand Down
8 changes: 8 additions & 0 deletions src/renderer/base/renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,14 @@ void Renderer::TriggerTitleChange()
NotifyPaintFrame();
}

void Renderer::TriggerNewTextNotification(const std::wstring_view newText)
{
FOREACH_ENGINE(pEngine)
{
LOG_IF_FAILED(pEngine->NotifyNewText(newText));
}
}

// Routine Description:
// - Update the title for a particular engine.
// Arguments:
Expand Down
2 changes: 2 additions & 0 deletions src/renderer/base/renderer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ namespace Microsoft::Console::Render
void TriggerCircling() override;
void TriggerTitleChange() override;

void TriggerNewTextNotification(const std::wstring_view newText) override;

void TriggerFontChange(const int iDpi,
const FontInfoDesired& FontInfoDesired,
_Out_ FontInfo& FontInfo);
Expand Down
1 change: 1 addition & 0 deletions src/renderer/inc/DummyRenderTarget.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@ class DummyRenderTarget final : public Microsoft::Console::Render::IRenderTarget
void TriggerScroll(const COORD* const /*pcoordDelta*/) override {}
void TriggerCircling() override {}
void TriggerTitleChange() override {}
void TriggerNewTextNotification(const std::wstring_view) override {}
};
1 change: 1 addition & 0 deletions src/renderer/inc/IRenderEngine.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ namespace Microsoft::Console::Render
[[nodiscard]] virtual HRESULT InvalidateAll() noexcept = 0;
[[nodiscard]] virtual HRESULT InvalidateCircling(_Out_ bool* pForcePaint) noexcept = 0;
[[nodiscard]] virtual HRESULT InvalidateTitle(std::wstring_view proposedTitle) noexcept = 0;
[[nodiscard]] virtual HRESULT NotifyNewText(const std::wstring_view newText) noexcept = 0;
[[nodiscard]] virtual HRESULT PrepareRenderInfo(const RenderFrameInfo& info) noexcept = 0;
[[nodiscard]] virtual HRESULT ResetLineTransform() noexcept = 0;
[[nodiscard]] virtual HRESULT PrepareLineTransform(LineRendition lineRendition, size_t targetRow, size_t viewportLeft) noexcept = 0;
Expand Down
2 changes: 2 additions & 0 deletions src/renderer/inc/IRenderTarget.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ namespace Microsoft::Console::Render
virtual void TriggerScroll(const COORD* const pcoordDelta) = 0;
virtual void TriggerCircling() = 0;
virtual void TriggerTitleChange() = 0;

virtual void TriggerNewTextNotification(const std::wstring_view newText) = 0;
};

inline Microsoft::Console::Render::IRenderTarget::~IRenderTarget() {}
Expand Down
2 changes: 2 additions & 0 deletions src/renderer/inc/RenderEngineBase.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ namespace Microsoft::Console::Render

[[nodiscard]] HRESULT UpdateTitle(const std::wstring_view newTitle) noexcept override;

[[nodiscard]] HRESULT NotifyNewText(const std::wstring_view newText) noexcept override;

[[nodiscard]] HRESULT UpdateSoftFont(const gsl::span<const uint16_t> bitPattern,
const SIZE cellSize,
const size_t centeringHint) noexcept override;
Expand Down
Loading