Repository navigation
OLYM-9858: [Windows] A semantics node that is not selected no longer reads as selected (cherry-pick of #184223) - #11
Merged
Conversation
fix flutter#184058 The issue and solution was both pointed out in flutter#184058 ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md (cherry picked from flutter/flutter commit d04108d, for OLYM-9858: on Windows NVDA said "selected" on every node whose semantics state selected: false.) Co-Authored-By: Claude Opus 5.5 <[email protected]>
…selected tristate) The shared desktop accessibility bridge reported a node whose semantics say "not selected" as selected, so NVDA on Windows said "selected" on every right-panel tab, the Tab settings gear and the rail buttons. Co-Authored-By: Claude Opus 5.5 <[email protected]>
MylioClaude
commented
Nov 11, 2026
MylioClaude
left a comment
MylioClaude
left a comment
Collaborator
Author
There was a problem hiding this comment.
Verdict: approved
The fix is right for OLYM-9858. The bridge now reports a node as selected only when its semantics flag is "true". Before, "false" (value 2) also counted as selected. Nothing must change before the merge. One suggestion below; it doesn't block the merge.
How I read it
- The PR is two commits on top of
OlymEDRSupport(2 ahead, 0 behind), so it merges cleanly. This machine has no checkout of the fork, so I read the diff and the files around it from GitHub at 9b9a111. - Same as upstream. The code change and the new test match upstream flutter/flutter d04108d exactly: the before and after blob hashes of both files are the same. The author is kept, and the commit message names the upstream commit.
- Upstream release claims hold. The 3.41.2 and 3.41.9 tags still pass
flags->is_selectedas a bool. 3.44.0 has the fix. No stable 3.42 or 3.43 exists, so "it falls away once the SDK is 3.44 or later" is right. - What I checked from the review list:
- The new test can fail. On the base, the "false" node (value 2) becomes
kSelected = true, so itsEXPECT_FALSEfails there. The "none" and "true" nodes make sure the fix doesn't go too far. It follows the file's existing pattern (CreateSemanticsNode, thenflags2pointing to a localFlutterSemanticsFlags). - Every door.
accessibility_bridge.ccline 445 is the only place in the common, Windows and macOS accessibility code that readsis_selected(I searched the common bridge and node delegates,accessibility_bridge_windows.cc,flutter_platform_node_delegate_windows.cc,accessibility_plugin.ccandAccessibilityBridgeMac.mm). The other three-state flags in that file (is_expanded,is_toggled,is_focused) already compare withkFlutterTristateTrue. The Linux embedder doesn't use this bridge, and itsfl_accessible_node.ccline 110 already compares correctly. - The platform where the bug was seen. Windows.
AXPlatformNodeWinsetsSTATE_SYSTEM_SELECTEDfromkSelected(ax_platform_node_win.ccline 5089), and that is the MSAA state the repro found on every node. The open tab still getstrue, so it still says "selected". macOS shares the bridge and gets the same fix. - Privacy, stored data, machine writes, async UI, undo: none of these applies to this change.
- Docs. The fork's
UPSTREAM-CANDIDATES.mdgets an entry under "Not candidates", next to the other cherry-picks. In Olym,BUILD_DEPLOY.md§ "Patched Flutter Engine" names only some of the carried fixes ("including …") and points to this file. The right-rail PRD already says each tab cell's node carries its selected state, which this change makes true on Windows. So no Olym doc goes stale.
- The change adds no warnings, dead code, logging, workarounds or dependencies. The code is upstream Flutter's own (BSD licence), so no licence question comes up.
- The new test can fail. On the base, the "false" node (value 2) becomes
Findings
1. Suggestion: try it with NVDA as soon as the Windows engine is rebuilt, before OLYM-9858 is closed
engine/src/flutter/shell/platform/common/accessibility_bridge.cc:445
- What. The PR says the engine wasn't built and the fix wasn't tried on Windows. The swarm can't build the Windows engine before the merge, and the code is the same as a fix upstream has shipped since 3.44.0. So I'm not asking for this before the merge.
- Why it matters.
Build Engineuploads the new tarballs under the samev3.41.2-edrtag, and Olym's engine injector picks them up by their ETag. So the first Windows build with this fix goes straight to every Olym build. The NVDA check right after that build is the only time anyone tries this fix on Windows. - What to do. Keep OLYM-9858 open until that check has been done, as the PR's "After the merge" section says. Check the left navigation rail as well as the right panel's tabs, the ⋮ and the gear: only Library (or whichever view is open) should say "selected" there. On macOS, the earlier "no selected state at all" finding (OLYM-9026) takes a different route. Note any change there on that ticket, not this one.
Collaborator
Author
|
Merged by the swarm's jira-watch as review round 1 approved it: a review that approves a pull request merges it as it was reviewed, with no answer session (the swarm's check-in rules, rule 2). The review's suggestions stay on this pull request; the swarm took none of them in it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes OLYM-9858: on Windows, NVDA says "selected" on every right-panel tab, the Info tab's ⋮ (Customize sections), the Tab settings gear and the rail buttons (Info, Share, Edit, Show QuickAlbum, and the whole left navigation rail), not just the one that is open.
Root cause. In the shared desktop accessibility bridge,
AccessibilityBridge::SetBooleanAttributesFromFlutterUpdate(engine/src/flutter/shell/platform/common/accessibility_bridge.cc) did:node_data.AddBoolAttribute(ax::mojom::BoolAttribute::kSelected, flags->is_selected);is_selectedis aFlutterTristate(none = 0, true = 1, false = 2). As a bool, false (2) is true, so a node whose Dart semantics sayselected: falsewas reported as selected. Only nodes with no selected state at all (none) came out right. The repro read the MSAA tree on syswindows1 and foundSTATE_SYSTEM_SELECTEDon every node that setsselected:, true or false. The Dart side passes the right values (Semantics(selected: widget.isSelected)on the tab cell,isSelected: _tabConfigMenuOpenon the gear). The Linux embedder does not use this bridge and already compares withkFlutterTristateTrue(fl_accessible_node.cc), so Linux was never affected. macOS uses the same bridge, so its roles that readkSelectedget the same fix.Fix. A cherry-pick of upstream flutter/flutter d04108d, "[a11y] fix selected tristate flags mismatch (flutter#184223)" (fixes flutter#184058), author kept. It compares with
kFlutterTristateTrue, likeis_expanded,is_toggledandis_focusedin the same file, and adds the testAccessibilityBridgeTest.IsSelectedAttribute(none, true, false). Upstream first shipped it in 3.44.0; every 3.41.x tag (checked 3.41.4 to 3.41.9) still has the bug. A second commit adds it toUPSTREAM-CANDIDATES.mdunder "Not candidates", so it is dropped once the SDK is 3.44 or later.How I tested.
Build Enginebuilds only the branch tip after a merge.clang++ -std=c++20 -fsyntax-onlyon the branch'saccessibility_bridge.ccagainst the engine's in-tree headers (stock 3.41.2): no errors. The same command reports an error when the new line is broken on purpose, so it does check that line.After the merge. Someone runs Olym's
Build Engineworkflow forwindowsandwindows-arm64(andmacos, which shares the bridge) so new tarballs go up underv3.41.2-edr. Olym's engine injector notices the new tarballs by their ETag, so nothing in Olym changes. Then on Windows with NVDA: select a photo, Tab through the right panel's tabs and the gear. Only the open tab should say "selected".Ticket: https://mylio.atlassian.net/browse/OLYM-9858
🤖 Generated with Claude Code