Add vmservices for accessibilityEvaluation by chunhtai · Pull Request #182791 · flutter/flutter

chunhtai · 2026-02-23T22:48:15Z

still needs to wire up devtools side

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 introduces VM service extensions for accessibility evaluations, allowing tools like DevTools to trigger and retrieve accessibility rule violations (tap target size, semantic labels, and text contrast). The changes include refactoring existing evaluation classes to be more configurable and adding the necessary registration logic in WidgetsBinding. Overall, the implementation is solid, but there are several areas in the service extension callback where input validation and error handling should be improved to prevent crashes from malformed parameters.

packages/flutter/lib/src/widgets/binding.dart

chunhtai · 2026-02-25T17:09:08Z

packages/flutter/lib/src/widgets/_accessibility_evaluations.dart

  final Size size;

-  /// A link describing the tap target evaluations for a platform.
-  final String link;


This is a clean up, I think the link or other supplement information should be provided by the subclass or consumer class that leverage this class.

chunhtai · 2026-02-25T17:10:10Z

packages/flutter/lib/src/widgets/_accessibility_evaluations.dart

-///  * Normal text must have a contrast ratio of at least 7.0
-///  * Large or bold text must have a contrast ratio of at least 4.5
-@internal
-class MinimumTextContrastEvaluationAAA extends MinimumTextContrastEvaluation {


This should be defined by the consumer classes like accessibilityGuidline or devtool's ui

chunhtai · 2026-02-25T17:11:15Z

packages/flutter/lib/src/widgets/binding.dart

+          }
+
+          switch (type) {
+            case 'MinimumTextContrastEvaluation':


not sure if there is a better way to define these string since this endpoint will be called by devtools

Are there other service extension implementations that define public constants anywhere for parameter names or types? Is there any enum associated with these names that could be used?

bkonyi · 2026-02-26T20:49:37Z

packages/flutter/lib/src/widgets/binding.dart

+
+  Map<String, List<Map<String, String>>> _formatEvaluationResult(List<Violation> violations) {
+    return <String, List<Map<String, String>>>{
+      'result': violations.map((Violation violation) {


Nit: this can be written without the map and the result key (it's already part of the JSON RPC response):

return List<Map<String, String>>[ for (final violation in violations) { 'nodeId': violation.node.id.toString(), 'message': violation.reason, } };

Or with some object destructuring:

return List<Map<String, String>>[ for (final Violation(:node, :reason) in violations) { 'nodeId': node.id.toString(), 'message': reason, } }

Will this work? ServiceExtensionCallback expects a Future<Map<String, dynamic>> as return value

Ugh, yup, you're right. The implementation of registerServiceExtension is more restrictive than it needs to be. In theory, any JSON serializable type should be supported as a return value, as that's what ServiceExtensionResponse.result(...) accepts.

This isn't a big deal, it just means there's some unnecessary layers of collections and the RPC response will look weird.

I'd say it'd be good to fix the registerServiceExtension implementation to allow for arbitrary JSON encodable responses, but it looks like we inject the method and set the type to _extensionType in the response, assuming it's a Map. Looking through code search, we really don't need to do this as nothing I can find cares about these values in the response outside of tests and they don't add any value, so I think it'd be safe to remove them and allow for non-Map results.

I know that's extra work, so I'll leave it up to you if that's a modification you're willing to pursue.

If we do change the return response, we may need to migrate the consumer of these endpoint. Do you know what other services we need to look into besides devtools and mcp?

I think this should probably be another pr, filed #183017