Wires up Android API to set section locale #173364

chunhtai · 2025-08-06T18:56:58Z

what still missing:

iOS equivalence
set application level locale

internal only doc: go/flutter-semantics-language

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.

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

gemini-code-assist

Code Review

This pull request wires up the Android accessibility API to support setting a locale on a semantics node. The changes span from the UI layer in C++ down to the Android platform-specific Java code. My review includes a few suggestions: fixing a bug in a new unit test, using the correct log level for a debug message, and removing an unused parameter from a function signature. Overall, the changes look good and the new test coverage is appreciated.

.../platform/android/platform_view_android_delegate/platform_view_android_delegate_unittests.cc

engine/src/flutter/lib/ui/semantics/semantics_update_builder.cc

engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java

flutter-dashboard · 2025-08-06T22:48:42Z

CI had a failure that stopped further tests from running. We need to investigate to determine the root cause.

SHA at time of execution: 494a34b.

Possible causes:

Configuration Changes: The .ci.yaml file might have been modified between the creation of this pull request and the start of this test run. This can lead to ci yaml validation errors.
Infrastructure Issues: Problems with the CI environment itself (e.g., quota) could have caused the failure.

A blank commit, or merging to head, will be required to resume running CI for this PR.

Error Details:

GitHub Error: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID B89A:2C3875:3D1A6AA:9252E28:6893DBCA.

Stack trace:

#0      GitHub.handleStatusCode (package:github/src/common/github.dart:486:5)
#1      GitHub.request (package:github/src/common/github.dart:422:7)
<asynchronous suspension>
#2      GitHub.requestJson (package:github/src/common/github.dart:323:22)
<asynchronous suspension>
#3      RetryOptions.retry (package:retry/retry.dart:131:16)
<asynchronous suspension>
#4      LuciBuildService.scheduleTryBuilds (package:cocoon_service/src/service/luci_build_service.dart:246:24)
<asynchronous suspension>
#5      Scheduler._runCiTestingStage (package:cocoon_service/src/service/scheduler.dart:1255:9)
<asynchronous suspension>
#6      Scheduler.proceedToCiTestingStage (package:cocoon_service/src/service/scheduler.dart:1316:7)
<asynchronous suspension>
#7      Scheduler._closeSuccessfulEngineBuildStage (package:cocoon_service/src/service/scheduler.dart:1127:5)
<asynchronous suspension>
#8      Scheduler.processCheckRunCompletion (package:cocoon_service/src/service/scheduler.dart:1060:9)
<asynchronous suspension>
#9      Scheduler.processCheckRun (package:cocoon_service/src/service/scheduler.dart:1400:9)
<asynchronous suspension>
#10     GithubWebhookSubscription.post (package:cocoon_service/src/request_handlers/github/webhook_subscription.dart:120:24)
<asynchronous suspension>
#11     RequestHandler.service (package:cocoon_service/src/request_handling/request_handler.dart:45:20)
<asynchronous suspension>
#12     SubscriptionHandler.service (package:cocoon_service/src/request_handling/subscription_handler.dart:140:5)
<asynchronous suspension>
#13     createServer.<anonymous closure> (package:cocoon_service/server.dart:329:7)
<asynchronous suspension>

chunhtai · 2025-08-15T21:01:36Z

/gemini review

gemini-code-assist

Code Review

This pull request wires up the Android accessibility API to support setting a locale on a semantics node. The changes span across Dart, C++, and Java to pass the locale information from the framework to the Android accessibility services. The implementation is mostly correct, but I've found a critical issue in the Java code where an incorrect locale format will cause a runtime crash. I've also pointed out an unused parameter in the C++ code that should be addressed.

engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java

gemini-code-assist · 2025-08-15T21:03:38Z

engine/src/flutter/lib/ui/semantics/semantics_update_builder.cc

@@ -69,7 +69,9 @@ void SemanticsUpdateBuilder::updateNode(
    std::string linkUrl,
    int role,
    const std::vector<std::string>& controlsNodes,
-    int validationResult) {
+    int validationResult,
+    int inputType,


The inputType parameter is accepted by updateNode but it is not used within the function's body. This could indicate an incomplete implementation or leftover code. To improve code clarity and maintainability, this unused parameter should be either utilized or removed.

Is this bot comment correct?

it is not used because there is no usage outside of web (this file is the glue code for non-web engine), but i have to keep it here because this is a positional parameter so that I can use the locale parameter

engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java

reidbaker · 2025-08-18T18:23:19Z

...tter/shell/platform/android/platform_view_android_delegate/platform_view_android_delegate.cc

+      if (node.locale.empty()) {
+        buffer_int32[position++] = -1;
+      } else {
+        buffer_int32[position++] = strings.size();
+        strings.push_back(node.locale);
+      }
+


No change you need to make but this makes me sad to review. This type of message passing is so easy to mess up in a way that is hard for authors or reviewers to realize. There are formats for passing data via strings and the conversion code is all generated.

There are formats for passing data via strings and the conversion code is all generated.

Do you have example? I only know json, but think that may not be efficient

protobuffs, msgpack, even pigeon which we maintain lets authors author a platform independent object that is typesafe after serialization across the wire.

reidbaker · 2025-08-18T18:24:41Z

...tter/shell/platform/android/platform_view_android_delegate/platform_view_android_delegate.cc

@@ -266,6 +266,13 @@ void PlatformViewAndroidDelegate::UpdateSemantics(
        strings.push_back(node.linkUrl);
      }

+      if (node.locale.empty()) {


Somewhere in this code please articulate the meaning of the magical number -1 when used in the local location and the expected format of the local string. Especially if the assumption is for it to match some standard.

I refactored out the code so less duplicate, but I am not sure how can i addressed the magical number -1 here since defining a constant won't be useful outside this class (it can't be use from java side when decoding the buffer)

reidbaker · 2025-08-18T18:26:48Z

engine/src/flutter/lib/ui/semantics/semantics_update_builder.cc

@@ -69,7 +69,9 @@ void SemanticsUpdateBuilder::updateNode(
    std::string linkUrl,
    int role,
    const std::vector<std::string>& controlsNodes,
-    int validationResult) {
+    int validationResult,
+    int inputType,


Is this bot comment correct?

reidbaker

Couple of comments, In addition to my review please get a review from someone who is more comfortable in c++

engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java

reidbaker · 2025-08-22T18:54:24Z

engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java

+  private static String getStringFromBuffer(@NonNull ByteBuffer buffer, @NonNull String[] strings) {
+    int stringIndex = buffer.getInt();
+
+    return stringIndex == -1 ? null : strings[stringIndex];


Please make -1 a private constant. Maybe name it NOT_FOUND?

reidbaker · 2025-08-22T18:55:10Z

engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java

+   * <p>Use {@code addLocale} to set the locale and/or {@code addUrl} to set the url for the entire
+   * string. Uses {@code addAttributes} to add any additional {@code StringAttribute} to the string
+   */
+  private static class AccessibilityStringBuilder {


Consider putting this into its own file. This file is already pretty large.

reidbaker · 2025-08-22T18:57:02Z

.../platform/android/platform_view_android_delegate/platform_view_android_delegate_unittests.cc

+  expected_strings.push_back(node0.identifier);
+  buffer_int32[position++] = expected_strings.size();  // node0.label
+  expected_strings.push_back(node0.label);
+  buffer_int32[position++] = -1;  // node0.labelAttributes


Thank you for labeling these values it helps with readability.

chunhtai requested a review from a team as a code owner August 6, 2025 18:56

github-actions bot added platform-android Android applications specifically engine flutter/engine repository. See also e: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) team-android Owned by Android platform team labels Aug 6, 2025

gemini-code-assist bot reviewed Aug 6, 2025

View reviewed changes

chunhtai commented Aug 6, 2025

View reviewed changes

engine/src/flutter/shell/platform/android/io/flutter/view/AccessibilityBridge.java Outdated Show resolved Hide resolved

chunhtai force-pushed the issues/99600 branch from 494a34b to edc268c Compare August 7, 2025 19:36

chunhtai requested a review from reidbaker August 14, 2025 17:40

chunhtai force-pushed the issues/99600 branch 2 times, most recently from 1519545 to a946cbb Compare August 15, 2025 20:36

gemini-code-assist bot reviewed Aug 15, 2025

View reviewed changes

reidbaker reviewed Aug 18, 2025

View reviewed changes

chunhtai force-pushed the issues/99600 branch 2 times, most recently from 6ebed01 to c3fa588 Compare August 19, 2025 15:40

chunhtai requested a review from reidbaker August 19, 2025 16:04

chunhtai force-pushed the issues/99600 branch 3 times, most recently from 5fc4116 to 1d44e84 Compare August 22, 2025 18:51

reidbaker reviewed Aug 22, 2025

View reviewed changes

reidbaker requested a review from gmackall August 22, 2025 18:58

Wires up android semantics locale

549a4a1

chunhtai requested a review from gaaclarke August 22, 2025 21:23

chunhtai added 2 commits August 22, 2025 14:25

format

0bb4818

format

02c9909

chunhtai force-pushed the issues/99600 branch from 1d44e84 to 02c9909 Compare August 22, 2025 21:26

Wires up Android API to set section locale #173364

Are you sure you want to change the base?

Wires up Android API to set section locale #173364

Conversation

chunhtai commented Aug 6, 2025

Pre-launch Checklist

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

flutter-dashboard bot commented Aug 6, 2025

Uh oh!

chunhtai commented Aug 15, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist bot Aug 15, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reidbaker Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reidbaker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

reidbaker Aug 18, 2025 •

edited

Loading