Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@cbracken
Copy link
Member

@cbracken cbracken commented Nov 19, 2021

This is the third in a series of patches adding accessibility support
for the Windows embedder. This patch wires in the Accessibility bridge,
and lands the core structure of the Windows FlutterPlatformNodeDelegate
and AccessibilityBridgeDelegate classes, including:

  • Instantiating the AccessibilityBridge when the semantics tree is
    enabled.
  • Creating FlutterPlatformNodeDelegate wrappers for semantics tree
    nodes.
  • Building and updating the accessibility tree on semantics updates.
  • Returning the native IAccessible objects when queried.
  • The scaffolding for an eventual UWP implementation. This is low
    priority, but required if we don't want to break the UWP build.

Breaking this out so that the follow-up patches can be reviewed and
landed in smaller, independent chunks.

Issue: flutter/flutter#77838
Issue: flutter/flutter#93928

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.

@cbracken cbracken requested a review from chunhtai November 19, 2021 02:24
@google-cla google-cla bot added the cla: yes label Nov 19, 2021
@cbracken cbracken added the Work in progress (WIP) Not ready (yet) for review! label Nov 19, 2021
@cbracken cbracken changed the title Wire in for Win32 AccessibilityBridge Win32 a11y bridge and platform node delegates Nov 19, 2021
gfx::NativeViewAccessible root_iaccessible = GetNativeViewAccessible();
if (root_iaccessible) {
LresultFromObject(IID_IAccessible, wparam, root_iaccessible);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a way to actively attach the root accessibility node to the view?

The first time this method is called root_iaccessible will be null because the update will only come after one frame delay.

Copy link
Contributor

Choose a reason for hiding this comment

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

btw, Is this also how chromium listens to the accessibility services?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is how the root accessibility node is attached to the view.

When Windows wants the accessibility node associated with the HWND it fires a WM_GETOBJECT event into the main event loop, which we then handle in this method. We look up the root AXPlatformNodeWin (which implements IAccessible) above in GetNativeViewAccessible() and return it via LresultFromObject() here.

[](const FlutterSemanticsCustomAction* action, void* user_data) {
auto host = static_cast<FlutterWindowsEngine*>(user_data);
if (!action || action->id == kFlutterSemanticsNodeIdBatchEnd) {
host->accessibility_bridge_->CommitUpdates();
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably need to attach root accessibility node to the view as well, this will be the first time the accessibility tree is avaliable

Copy link
Member Author

Choose a reason for hiding this comment

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

Replied above about how attaching the root node is done via WM_GETOBJECT.

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good

@chunhtai
Copy link
Contributor

I understand this is only part of the patch, but since it is already hooked up to the flutter engine, we should have a simple test to just make sure the thing doesn't crash.

Maybe just some test to make sure you can turn semantics on and off and receive some update without crashing the test

This is the third in a series of patches adding accessibility support
for the Windows embedder. This patch wires in the Accessibility bridge,
and lands the core structure of the Windows FlutterPlatformNodeDelegate
and AccessibilityBridgeDelegate classes, including:

* Instantiating the AccessibilityBridge when the semantics tree is
  enabled.
* Creating FlutterPlatformNodeDelegate wrappers for semantics tree
  nodes.
* Building and updating the accessibility tree on semantics updates.
* Returning the native IAccessible objects when queried.

Breaking this out so that the follow-up patches can be reviewed and
landed in smaller, independent chunks.

Issue: flutter/flutter#77838
There's nothing to do here.
Verify that a windows IAccessible COM object is generated.
@cbracken
Copy link
Member Author

Maybe just some test to make sure you can turn semantics on and off and receive some update without crashing the test.

Added.

Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

LGTM, I didn't test it locally since i don't have a device, but the wiring logic looks good

void FlutterPlatformNodeDelegateWin32::Init(std::weak_ptr<OwnerBridge> bridge,
ui::AXNode* node) {
FlutterPlatformNodeDelegate::Init(bridge, node);
ax_platform_node_ = ui::AXPlatformNode::Create(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the fragment root created? Is it in a later patch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fragment roots are only needed if we implement UIA, which is the newer a11y API. Talked to @clarkezone who mentioned we're safe with sticking with MSAA for the time being.

[](const FlutterSemanticsCustomAction* action, void* user_data) {
auto host = static_cast<FlutterWindowsEngine*>(user_data);
if (!action || action->id == kFlutterSemanticsNodeIdBatchEnd) {
host->accessibility_bridge_->CommitUpdates();
Copy link
Contributor

Choose a reason for hiding this comment

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

sounds good

@cbracken cbracken merged commit 57ac30a into flutter:main Nov 29, 2021
@cbracken cbracken deleted the a11y-windows-delegates branch November 29, 2021 21:23
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants