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

Conversation

@loic-sharma
Copy link
Member

@loic-sharma loic-sharma commented Mar 1, 2023

Today, the root accessibility semantics node is guaranteed to have ID 0. In a multi-window world, each view will have its own semantics tree, but semantic node IDs will be globally unique. In other words, the semantics tree's root will no longer be guaranteed to have ID 0.

This change removes another "root ID is 0" assumption from the Windows embedder that was missed in #39441:

  1. Remove a hardcoded ID 0 when getting the native root IAccessible object
  2. Move logic to get the native root IAccessible object from FlutterWindowsView to AccessibilityBridgeWindows

Part of flutter/flutter#119391

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.

@loic-sharma loic-sharma changed the title [Windows] Remove accessibility root ID assumption [Windows] Remove another accessibility root ID assumption Mar 1, 2023
@loic-sharma loic-sharma marked this pull request as ready for review March 1, 2023 23:46
bridge->CommitUpdates();

// Look up the root windows node delegate.
auto node_delegate = bridge->GetFlutterPlatformNodeDelegateFromID(1).lock();
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also test that GetFlutterPlatformNodeDelegateFromID(0) returns node0 as we would expect? If the rest of the test passes, I'd think it would, but it may still be useful to make sure arbitrary ID ordering works.

Copy link
Member Author

@loic-sharma loic-sharma Mar 2, 2023

Choose a reason for hiding this comment

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

Good idea. I also made the root ID 1 and the child ID 2 and updated the tests to:

  1. Verify node ID 0 is unused
  2. Verify node ID 1 and 2 exist as expected

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

autosubmit Merge PR when tree becomes green via auto submit App platform-windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants