-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[Android] Use headingLevel for heading accessibility property #175416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly updates the Android accessibility implementation to use the headingLevel
property for determining headings, moving away from the older IS_HEADER
flag. The changes are consistently applied across the Java and C++ layers, including serialization and deserialization logic. The adjustment to kBytesPerNode
is correct, and the new unit tests thoroughly validate the new behavior, including edge cases. I have a couple of minor suggestions to improve code clarity and test structure, but overall this is a solid improvement.
...utter/shell/platform/android/platform_view_android_delegate/platform_view_android_delegate.h
Show resolved
Hide resolved
engine/src/flutter/shell/platform/android/test/io/flutter/view/AccessibilityBridgeTest.java
Outdated
Show resolved
Hide resolved
b667fb8
to
2920d02
Compare
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
9081031
to
ceb0889
Compare
f6b117a
to
dfd7461
Compare
b256564
to
e5f52b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The major concern is the breaking ABI change. Left some suggestions on how to avoid it.
0bbb4ec
to
8204134
Compare
f01f2d4
to
ca98f06
Compare
The requested changes have been made.
The embedder API change LGTM. I'll leave the rest to the @flutter/android-reviewers. Thanks! |
1daf188
to
7d4eb1d
Compare
auto label is removed for flutter/flutter/175416, Failed to enqueue flutter/flutter/175416 with HTTP 400: GraphQL mutate failed. |
…10145) Manual roll requested by tarrinneal@google.com flutter/flutter@96fe3b3...c9608e2 2025-09-30 matt.kosarek@canonical.com Implement framework interface for the dialog window archetype (flutter/flutter#176202) 2025-09-30 jhy03261997@gmail.com Update flutter test to use SemanticsFlags (flutter/flutter#175987) 2025-09-30 1063596+reidbaker@users.noreply.github.com Set minimum supported java version to 17 (flutter/flutter#176226) 2025-09-30 mdebbar@google.com Reduce timeout for Linux web_tool_tests back to 60 (flutter/flutter#176286) 2025-09-30 engine-flutter-autoroll@skia.org Roll Packages from 34eec78 to 287739d (9 revisions) (flutter/flutter#176284) 2025-09-30 mdebbar@google.com [web] Bump Firefox to 143.0 (flutter/flutter#176110) 2025-09-30 32538273+ValentinVignal@users.noreply.github.com Migrate to `WidgetStateBorderSide` (flutter/flutter#176164) 2025-09-30 78146827+RootHex200@users.noreply.github.com Enhance input decorator padding logic for character counter in text f… (flutter/flutter#175706) 2025-09-30 jacksongardner@google.com Update the test package for the web engine unit test bits. (flutter/flutter#176241) 2025-09-30 robert.ancell@canonical.com Warn if embedder API calls don't return success (flutter/flutter#176184) 2025-09-30 engine-flutter-autoroll@skia.org Roll Fuchsia Test Scripts from APSBP-sS-3FX69Ihf... to JUeFbA8y0E-_pj-bg... (flutter/flutter#176243) 2025-09-30 jason-simmons@users.noreply.github.com Roll GN to 81b24e01 (flutter/flutter#176119) 2025-09-29 matt.kosarek@canonical.com Rename DisplayMonitor to DisplayManager on Win32 (flutter/flutter#175619) 2025-09-29 mohamed.nayef95@gmail.com [Android] Use headingLevel for heading accessibility property (flutter/flutter#175416) 2025-09-29 80628866+markyang92@users.noreply.github.com BUILD.gn: Support LTO build on Linux (flutter/flutter#176191) 2025-09-29 mohellebiabdessalem@gmail.com fix `assertEquals` arguments are in wrong order in `FlutterJNITest.java` (flutter/flutter#175728) 2025-09-29 mohellebiabdessalem@gmail.com Add tests for `Project` getters (flutter/flutter#175994) 2025-09-29 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 8zjcJic_DtvB2Bo2x... to rcOl0yxJb4znJ903Y... (flutter/flutter#176215) 2025-09-29 mohellebiabdessalem@gmail.com Clean up typos in `PlatformViewsControllerTest.java` (flutter/flutter#175725) 2025-09-29 1063596+reidbaker@users.noreply.github.com Migrate java 11 usage to java 17 usage for templates (flutter/flutter#176203) 2025-09-29 aam@google.com User Invoke-Expression instead of call operator for nested Powershell scripts invocations (on Windows) (flutter/flutter#175941) 2025-09-29 jmccandless@google.com Update changelog as on 3.35 branch (flutter/flutter#176216) 2025-09-29 mohellebiabdessalem@gmail.com fix typo in `Crashes.md` (flutter/flutter#175959) 2025-09-29 15619084+vashworth@users.noreply.github.com Add scene plugin lifecycle events (flutter/flutter#175866) 2025-09-29 1063596+reidbaker@users.noreply.github.com Migrate tests and documentation to set java version to 17 (flutter/flutter#176204) 2025-09-29 jessiewong401@gmail.com Update Engine CI to use NDK r28c (flutter/flutter#175870) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes #174150
This PR updates the Android
AccessibilityBridge
to use theheadingLevel
property fromSemanticsNode
to determine if a node should be marked as a heading for accessibility purposes.Previously, the
IS_HEADER
flag was used. Now, a node is considered a heading ifheadingLevel > 0
. This aligns with the common accessibility pattern where heading levels (like H1, H2, etc.) define the structure.The following changes were made:
AccessibilityBridge.java
to set the heading status based onsemanticsNode.headingLevel > 0
for Android API level 28 and above.headingLevel
to theSemanticsNode
data structure in Java and its serialization/deserialization logic.PlatformViewAndroidDelegate
to includeheadingLevel
when serializingSemanticsNode
data.kBytesPerNode
inplatform_view_android_delegate.h
to account for the newheadingLevel
field.AccessibilityBridgeTest.java
to verify the new heading logic:headingLevel = 0
are not headings.headingLevel > 0
are headings.Pre-launch Checklist
///
).