Skip to content

Conversation

@jwlilly
Copy link
Contributor

@jwlilly jwlilly commented Oct 27, 2025

Fix for PR #174374. It was reverted due to failing tests for creating CollectionInfo and CollectionItemInfo using constructors added in API 30. I used the obtain method for creating both CollectionInfo and CollectionItemInfo if the API version is lower than 33. The obtain method was deprecated in API 33.

Updating AccessibilityBridge.java and AccessibilityBridgeTest.java to include AccessibilityNodeInfo.CollectionItemInfo to get TalkBack to announce item indexes for ListViews.

Updated the logic for adding CollectionInfo to add when the scrollChildren count is greater than 1. This is an attempt to exclude something like a SingleChildScrollView from getting CollectionInfo added. The only semantics info we can rely on from Android is the hasImplicitScrolling and the number of scrollChildren.
Added logic for adding the CollectionItemInfo to ListView children. It first checks to see if the parent node qualifies for CollectionInfo and then adds the CollectionItemInfo to the child view. The index values come from the childrenInTraversalOrder. We aren't currently passing the indexedSemantics into the SemanticsNode. Once those values are available, this can be made more robust.

The following video includes the new changes with a ListView

list_info_with_a_few_children_screen_recording_20250811_204432.mp4

This video includes the logic with a SingleChildScrollView (no list announcements as expected)

SingleChildScrollView_screen_recording_20250811_204759.mp4

Fixes #168651 - This is a minimal fix. A full fix would be to pass the indexedSemantics to the SemanticsNode in Android and build the list with those values but that is a larger effort.

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], 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.

jwlilly and others added 24 commits August 6, 2025 20:48
…action was present

- Limiting the CollectionInfo to only be apply if there is more than 1 scrollChild
Fixing logic bug with column/row index
Moving comment for setScrollable
@jwlilly jwlilly requested a review from a team as a code owner October 27, 2025 23:25
@github-actions github-actions bot added platform-android Android applications specifically engine flutter/engine related. See also e: labels. team-android Owned by Android platform team labels Oct 27, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 fixes an issue where TalkBack would not announce list information by correctly creating CollectionInfo and CollectionItemInfo for different Android API levels. The changes introduce version checks to handle deprecated methods and improve the logic for identifying lists and list items. My review focuses on improving code maintainability by refactoring duplicated code blocks and enhancing the new test cases with more specific assertions for better debuggability.

Comment on lines 931 to 969
// children.
if (shouldSetCollectionInfo(semanticsNode)) {
if (semanticsNode.hasAction(Action.SCROLL_LEFT)
|| semanticsNode.hasAction(Action.SCROLL_RIGHT)) {
// This code will only run on devices with API level 32 or lower.
// The obtain method was deprecated in API 33.
if (Build.VERSION.SDK_INT < 33) {
result.setCollectionInfo(
AccessibilityNodeInfo.CollectionInfo.obtain(
1, // row count
semanticsNode.scrollChildren, // column count
false // hierarchical
));

} else {
if (shouldSetCollectionInfo(semanticsNode)) {
result.setCollectionInfo(
AccessibilityNodeInfo.CollectionInfo.obtain(
semanticsNode.scrollChildren, // rows
0, // columns
false // hierarchical
));
} else {
result.setClassName("android.widget.ScrollView");
}
result.setCollectionInfo(
new AccessibilityNodeInfo.CollectionInfo(
1, // row count
semanticsNode.scrollChildren, // column count
false // hierarchical
));
}
} else {
// This code will only run on devices with API level 32 or lower.
// The obtain method was deprecated in API 33.
if (Build.VERSION.SDK_INT < 33) {
result.setCollectionInfo(
AccessibilityNodeInfo.CollectionInfo.obtain(
semanticsNode.scrollChildren, // row count
1, // column count
false // hierarchical
));
} else {
result.setCollectionInfo(
new AccessibilityNodeInfo.CollectionInfo(
semanticsNode.scrollChildren, // row count
1, // column count
false // hierarchical
));
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The logic for creating CollectionInfo for horizontal and vertical lists is duplicated and can be simplified. You can determine the row and column counts first, and then use a single block for the version check to create the CollectionInfo object. This will make the code more concise and easier to maintain.

Also, it's better to use Build.VERSION_CODES.TIRAMISU instead of the magic number 33 for better readability.

      final boolean isHorizontal =
          semanticsNode.hasAction(Action.SCROLL_LEFT)
              || semanticsNode.hasAction(Action.SCROLL_RIGHT);
      final int rowCount = isHorizontal ? 1 : semanticsNode.scrollChildren;
      final int colCount = isHorizontal ? semanticsNode.scrollChildren : 1;

      // The obtain method was deprecated in API 33.
      if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) {
        result.setCollectionInfo(
            AccessibilityNodeInfo.CollectionInfo.obtain(
                rowCount, colCount, false /* hierarchical */));
      } else {
        result.setCollectionInfo(
            new AccessibilityNodeInfo.CollectionInfo(rowCount, colCount, false /* hierarchical */));
      }

Comment on lines +985 to +1029
!(parent.hasAction(Action.SCROLL_LEFT) || parent.hasAction(Action.SCROLL_RIGHT));
int nodeIndex = scrollChildren.indexOf(semanticsNode);
if (verticalScroll) {
// This code will only run on devices with API level 32 or lower.
// The obtain method was deprecated in API 33.
if (Build.VERSION.SDK_INT < 33) {
result.setCollectionItemInfo(
AccessibilityNodeInfo.CollectionItemInfo.obtain(
nodeIndex, // row index
1, // row span
0, // column index
1, // column span
semanticsNode.hasFlag(Flag.IS_HEADER) // is heading
));
} else {
result.setCollectionItemInfo(
new AccessibilityNodeInfo.CollectionItemInfo(
nodeIndex, // row index
1, // row span
0, // column index
1, // column span
semanticsNode.hasFlag(Flag.IS_HEADER) // is heading
));
}
} else {
// This code will only run on devices with API level 32 or lower.
// The obtain method was deprecated in API 33.
if (Build.VERSION.SDK_INT < 33) {
result.setCollectionItemInfo(
AccessibilityNodeInfo.CollectionItemInfo.obtain(
0, // row index
1, // row span
nodeIndex, // column index
1, // column span
semanticsNode.hasFlag(Flag.IS_HEADER) // is heading
));
} else {
result.setCollectionItemInfo(
new AccessibilityNodeInfo.CollectionItemInfo(
0, // row index
1, // row span
nodeIndex, // column index
1, // column span
semanticsNode.hasFlag(Flag.IS_HEADER) // is heading
));
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Similar to CollectionInfo, the logic for creating CollectionItemInfo is duplicated for different SDK versions and scroll directions. This can be refactored to be more concise. Determine the row and column indices first, then use a single version check to create the CollectionItemInfo.

Using Build.VERSION_CODES.TIRAMISU instead of 33 is also recommended here for readability.

      final int rowIndex = verticalScroll ? nodeIndex : 0;
      final int colIndex = verticalScroll ? 0 : nodeIndex;

      // The obtain method was deprecated in API 33.
      if (Build.VERSION.SDK_INT < Build.VERSION_CODES.TIRAMISU) {
        result.setCollectionItemInfo(
            AccessibilityNodeInfo.CollectionItemInfo.obtain(
                rowIndex,
                1, // row span
                colIndex,
                1, // column span
                semanticsNode.hasFlag(Flag.IS_HEADER) // is heading
                ));
      } else {
        result.setCollectionItemInfo(
            new AccessibilityNodeInfo.CollectionItemInfo(
                rowIndex,
                1, // row span
                colIndex,
                1, // column span
                semanticsNode.hasFlag(Flag.IS_HEADER) // is heading
                ));
      }

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is an improvement in readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used API_LEVELS.API_33 instead of Build.VERSION_CODES.TIRAMISU. We are using API_LEVELS.API_33 in other places so I thought that might more sense

Copy link
Contributor

Choose a reason for hiding this comment

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

That was the correct thing to sorry I was focused on reducing the duplication and cared less about the raw api number. Using API_LEVELS.API_33 is an improvement thank you.

Comment on lines +2228 to +2232
testSemanticsNode.scrollChildren = 1;
TestSemanticsUpdate testSemanticsUpdate = testSemanticsNode.toUpdate();
testSemanticsUpdate.sendUpdateToBridge(accessibilityBridge);
AccessibilityNodeInfo nodeInfo = accessibilityBridge.createAccessibilityNodeInfo(0);
assertNull(nodeInfo.getCollectionInfo());
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

Combining multiple checks into a single assertTrue can make it harder to debug test failures. It's better to use separate assertions for each condition. This provides clearer failure messages, indicating exactly which assertion failed.

    assertEquals(testSemanticsNode.scrollChildren, collectionInfo.getRowCount());
    assertEquals(1, collectionInfo.getColumnCount()); // 1 column for a list
    assertFalse(collectionInfo.isHierarchical()); // this should currently always be false

Comment on lines +2258 to +2265
childNode1.id = 1;
childNode1.label = "Test 1";
TestSemanticsNode childNode2 = new TestSemanticsNode();
childNode2.id = 2;
childNode2.label = "Test 2";
parentTestSemanticsNode.addChild(childNode1);
parentTestSemanticsNode.addChild(childNode2);
TestSemanticsUpdate testSemanticsUpdate = parentTestSemanticsNode.toUpdate();
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

For better test readability and easier debugging, it's recommended to use separate assertions for each check instead of combining them into a single assertTrue. This will provide more specific feedback if the test fails.

    assertEquals(0, itemInfo.getRowIndex()); // first item in the list
    assertEquals(1, itemInfo.getRowSpan());
    assertEquals(0, itemInfo.getColumnIndex()); // only a single column
    assertEquals(1, itemInfo.getColumnSpan());
    // Note: isHeading() is deprecated, and since this test node doesn't have IS_HEADER flag,
    // we expect it to not be a heading. The heading state is set during CollectionItemInfo
    // construction.
    assertFalse(itemInfo.isHeading());

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with this recommendation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in most recent commit. I changed the assertFalse to assertFalse(nodeInfo.isHeading() since CollectionItemInfo.isHeading() was deprecated in API 28

@reidbaker reidbaker requested a review from a team November 12, 2025 14:48
Copy link
Contributor

@camsim99 camsim99 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for getting this re-landed :)

@camsim99 camsim99 added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 17, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Nov 17, 2025

autosubmit label was removed for flutter/flutter/177622, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 17, 2025
@gmackall gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 1, 2025
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 1, 2025
@auto-submit
Copy link
Contributor

auto-submit bot commented Dec 1, 2025

autosubmit label was removed for flutter/flutter/177622, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

@gmackall gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 1, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Dec 1, 2025
Merged via the queue into flutter:master with commit d18ab0e Dec 1, 2025
180 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 2, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Dec 2, 2025
flutter/flutter@05d6005...5545bb3

2025-12-02 [email protected] Roll Skia from 4371ed0ce49e to 45337c4e919d (1 revision) (flutter/flutter#179342)
2025-12-02 [email protected] Roll Fuchsia Linux SDK from sTk6OB7a4yudbfdZg... to l0DvmZrMHlF12frrX... (flutter/flutter#179338)
2025-12-02 [email protected] Unfocus search anchor bar when the view is closed (flutter/flutter#178910)
2025-12-02 [email protected] Directly generate a Mach-O dynamic library using gen_snapshot. [reland] (flutter/flutter#174870)
2025-12-02 [email protected] Roll Skia from 1fc59bf5cbb1 to 4371ed0ce49e (3 revisions) (flutter/flutter#179326)
2025-12-02 [email protected] [win32] Replace threadpool timer with custom background thread timer (flutter/flutter#179249)
2025-12-02 [email protected] Roll Skia from 61257a1036fb to 1fc59bf5cbb1 (1 revision) (flutter/flutter#179321)
2025-12-02 [email protected] Roll Skia from ef52cf952211 to 61257a1036fb (2 revisions) (flutter/flutter#179319)
2025-12-02 [email protected] Roll Skia from 8887653a773e to ef52cf952211 (1 revision) (flutter/flutter#179316)
2025-12-02 [email protected] Roll pub packages (flutter/flutter#179313)
2025-12-02 [email protected] Update customer tests (flutter/flutter#179309)
2025-12-01 [email protected] Marks Linux_pixel_7pro new_gallery__transition_perf to be unflaky (flutter/flutter#176339)
2025-12-01 [email protected] Fix typo (flutter/flutter#179200)
2025-12-01 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 5.0.1 to 6.0.0 in the all-github-actions group (flutter/flutter#179308)
2025-12-01 [email protected] Roll Dart SDK from c54108eeb2c1 to eb743a1d4ade (1 revision) (flutter/flutter#179304)
2025-12-01 [email protected] Roll pub packages (flutter/flutter#179280)
2025-12-01 [email protected] Roll Skia from 68cc3257e734 to 8887653a773e (4 revisions) (flutter/flutter#179302)
2025-12-01 [email protected] Support round caps for the fast arc stroke generator (flutter/flutter#178269)
2025-12-01 [email protected] Fix for PR #174374 - Fix - TalkBack does not announce list information (flutter/flutter#177622)
2025-12-01 [email protected] Small cleanup in `‎AccessibilityBridge.java‎` (flutter/flutter#179226)
2025-12-01 [email protected] Roll Skia from 925c311f4b37 to 68cc3257e734 (44 revisions) (flutter/flutter#179294)
2025-12-01 [email protected] [ Widget Preview ] Ignore changes under `ios/.symlinks` (flutter/flutter#179290)
2025-12-01 [email protected] Delete unecessary lockfile (flutter/flutter#179052)
2025-12-01 [email protected] Resolving and piping the view ID  through the WidgetController and the TestPointer so that clicks wind up on the right view (flutter/flutter#178941)
2025-12-01 [email protected] Fix link specified as plain text `FlutterApplication.java‎` (flutter/flutter#178573)
2025-12-01 [email protected] Update some comments to reflect theme normalization (flutter/flutter#179013)
2025-12-01 [email protected] Roll Dart SDK from 51fe8cd01fbe to c54108eeb2c1 (1 revision) (flutter/flutter#179267)
2025-12-01 [email protected] Explicitly use FreeType font scanner with Fuchsia (flutter/flutter#179055)

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 [email protected] 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
mboetger pushed a commit to mboetger/flutter that referenced this pull request Dec 2, 2025
…ormation (flutter#177622)

Fix for PR flutter#174374. It was reverted due to failing tests for creating
CollectionInfo and CollectionItemInfo using constructors added in API
30. I used the `obtain` method for creating both CollectionInfo and
CollectionItemInfo if the API version is lower than 33. The `obtain`
method was deprecated in API 33.

> 
> Updating AccessibilityBridge.java and AccessibilityBridgeTest.java to
include AccessibilityNodeInfo.CollectionItemInfo to get TalkBack to
announce item indexes for ListViews.
> 
> Updated the logic for adding CollectionInfo to add when the
scrollChildren count is greater than 1. This is an attempt to exclude
something like a SingleChildScrollView from getting CollectionInfo
added. The only semantics info we can rely on from Android is the
hasImplicitScrolling and the number of scrollChildren.
> Added logic for adding the CollectionItemInfo to ListView children. It
first checks to see if the parent node qualifies for CollectionInfo and
then adds the CollectionItemInfo to the child view. The index values
come from the childrenInTraversalOrder. We aren't currently passing the
indexedSemantics into the SemanticsNode. Once those values are
available, this can be made more robust.
> 
> The following video includes the new changes with a ListView
>
>
https://github.com/user-attachments/assets/b9bf1b80-9f29-40e2-8f33-a08919554b7e
> 
> 
> This video includes the logic with a SingleChildScrollView (no list
announcements as expected)
> 
>
https://github.com/user-attachments/assets/8fe4dd24-cc5a-4a6b-b4b0-0f371fea20df

Fixes flutter#168651 - This is a minimal fix. A full fix would be to pass the
indexedSemantics to the SemanticsNode in Android and build the list with
those values but that is a larger effort.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

---------

Signed-off-by: jwlilly <[email protected]>
Co-authored-by: Reid Baker <[email protected]>
Co-authored-by: Camille Simon <[email protected]>
reidbaker added a commit to AbdeMohlbi/flutter that referenced this pull request Dec 10, 2025
…ormation (flutter#177622)

Fix for PR flutter#174374. It was reverted due to failing tests for creating
CollectionInfo and CollectionItemInfo using constructors added in API
30. I used the `obtain` method for creating both CollectionInfo and
CollectionItemInfo if the API version is lower than 33. The `obtain`
method was deprecated in API 33.

>
> Updating AccessibilityBridge.java and AccessibilityBridgeTest.java to
include AccessibilityNodeInfo.CollectionItemInfo to get TalkBack to
announce item indexes for ListViews.
>
> Updated the logic for adding CollectionInfo to add when the
scrollChildren count is greater than 1. This is an attempt to exclude
something like a SingleChildScrollView from getting CollectionInfo
added. The only semantics info we can rely on from Android is the
hasImplicitScrolling and the number of scrollChildren.
> Added logic for adding the CollectionItemInfo to ListView children. It
first checks to see if the parent node qualifies for CollectionInfo and
then adds the CollectionItemInfo to the child view. The index values
come from the childrenInTraversalOrder. We aren't currently passing the
indexedSemantics into the SemanticsNode. Once those values are
available, this can be made more robust.
>
> The following video includes the new changes with a ListView
>
>
https://github.com/user-attachments/assets/b9bf1b80-9f29-40e2-8f33-a08919554b7e
>
>
> This video includes the logic with a SingleChildScrollView (no list
announcements as expected)
>
>
https://github.com/user-attachments/assets/8fe4dd24-cc5a-4a6b-b4b0-0f371fea20df

Fixes flutter#168651 - This is a minimal fix. A full fix would be to pass the
indexedSemantics to the SemanticsNode in Android and build the list with
those values but that is a larger effort.

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

---------

Signed-off-by: jwlilly <[email protected]>
Co-authored-by: Reid Baker <[email protected]>
Co-authored-by: Camille Simon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine flutter/engine related. See also e: labels. platform-android Android applications specifically team-android Owned by Android platform team

Projects

None yet

4 participants