Accessibility: Set-up UIA Tree #1691

carlos-zamora · 2019-06-28T01:55:21Z

Summary of the Pull Request

I touched A LOT of files. The main reason for this is that the WindowUiaProvider was moved to Microsoft::Console::Types so that it can be accessed by both projects. This meant we had to make a lot of propagating changes. But more info on that later.

The Basics of Accessibility

What is a User Interaction Automation (UIA) Tree?
Other projects (i.e.: Narrator) can take advantage of this UIA tree and are used to present information within it.
Some things like XAML already have a UIA Tree. So some UIA tree navigation and features are already there. It's just a matter of getting them hooked up and looking right.

Accessibility in our Project
There's a few important classes...
regarding Accessibility...

WindowUiaProvider: This sets up the UIA tree for a window. So this is the top-level for the UIA tree.
ScreenInfoUiaProvider: This sets up the UIA tree for a terminal buffer.
UiaTextRange: This is essential to interacting with the UIA tree for the terminal buffer. Actually gets portions of the buffer and presents them.

regarding the Windows Terminal window...

BaseWindow: The foundation to a window. Deals with HWNDs and that kind of stuff.
IslandWindow: This extends BaseWindow and is actually what holds our Windows Terminal
NonClientIslandWindow: An extension of the IslandWindow

regarding ConHost...

IConsoleWindow: This is an interface for the console window.
Window: This is the actual window for ConHost. Extends IConsoleWindow

References

PR Checklist

Closes Windows Terminal: Set up UIA tree and hook it up properly #634
CLA signed. If not, go over here and sign the CLA
Tests added/passed
Requires documentation to be updated

Detailed Description of the Pull Request / Additional comments

IConsoleWindow changes:
- move into Microsoft::Console::Types (a shared space)
- Have IslandWindow extend it
WindowUiaProvider changes:
- move into Microsoft::Console::Types (a shared space)
Hook up WindowUiaProvider to IslandWindow (yay! we now have a tree)

Validation Steps Performed

I used "inspect.exe" to verify that the tree is hooked up. Verified this on ConHost and Windows Terminal.

Comments after merging in PR #1915

Please refer to #1915 for a large chunk of the work done here. There are also a few known issues/TODOs that carry over:

ConHost: the UIA Rects for the caption buttons are missing. Still investigating this issue.
Issue Task: Re-attach Tracing to UIA Tree #1914 - Tracing needs to be reattached
Issue Fundamental Accessibility: Modify conhost accessibility model #1352 - Windows Terminal: the ScreenInfoUiaProvider needs to be attached to the TermControl.
Task: Explore de-static-fying UiaTextRange functions #1993
Task: Refactor IRenderData and create IAccessibilityData #1992

carlos-zamora · 2019-06-28T15:49:07Z

This might also close #1351 ? We can access the tab control through the UIA tree. But I think there will need to be some polishing afterwards.

adiviness · 2019-06-28T18:23:27Z

src/types/IConsoleWindow.hpp

@@ -18,7 +18,7 @@ Author(s):
 // copied typedef from uiautomationcore.h


this file should be moved back to where it was. that'll also eliminate a bunch of the changes in the PR.

(Just writing what we discussed in person):

IConsoleWindow, WindowUiaProvider, and more files to come will be moved into this shared space

They will be moved into a new shared LIB (name TBD) to keep Types independent of dependencies

Upcoming changes to ScreenInfoUiaProvider include:

Moving it to a shared space (same LIB as above)

Adding some pure virtual functions to abstract out the ServiceLocator and other things we don't need from ConHost

zadjii-msft

I have minor nits, and one big question on IConsoleWindow

src/interactivity/win32/windowproc.cpp

src/host/tracing.cpp

src/types/WindowUiaProvider.cpp

src/cascadia/WindowsTerminal/IslandWindow.h

- Moved WindowUiaProvider to Types (Shared) - Hooked up WindowUiaProvider to BaseWindow (WT) UIATree is now accessible in WT

…g used. TODO: - replace IWindow with IConsoleWindow (for my own sanity) - turn SIUP into an abstract class where any use of ServiceLocator and Selection is abstracted out

- Made IslandWindow inherit IConsoleWindow - Moved some UIATree logic to IslandWindow - Fixed ConHost to work with UIATree (sorta) At the moment, ConHost is using a temp ScreenInfoUIAProvider. This will be fixed with next inheritance patch. TODO: - turn SIUP into an abstract class where any use of ServiceLocator and Selection is abstracted out

Added correct strings to WindowUiaProvider description fields

zadjii-msft

I feel better about this. I have some nits, and I'm still annoyed by the methods added to IRenderData, but it's not worth blocking this any longer on that. We can follow up to add another interface later.
I'm thinking IUiaData : IRenderData, which just adds the methods you've added here.

src/renderer/inc/IRenderData.hpp

zadjii-msft · 2019-07-25T16:34:38Z

src/types/ScreenInfoUiaProvider.cpp

+
+void ScreenInfoUiaProvider::_LockConsole() noexcept
+{
+    _pData->LockConsole();


small danger:
::LockConsole() != gci.LockConsole(). The gci one will dispatch control events (ctrl+C) in unlock, while the global one won't.

RenderData calls the global one, while this used to call the gci one.

This might be okay, since I don't think people are going to be writing Ctrl+C (as an interrupt) to the console via the UIA provider, but if you were for some reason, be aware it might not actually get dispatched until a gci.Unlock().

So, to fix this in the long run, does that mean we need to change how Lock/Unlock works in the Terminal class?

The whole "dispatching events on unlock" thing needs to be revisited completely at some point. I just filed #2141 for that.

src/interactivity/win32/window.cpp

DHowett-MSFT · 2019-07-26T19:31:13Z

Carlos, when you merge this, please make sure you manually copy the body of teh pull request description (preferably in raw markdown) into the body of the commit. It's a great description. Otherwise, github will take your individual commit messages and concatenate them

src/cascadia/WindowsTerminal/BaseWindow.h

DHowett-MSFT · 2019-07-26T19:37:54Z

src/cascadia/WindowsTerminal/IslandWindow.h

+    [[nodiscard]] HRESULT SignalUia(_In_ EVENTID id) override { return E_NOTIMPL; };
+    [[nodiscard]] HRESULT UiaSetTextAreaFocus() override { return E_NOTIMPL; };
+
+    RECT GetWindowRect() const noexcept override


why override to just call parent?

is this because IUIAWindowNeedsIt?

Yup. BaseWindow AND IUiaWindow both have GetWindowRect() and GetWindowHandle() (BaseWindow just calls it GetHandle()). When I remove these from here, I get "ambiguous access of 'GetWindowRect'" in NonClientIslandWindow (inherits from IslandWindow). Is there a C++-ism that you know about to get around this?

DHowett-MSFT · 2019-07-26T19:38:43Z

src/cascadia/WindowsTerminal/WindowUiaProvider.cpp

+    }
+    catch (...)
+    {
+        if (nullptr != pWindowProvider)


okay but 100% we absolutely have to use real and correct lifetime tracking types for this

This part was copied over from the ConHost code when I created the inheritance model with WindowUiaProviderBase. Since we've been talking about switching over to wil in this area and the WindowUiaProvider is going to be important for changing the navigation of the tree (one of the upcoming tasks), could I just add a work item here and call it a day?

just so long as we do get to it.

src/cascadia/WindowsTerminal/WindowUiaProvider.cpp

src/host/tracing.cpp

src/interactivity/win32/WindowMetrics.cpp

Used #if 0 for tracing Made GetWindowRect() noexcept to align better between ConHost and WindowsTerminal

src/cascadia/WindowsTerminal/WindowUiaProvider.cpp

src/interactivity/win32/windowUiaProvider.cpp

miniksa · 2019-07-29T19:57:11Z

src/types/ScreenInfoUiaProvider.cpp

+
+void ScreenInfoUiaProvider::_LockConsole() noexcept
+{
+    _pData->LockConsole();


The whole "dispatching events on unlock" thing needs to be revisited completely at some point. I just filed #2141 for that.

Builds on the work of #1691 and #1915 Let's start with the easy change: - `TermControl`'s `controlRoot` was removed. `TermControl` is a `UserControl` now. Ok. Now we've got a story to tell here.... ### TermControlAP - the Automation Peer Here's an in-depth guide on custom automation peers: https://docs.microsoft.com/en-us/windows/uwp/design/accessibility/custom-automation-peers We have a custom XAML element (TermControl). So XAML can't really hold our hands and determine an accessible behavior for us. So this automation peer is responsible for enabling that interaction. We made it a FrameworkElementAutomationPeer to get as much accessibility as possible from it just being a XAML element (i.e.: where are we on the screen? what are my dimensions?). This is recommended. Any functions with "Core" at the end, are overwritten here to tweak this automation peer into what we really need. But what kind of interactions can a user expect from this XAML element? Introducing ControlPatterns! There's a ton of interfaces that just define "what can I do". Thankfully, we already know that we're supposed to be `ScreenInfoUiaProvider` and that was an `ITextProvider`, so let's just make the TermControlAP an `ITextProvider` too. So now we have a way to define what accessible actions can be performed on us, but what should those actions do? Well let's just use the automation providers from ConHost that are now in a shared space! (Note: this is a great place to stop and get some coffee. We're about to hop into the .cpp file in the next section) ### Wrapping our shared Automation Providers Unfortunately, we can't just use the automation providers from ConHost. Or, at least not just hook them up as easily as we wish. ConHost's UIA Providers were written using UIAutomationCore and ITextRangeProiuder. XAML's interfaces ITextProvider and ITextRangeProvider are lined up to be exactly the same. So we need to wrap our ConHost UIA Providers (UIAutomationCore) with the XAML ones. We had two providers, so that means we have two wrappers. #### TermControlAP (XAML) <----> ScreenInfoUiaProvider (UIAutomationCore) Each of the functions in the pragma region `ITextProvider` for TermControlAP.cpp is just wrapping what we do in `ScreenInfoUiaProvider`, and returning an acceptable version of it. Most of `ScreenInfoUiaProvider`'s functions return `UiaTextRange`s. So we need to wrap that too. That's this next section... #### XamlUiaTextRange (XAML) <----> UiaTextRange (UIAutomationCore) Same idea. We're wrapping everything that we could do with `UiaTextRange` and putting it inside of `XamlUiaTextRange`. ### Additional changes to `UiaTextRange` and `ScreenInfoUiaProvider` If you don't know what I just said, please read this background: - #1691: how accessibility works and the general responsibility of these two classes - #1915: how we pulled these Accessibility Providers into a shared area TL;DR: `ScreenInfoUiaProvider` lets you interact with the displayed text. `UiaTextRange` is specific ranges of text in the display and navigate the text. Thankfully, we didn't do many changes here. I feel like some of it is hacked together but now that we have a somewhat working system, making changes shouldn't be too hard...I hope. #### UiaTextRange We don't have access to the window handle. We really only need it to draw the bounding rects using WinUser's `ScreenToClient()` and `ClientToScreen()`. I need to figure out how to get around this. In the meantime, I made the window handle optional. And if we don't have one....well, we need to figure that out. But other than that, we have a `UiaTextRange`. #### ScreenInfoUiaProvider At some point, we need to hook up this automation provider to the WindowUiaProvider. This should help with navigation of the UIA Tree and make everything just look waaaay better. For now, let's just do the same approach and make the pUiaParent optional. This one's the one I'm not that proud of, but it works. We need the parent to get a bounding rect of the terminal. While we figure out how to attach the WindowUiaProvider, we should at the very least be able to get a bunch of info from our xaml automation peer. So, I've added a _getBoundingRect optional function. This is what's called when we don't have a WindowUiaProvider as our parent. ## Validation Steps Performed I've been using inspect.exe to see the UIA tree. I was able to interact with the terminal mostly fine. A few known issues below. Unfortunately, I tried running Narrator on this and it didn't seem to like it (by that I mean WT crashed). Then again, I don't really know how to use narrator other than "click on object" --> "listen voice". I feel like there's a way to get the other interactions with narrator, but I'll be looking into more of that soon. I bet if I fix the two issues below, Narrator will be happy. ## Miscellaneous Known Issues - `GetSelection()` and `GetVisibleRanges()` crashes. I need to debug through these. I want to include them in this PR. Fixes #1353.

ghost · 2019-08-03T01:49:42Z

🎉Windows Terminal Preview v0.3.2142.0 has been released which incorporates this pull request.:tada:

Handy links:

carlos-zamora self-assigned this Jun 28, 2019

carlos-zamora requested a review from adiviness June 28, 2019 01:57

carlos-zamora force-pushed the dev/cazamor/accessibility branch from d87ba29 to 3457910 Compare June 28, 2019 18:05

adiviness suggested changes Jun 28, 2019

View reviewed changes

ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Jun 28, 2019

This was referenced Jul 10, 2019

Task: Re-attach Tracing to UIA Tree #1914

Closed

Accessibility: Finalized Shared UIA Tree Model #1915

Merged

zadjii-msft reviewed Jul 16, 2019

View reviewed changes

src/interactivity/win32/windowproc.cpp Outdated Show resolved Hide resolved

src/host/tracing.cpp Show resolved Hide resolved

src/types/WindowUiaProvider.cpp Outdated Show resolved Hide resolved

src/cascadia/WindowsTerminal/IslandWindow.h Outdated Show resolved Hide resolved

carlos-zamora force-pushed the dev/cazamor/accessibility branch from df0ae03 to b618509 Compare July 17, 2019 01:51

carlos-zamora marked this pull request as ready for review July 17, 2019 01:51

carlos-zamora force-pushed the dev/cazamor/accessibility branch from b5d81b3 to 4ecdcce Compare July 23, 2019 16:37

carlos-zamora and others added 8 commits July 24, 2019 16:01

- Created IWindow as a common interface to IConsoleWindow and BaseWindow

2be5ff2

- Moved WindowUiaProvider to Types (Shared) - Hooked up WindowUiaProvider to BaseWindow (WT) UIATree is now accessible in WT

ConHost is building again. Wrong ScreenInfoUiaProvider (SIUP) is bein…

4c6faec

…g used. TODO: - replace IWindow with IConsoleWindow (for my own sanity) - turn SIUP into an abstract class where any use of ServiceLocator and Selection is abstracted out

Fixed rebase onto master

65dc3ff

CODE FORMATTING

8290f78

Accessibility: Finalized Shared UIA Tree Model (#1915)

7417a9a

included some "using Microsoft::Console::Types" to make code prettier

867412a

Added correct strings to WindowUiaProvider description fields

Changed IslandWindow from IConsoleWindow --> IUiaWindow

a815b42

carlos-zamora force-pushed the dev/cazamor/accessibility branch from 4ecdcce to a815b42 Compare July 24, 2019 23:12

carlos-zamora mentioned this pull request Jul 25, 2019

Accessibility: TermControl Automation Peer #2083

Merged

zadjii-msft approved these changes Jul 25, 2019

View reviewed changes

Moar PR changes

3eb9947

DHowett-MSFT reviewed Jul 26, 2019

View reviewed changes

carlos-zamora added 2 commits July 29, 2019 11:12

PR Changes

fc1ad7d

Used #if 0 for tracing Made GetWindowRect() noexcept to align better between ConHost and WindowsTerminal

code formatting

a158e61

miniksa approved these changes Jul 29, 2019

View reviewed changes

carlos-zamora added 2 commits July 29, 2019 13:43

remove unneccessary comments

dd1b503

I derped on the commit of removing comments

b6eed7a

carlos-zamora merged commit 96496d8 into master Jul 29, 2019

carlos-zamora deleted the dev/cazamor/accessibility branch July 31, 2019 16:59

ghost mentioned this pull request Aug 3, 2019

Windows Terminal: Set up UIA tree and hook it up properly #634

Closed

carlos-zamora mentioned this pull request Aug 16, 2019

Revisit TerminalSelection to add tests, clean up, proper selection mechanics #1327

Closed

4 tasks

zadjii-msft mentioned this pull request Mar 8, 2022

Bug Report: ConHost Caption BoundingRects dissapeared #2176

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Accessibility: Set-up UIA Tree #1691

Accessibility: Set-up UIA Tree #1691

carlos-zamora commented Jun 28, 2019 •

edited

Loading

carlos-zamora commented Jun 28, 2019

adiviness Jun 28, 2019

carlos-zamora Jun 28, 2019

zadjii-msft left a comment

zadjii-msft left a comment

zadjii-msft Jul 25, 2019

carlos-zamora Jul 26, 2019

miniksa Jul 29, 2019

DHowett-MSFT commented Jul 26, 2019

DHowett-MSFT Jul 26, 2019

DHowett-MSFT Jul 26, 2019

carlos-zamora Jul 29, 2019

DHowett-MSFT Jul 26, 2019

carlos-zamora Jul 29, 2019

DHowett-MSFT Jul 29, 2019

miniksa Jul 29, 2019

ghost commented Aug 3, 2019

		@@ -18,7 +18,7 @@ Author(s):
		// copied typedef from uiautomationcore.h

Accessibility: Set-up UIA Tree #1691

Accessibility: Set-up UIA Tree #1691

Conversation

carlos-zamora commented Jun 28, 2019 • edited Loading

Summary of the Pull Request

References

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Comments after merging in PR #1915

carlos-zamora commented Jun 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

zadjii-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DHowett-MSFT commented Jul 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Aug 3, 2019

carlos-zamora commented Jun 28, 2019 •

edited

Loading