MVP of Accessibility/UI automation support #5177
Conversation
|
@jkoritzinsky in #585 it sounded like you had some ideas for this, and sounds like it's not really along the lines of what you were thinking (instead it's loosely based on WPF's automation system). Is this the right direction do you think? |
|
To sum up my initial testing - I was able to use https://github.com/microsoft/WinAppDriver to interact with ControlCatalog running on top of this branch. I was able to navigate between tab pages and click buttons. I was mostly using names and type names to find things. I can see that having support for automation id will be useful: https://docs.microsoft.com/en-us/uwp/api/windows.ui.xaml.automation.automationproperties.automationidproperty?view=winrt-19041 |
|
@MarchingCube |
|
@grokys Right, I didn't realize that it is an attached property. I will try testing again sometime soon. |
grokys
force-pushed
the
feature/ui-automation
branch
2 times, most recently
from
5bef7e3
to
2dbccc0
Compare
|
As I keep working on this the more I hate the WPF/UWP The more I look at the OSX NSAccessibility API the more I feel jealous. Compare the workflow in WPF and OSX to make a control an accessible button. WPF:
OSX:
Does anyone have any experience of these APIs? Is there an advantage to the WPF/UWP way of doing it? |
Follows WPF/UWP API as closely as possible. Limited to win32 right now. Broken in many places.
grokys
force-pushed
the
feature/ui-automation
branch
from
17887bb
to
2a44d8b
Compare
|
Ok, due to my inexperience with these APIs and the general discussion over at #585 I've decided to start by modelling the WPF API as closely as possible. From there we'll have to adjust it as we implement support for other platforms. |
And fixed some issues with ComboBox/popups.
And start adding integration tests for menu items.
Seems to be what best fits, and what e.g. chrome uses.
And fixed NRE in MenuItemAutomationPeer.
Causing the build to fail on OSX/Linux
This will be needed for OSX.
There was a problem hiding this comment.
I think the UIA API from WPF/UWP are good and I think the API you have so far (besides my remarks/comments) is good. One thing you could change is that the AutomationControlType could also be set from XAML. Some peers only seem to exist to override this peer, this might not be ideal. In contrast, the Web A11y API (aria) only works using HTML properties, so it might be worth exploring moving things that don't need to be done from code behind into XAML (e.g. AutomationControlType).
|
|
||
| namespace Avalonia.Automation.Peers | ||
| { | ||
| public enum AutomationControlType |
There was a problem hiding this comment.
I would double check this list with the ARIA roles as those is a minimum supported on most platforms: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles
There was a problem hiding this comment.
Yeah this list comes from the WPF API. Should we just replace this list with values from the ARIA roles if that's the common subset? Should we keep the WPF/win32 naming or use the ARIA naming? Perhaps that latter would make more sense from an x-plat point of view.
There was a problem hiding this comment.
I think sticking with the WPF/Win23/UWP naming is easier since folks familiar with XAML are more familiar with those names. Developers coming from the Web will have to learn XAML first anyway, having some web naming in here might seem weird then.
| Node.PropertyChanged(automationProperty, oldValue, newValue); | ||
| } | ||
|
|
||
| protected virtual string GetLocalizedControlTypeCore() |
There was a problem hiding this comment.
Is this not being localized intentional?
There was a problem hiding this comment.
Just not done yet. Would it be best to use satellite assembles for localizing the names, or just hardcode the names for all languages in code/resources in the main assembly?
There was a problem hiding this comment.
Probably hardcoding is fine? Not sure how localization works with Avalonia so whatever is the best way of doing it in Avalonia might be best here. But as a developer I would expect this assembly to be able to return translated control types.
|
|
||
| namespace Avalonia.Automation | ||
| { | ||
| public static class AutomationProperties |
There was a problem hiding this comment.
A HeadingLevel property is missing, however it makes scanning documents a lot easier for people using assistive technology. Also, a LandMarkType property is missing but is very important for people with assistive technology.
There was a problem hiding this comment.
This was ported from WPF which doesn't have HeadingLevel/LandmarkType . I can add those.
There was a problem hiding this comment.
Those properties are present in UWP and the web (as semantic html). Honestly, I'm a bit surprised that WPF didn't have those.
There was a problem hiding this comment.
So should we add the HeadingLevel or LandMarkType properties? When using narrator on Windows, those are definitely helping people navigate UWP/WinUI apps more easily and those properties are also present in web accessibility.
There was a problem hiding this comment.
Yeah definitely, as I mention in the PR description though, this PR is already huge so stuff like this will be implemented later.
|
|
||
| protected override AutomationControlType GetAutomationControlTypeCore() | ||
| { | ||
| return AutomationControlType.Group; |
There was a problem hiding this comment.
This does not sound right, "None" content is a group? How about just removing it from UIA tree then instead of saying it is a group?
There was a problem hiding this comment.
The problem with completely removing them from the OS-level automation tree is that it breaks the AutomationPeer -> OS automation node 1:1 mapping and means that we have to have another level of abstraction to handle parent/child relationships in the case where peers don't map directly to OS automation nodes. We've already got 3 trees to keep in sync so at least if the nodes in the "peer" tree match 1:1 with the nodes in the OS tree it reduces some complexity.
Win32 and OSX provide a facility for "hiding" controls at the automation node level (UIA_IsControlElementPropertyId/accessibilityElement), which looked to me to be sufficient?
Maybe instead of using Group we should have a new AutomationControlType.None type which will be mapped to the most relevant type for "ignored" elements at the OS level?
There was a problem hiding this comment.
Ah I see. In that case, group is fine, having a "none" control type would be really cool, however I'm not sure how much of a benefit there would be.
| @@ -17,6 +18,7 @@ public partial class WindowImpl | |||
| protected virtual unsafe IntPtr AppWndProc(IntPtr hWnd, uint msg, IntPtr wParam, IntPtr lParam) | |||
| { | |||
| const double wheelDelta = 120.0; | |||
| const long UiaRootObjectId = -25; | |||
There was a problem hiding this comment.
This is from the Win32 API: https://github.com/tpn/winsdk-10/blob/master/Include/10.0.16299.0/um/UIAutomationCoreApi.h#L52
There was a problem hiding this comment.
I see, good to know. Could add a comment for this, but I think it's fine as is.
tests/Avalonia.Controls.UnitTests/Automation/ControlAutomationPeerTests.cs
Show resolved
Hide resolved
|
Thanks so much for the review @chingucoding - as I've mentioned I'm new to this stuff so getting some input from someone with experience in this area is invaluable.
Sounds like a good idea - so something like
Yep, peers definitely do seem heavyweight from my point of view compared to HTML area properties! Any other ideas for things we could do directly from XAML? |
|
In addition, I have an initial implementation for OSX that I need to merge into this PR. Are you familiar with a11y on OSX at all? |
Previous change showed up a problem where incorrect interface was getting called for `IsReadOnly`.
|
You can test this PR using the following package version. |
grokys
force-pushed
the
feature/ui-automation
branch
from
51b3f5b
to
bff4616
Compare
`PlatformImpl` may be a `ValidatingTopLevelImpl`.
And remove now-unneeded `#nullable enable` statements.
|
You can test this PR using the following package version. |
1 similar comment
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
1 similar comment
|
You can test this PR using the following package version. |
|
is usage of ATK instead of AT-SPI a viable option for adding linux accessibility? might be easier... or not |
|
You can test this PR using the following package version. |
1 similar comment
|
You can test this PR using the following package version. |
|
Great PR! and I know it's already merged. Just wanted to voice my concern about putting all automation peers in a directory separate from the control implementations. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
What does the pull request do?
Adds a Minimum Viable Implementation of Accessibility/UI automation to Avalonia along with the start of some integration tests using Appium.
There is still a lot missing here, but the PR is already huge so I'd like to propose getting what's here merged and improving it in smaller PRs.
The API tries to stick as closely to the WPF/UWP API as possible, while adding a few improvements.
Platforms
Providers
IWindowProviderin WPF)Controls
Integration Tests
Integration tests are currently using Appium in order to only need to write one set for Windows/MacOS. However Appium is very buggy on Windows/MacOS and also quite limited:
Maybe that we need to write platform-specific integration tests in the long run, but for now lets see how far Appium can take us.
Fixed issues
Fixes #585