Preliminary implementation of UIA for A11y on Windows #37754

yaakovschectman · 2022-11-18T18:59:44Z

We are now able to incorporate the implementation for UI Automation for accessibility in addition to/in place of MSAA. By default, the UIA object will not be returned to a screen reader in the WM_GETOBJECT message. For this, the engine must be built with the FLUTTER_ENGINE_USE_UIA macro defined.

In addition to the change to the window message handler, the Flutter platform node delegate has been modified to return its HWND in GetTargetForNativeAccessibilityEvent so that the root of the element can be obtained for IRawElementProviderFragment::get_FragmentRoot.
A typo in AXActivePopup has been fixed.
Accessibility event dispatch procedure on Windows has been modified to defer to AXPlatformNodeWin::NotifyAccessibilityEvent so that either MSAA and/or UIA events may be fired in response.
Unit tests have been added/modified/re-enabled.

We will still need to implement ITextProvider and ITextRangeProvider for their appropriate UIA patterns in order to have text edit carat navigation narrated correctly.

Part of flutter/flutter#114547

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

shell/platform/windows/window.cc

loic-sharma · 2022-11-21T23:09:38Z

shell/platform/windows/window.cc

+    // Retrieve UIA object for the root view.
+    Microsoft::WRL::ComPtr<IRawElementProviderSimple> root;
+    if (SUCCEEDED(ax_fragment_root_->GetNativeViewAccessible()->QueryInterface(
+            IID_PPV_ARGS(&root)))) {


I see that Chrome doesn't have this SUCCEEDED check. Just curious, why did you add this? Feel free to keep as is, just asking for knowledge :)

I do not know for sure why they do not use the check, but QueryInterface can return failure

Can we not guarantee that QueryInterface will succeed here since we entirely own the logic?

I don't know how accurate it is to say we entirely own the logic. ATL provides a default QueryInterface which calls InternalQueryInterface, which we own, which then calls, CComObjectRootBase::InternalQueryInterface, which is also a provided ATL implementation.

shell/platform/windows/window.cc

shell/platform/windows/window.h

skia-gold · 2022-11-28T16:15:30Z

Gold has detected about 1 new digest(s) on patchset 16.
View them at https://flutter-engine-gold.skia.org/cl/github/37754

cbracken · 2022-12-06T18:54:14Z

shell/platform/windows/windowsx_shim.h

+// common function names. To work around that, windowsx.h is not to be
+// included directly, and instead this file should be included. If one
+// of the removed Win32 macros is wanted, use the expanded form
+// manually instead.


Thank you for cleaning this up. Much nicer.

shell/platform/windows/window.h

loic-sharma

LGTM, nice work!

cbracken · 2022-12-06T20:11:14Z

shell/platform/windows/window_binding_handler_delegate.h

@@ -135,6 +136,9 @@ class WindowBindingHandlerDelegate {

  // Update the status of the high contrast feature
  virtual void UpdateHighContrastEnabled(bool enabled) = 0;
+
+  // Obtain a pointer to the fragment root delegate.


It may be useful to add a little detail on how this is used by UIA, since it's not something that was required in MSAA.

cbracken · 2022-12-06T20:14:14Z

third_party/accessibility/ax/platform/ax_platform_node_win.cc

@@ -5205,6 +5218,10 @@ std::optional<DWORD> AXPlatformNodeWin::MojoEventToMSAAEvent(
      return EVENT_OBJECT_SHOW;
    case ax::mojom::Event::kValueChanged:
      return EVENT_OBJECT_VALUECHANGE;
+#ifdef EVENT_OBJECT_TEXTSELECTIONCHANGED


I'm assuming this was added in Win 8 or 10 maybe? Consider adding a doc comment pointing out why it's ifdefed.

Actually on second look we probably do not need it. This macro should be defined from Windows Server 2008 onwards.

cbracken · 2022-12-06T20:16:19Z

third_party/accessibility/ax/platform/test_ax_node_wrapper.cc

@@ -54,9 +54,14 @@ std::map<AXNode::AXID, AXNode::AXID> g_hit_test_result;
 class TestAXTreeObserver : public AXTreeObserver {
 private:
  void OnNodeDeleted(AXTree* tree, int32_t node_id) override {
-    const auto iter = g_node_id_to_wrapper_map.find(node_id);
+    const auto& iter = g_node_id_to_wrapper_map.find(node_id);


cbracken

…ter/engine#37754)

…116802) * bd8bcf956 Roll Fuchsia Mac SDK from crEcyXdyZ686cAqMV... to pMV6A0ykZQ8aA3NG2... (flutter/engine#38120) * dec8b5221 Preliminary implementation of UIA for A11y on Windows (flutter/engine#37754) * 5545ccf87 Roll Fuchsia Linux SDK from NlJGkMbtZqQ6_BCpu... to xn8ztWtp-zww-jObz... (flutter/engine#38122) * 80a15a419 Create FlutterActivity/FlutterFragment using light weight engine with FlutterEngineGroup (flutter/engine#36963) * 5caef8585 Full implementation of text-input-test (flutter/engine#37986) * 8f6036e58 Reland fix wrong VSYNC event (flutter/engine#37865) * 4101c363c [iOS] Change locale format for spell check (flutter/engine#38080) * 2f5b67e4d [embedder] Ensure destruction called on present (flutter/engine#38078) * 0bddc6045 [Impeller Scene] Depth attachment; baked lighting example (flutter/engine#38118) * 6aa4ccd60 Remove dlCanvasRecorder from flutter::PictureRecorder (flutter/engine#38127) * dbb5284f2 [Windows] Add more cursor plugin tests (flutter/engine#38112) * 6e91204d9 Roll Fuchsia Mac SDK from pMV6A0ykZQ8aA3NG2... to 9SnrQ0vbR8IC7UIoP... (flutter/engine#38135) * 3140ad924 [Impeller] order metal samplers according to declared order and not usage order (flutter/engine#38115) * 84abf21d4 Remove autoninja. (flutter/engine#38136) * 8a113d328 [embedder] Expose metal surface from test context (flutter/engine#38133) * 1ef25b63f Roll Fuchsia Mac SDK from 9SnrQ0vbR8IC7UIoP... to aMW0DjntzFJj4RoR3... (flutter/engine#38139) * 748b3bc15 Revert "Remove dlCanvasRecorder from flutter::PictureRecorder (#38127)" (flutter/engine#38137) * b6daf3d06 [embedder] Consistent naming for GL/Metal tests (flutter/engine#38141) * 339d04baf [web] Trivial fix for non-static interop JS interop class. (flutter/engine#38126) * 1fcbb9c11 [tools] Eliminate version on Obj-C docs (flutter/engine#38145) * 71928b6a4 [Impeller] Use DrawPath instead of Rect geometry when the paint style is stroke (flutter/engine#38146) * 23ce8fdbc Roll Skia from dd3285a80b23 to f84dc9303045 (4 revisions) (flutter/engine#38123) * 366f8663b Roll Skia from f84dc9303045 to 2691cd7b4110 (40 revisions) (flutter/engine#38151) * 447e7013e Roll Skia from 2691cd7b4110 to 711396b81248 (1 revision) (flutter/engine#38152) * cd5d91bf9 Pylint testing/run_tests.py (flutter/engine#38016) * aafac083b Roll Skia from 711396b81248 to b253b10374e7 (7 revisions) (flutter/engine#38157) * 799dc78e8 Roll Fuchsia Linux SDK from xn8ztWtp-zww-jObz... to rRJIjuO-dPNCpCTd9... (flutter/engine#38134) * 3aa3d2a8f Massage the JS interop around `didCreateEngineInitializer` (flutter/engine#38147) * 030950f30 Roll Skia from b253b10374e7 to ec407902999b (3 revisions) (flutter/engine#38158)

…lutter#116802) * bd8bcf956 Roll Fuchsia Mac SDK from crEcyXdyZ686cAqMV... to pMV6A0ykZQ8aA3NG2... (flutter/engine#38120) * dec8b5221 Preliminary implementation of UIA for A11y on Windows (flutter/engine#37754) * 5545ccf87 Roll Fuchsia Linux SDK from NlJGkMbtZqQ6_BCpu... to xn8ztWtp-zww-jObz... (flutter/engine#38122) * 80a15a419 Create FlutterActivity/FlutterFragment using light weight engine with FlutterEngineGroup (flutter/engine#36963) * 5caef8585 Full implementation of text-input-test (flutter/engine#37986) * 8f6036e58 Reland fix wrong VSYNC event (flutter/engine#37865) * 4101c363c [iOS] Change locale format for spell check (flutter/engine#38080) * 2f5b67e4d [embedder] Ensure destruction called on present (flutter/engine#38078) * 0bddc6045 [Impeller Scene] Depth attachment; baked lighting example (flutter/engine#38118) * 6aa4ccd60 Remove dlCanvasRecorder from flutter::PictureRecorder (flutter/engine#38127) * dbb5284f2 [Windows] Add more cursor plugin tests (flutter/engine#38112) * 6e91204d9 Roll Fuchsia Mac SDK from pMV6A0ykZQ8aA3NG2... to 9SnrQ0vbR8IC7UIoP... (flutter/engine#38135) * 3140ad924 [Impeller] order metal samplers according to declared order and not usage order (flutter/engine#38115) * 84abf21d4 Remove autoninja. (flutter/engine#38136) * 8a113d328 [embedder] Expose metal surface from test context (flutter/engine#38133) * 1ef25b63f Roll Fuchsia Mac SDK from 9SnrQ0vbR8IC7UIoP... to aMW0DjntzFJj4RoR3... (flutter/engine#38139) * 748b3bc15 Revert "Remove dlCanvasRecorder from flutter::PictureRecorder (flutter#38127)" (flutter/engine#38137) * b6daf3d06 [embedder] Consistent naming for GL/Metal tests (flutter/engine#38141) * 339d04baf [web] Trivial fix for non-static interop JS interop class. (flutter/engine#38126) * 1fcbb9c11 [tools] Eliminate version on Obj-C docs (flutter/engine#38145) * 71928b6a4 [Impeller] Use DrawPath instead of Rect geometry when the paint style is stroke (flutter/engine#38146) * 23ce8fdbc Roll Skia from dd3285a80b23 to f84dc9303045 (4 revisions) (flutter/engine#38123) * 366f8663b Roll Skia from f84dc9303045 to 2691cd7b4110 (40 revisions) (flutter/engine#38151) * 447e7013e Roll Skia from 2691cd7b4110 to 711396b81248 (1 revision) (flutter/engine#38152) * cd5d91bf9 Pylint testing/run_tests.py (flutter/engine#38016) * aafac083b Roll Skia from 711396b81248 to b253b10374e7 (7 revisions) (flutter/engine#38157) * 799dc78e8 Roll Fuchsia Linux SDK from xn8ztWtp-zww-jObz... to rRJIjuO-dPNCpCTd9... (flutter/engine#38134) * 3aa3d2a8f Massage the JS interop around `didCreateEngineInitializer` (flutter/engine#38147) * 030950f30 Roll Skia from b253b10374e7 to ec407902999b (3 revisions) (flutter/engine#38158)

yaakovschectman added accessibility affects: desktop platform-windows labels Nov 18, 2022

yaakovschectman requested review from cbracken, loic-sharma, moffatman, dkwingsmt and a-wallen November 18, 2022 18:59

yaakovschectman self-assigned this Nov 18, 2022

yaakovschectman force-pushed the uia branch 2 times, most recently from 7456610 to 2176a76 Compare November 21, 2022 16:49

loic-sharma requested a review from chunhtai November 21, 2022 22:53

loic-sharma reviewed Nov 21, 2022

View reviewed changes

shell/platform/windows/window.cc Show resolved Hide resolved

loic-sharma reviewed Nov 21, 2022

View reviewed changes

shell/platform/windows/window.cc Outdated Show resolved Hide resolved

loic-sharma reviewed Nov 21, 2022

View reviewed changes

shell/platform/windows/window.h Outdated Show resolved Hide resolved

yaakovschectman force-pushed the uia branch from f39c9e3 to 4f5c6fa Compare November 28, 2022 16:50

yaakovschectman requested a review from loic-sharma November 28, 2022 21:14

cbracken reviewed Dec 6, 2022

View reviewed changes

yaakovschectman added 10 commits December 6, 2022 14:04

Initial

b163965

Comment TODOs

d73603e

Shim windowsx

0ee4a7c

Comment testing

18aef20

UIA works at most basic

0406379

Get Native target in win delegate

8687254

Rework events

405ab98

FlutterWindowsView unit tests for UIA

30ed86c

Enable UIA unit tests

5243bca

Conditional UIA implementation

dda9ff1

yaakovschectman added 8 commits December 6, 2022 14:04

Text selection MSAA event

2b0daae

License

8136225

PR

e87e43a

Before change to AccessibilityBridgeWindows

7e1c4d0

AccessibilityBridgeWindows in FlutterWindowsEngine

43920ff

Use AccessibilityBridgeWindows for AXFragmentRootDelegateWin

8a05697

Format

978657d

Remove unneeded windowsx_shim imports

ca62e04

yaakovschectman force-pushed the uia branch from 42748ec to ca62e04 Compare December 6, 2022 19:04

loic-sharma reviewed Dec 6, 2022

View reviewed changes

shell/platform/windows/window.h Show resolved Hide resolved

loic-sharma approved these changes Dec 6, 2022

View reviewed changes

cbracken reviewed Dec 6, 2022

View reviewed changes

cbracken approved these changes Dec 6, 2022

View reviewed changes

PR Comment

77e0ba1

yaakovschectman merged commit dec8b52 into flutter:main Dec 7, 2022

yaakovschectman deleted the uia branch December 7, 2022 16:12

engine-flutter-autoroll mentioned this pull request Dec 9, 2022

Roll Flutter Engine from 8d83b98c55b3 to 030950f3070c (29 revisions) flutter/flutter#116802

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 9, 2022

dec8b5221 Preliminary implementation of UIA for A11y on Windows (flut…

2c175c7

…ter/engine#37754)

nicerloop mentioned this pull request Jun 28, 2024

Add Windows AutomationId #53476

Closed

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Preliminary implementation of UIA for A11y on Windows #37754

Preliminary implementation of UIA for A11y on Windows #37754

yaakovschectman commented Nov 18, 2022

loic-sharma Nov 21, 2022

yaakovschectman Nov 23, 2022

loic-sharma Nov 25, 2022

yaakovschectman Nov 28, 2022

skia-gold commented Nov 28, 2022

cbracken Dec 6, 2022

loic-sharma left a comment

cbracken Dec 6, 2022

cbracken Dec 6, 2022

yaakovschectman Dec 6, 2022

cbracken Dec 6, 2022

cbracken left a comment

Preliminary implementation of UIA for A11y on Windows #37754

Preliminary implementation of UIA for A11y on Windows #37754

Conversation

yaakovschectman commented Nov 18, 2022

Pre-launch Checklist

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

skia-gold commented Nov 28, 2022

Choose a reason for hiding this comment

loic-sharma 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

Choose a reason for hiding this comment

cbracken left a comment

Choose a reason for hiding this comment