-
Notifications
You must be signed in to change notification settings - Fork 29.2k
Split hint from label and expose it via aria-description or aria-describedby #169157
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
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. |
@@ -456,6 +462,7 @@ class LabelAndValue extends SemanticBehavior { | |||
} | |||
|
|||
_getEffectiveRepresentation().update(computedLabel); | |||
_updateHintDescription(semanticsObject.hint); |
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.
somewhere in the doc of this class should mention hint translate to description
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.
Updated.
void _updateHintDescription(String? hint) { | ||
owner.removeAttribute('aria-description'); | ||
owner.removeAttribute('aria-describedby'); | ||
if (hint != null && hint.trim().isNotEmpty) { |
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.
if (hint != null && hint.trim().isNotEmpty) { | |
if (hint?.trim().isNotEmpty ?? false) { |
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.
ignore this if this doesn't promote the hint to non-null.
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.
it does not promote hint to non-null inside the block, so hint is still of type String?, but _setAriaDescriptionOrDescribedBy expects a non-nullable String.
} | ||
|
||
@visibleForTesting | ||
static set supportsAriaDescriptionForTest(bool? value) { |
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.
why nullable value?
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.
Passing null resets _supportsAriaDescription to an uninitialized state.
The backing field _supportsAriaDescription is also nullable (bool?).
This setter is used for testing, to override the browser-detected value.
@@ -446,6 +449,9 @@ class LabelAndValue extends SemanticBehavior { | |||
/// instead. | |||
LabelRepresentation preferredRepresentation; | |||
|
|||
String? _describedById; |
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.
maybe convert this to a getter so we we don't need to keep them in sync?
String? get _describedById => _describedBySpan?.id
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.
Good idea. Updated.
_describedBySpan!.setAttribute('hidden', ''); | ||
_describedBySpan!.text = hint; | ||
// Attach as a child of the semantics element, not document.body | ||
if (_describedBySpan!.isConnected != true) { |
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.
I remember a lint would suggest to use ?? instead of equality for checking nullable boolean
if (_describedBySpan!.isConnected != true) { | |
if (!(_describedBySpan?.isConnected ?? false)) { |
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.
Updated.
_describedBySpan = null; | ||
_describedById = null; | ||
final String hintId = 'hint-${semanticsObject.id}'; | ||
domDocument.getElementById(hintId)?.remove(); |
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.
what is this for?
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.
Updated the logic and remove dead code a bit.
_describedBySpan?.remove() and _describedBySpan = null; removes our aria-describedby span and clears the reference.
if (isSafari) { | ||
final RegExp safariRegexp = RegExp(r'Version\/([0-9]+)\.([0-9]+)'); | ||
final RegExpMatch? match = safariRegexp.firstMatch(ui_web.browser.userAgent); | ||
if (match != null) { | ||
final int majorVersion = int.parse(match.group(1)!); | ||
final int minorVersion = int.parse(match.group(2)!); | ||
return majorVersion > 17 || (majorVersion == 17 && minorVersion >= 4); | ||
} | ||
return false; | ||
} | ||
if (isFirefox) { | ||
final RegExp firefoxRegexp = RegExp(r'Firefox\/([0-9]+)'); | ||
final RegExpMatch? match = firefoxRegexp.firstMatch(ui_web.browser.userAgent); | ||
if (match != null) { | ||
final int version = int.parse(match.group(1)!); | ||
return version >= 119; | ||
} | ||
return false; | ||
} |
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.
In the past, we usually put detection logic in the engine/browser_detection.dart
file and expose booleans from there.
You could, for example, introduce a isSafari174OrNewer
and isFirefox119OrNewer
in engine/browser_detection.dart
.
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.
Great suggestion. updated.
owner.removeAttribute('aria-description'); | ||
owner.removeAttribute('aria-describedby'); | ||
if (hint != null && hint.trim().isNotEmpty) { | ||
_setAriaDescriptionOrDescribedBy(hint); | ||
} |
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.
Can we avoid removing the attributes when not necessary?
Something like this might work:
owner.removeAttribute('aria-description'); | |
owner.removeAttribute('aria-describedby'); | |
if (hint != null && hint.trim().isNotEmpty) { | |
_setAriaDescriptionOrDescribedBy(hint); | |
} | |
if (hint != null && hint.trim().isNotEmpty) { | |
_setAriaDescriptionOrDescribedBy(hint); | |
} else { | |
owner.removeAttribute('aria-description'); | |
owner.removeAttribute('aria-describedby'); | |
} |
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.
Great suggestion. updated.
…t to avoid 'Too many elements' test failures
flutter/flutter@4372bfb...0e536eb 2025-05-28 30870216+gaaclarke@users.noreply.github.com Introduces FlutterPluginRegistrant protocol. (flutter/flutter#169399) 2025-05-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Initialize `default-flavor` in `FlutterCommand`, adds integration test. (#169298)" (flutter/flutter#169581) 2025-05-28 matanlurey@users.noreply.github.com Initialize `default-flavor` in `FlutterCommand`, adds integration test. (flutter/flutter#169298) 2025-05-28 jakemac@google.com Update DEPS to add dart-lang/ai repo (flutter/flutter#169540) 2025-05-28 engine-flutter-autoroll@skia.org Roll Skia from 92311f2ba0b7 to 82d326fc2148 (1 revision) (flutter/flutter#169552) 2025-05-28 kevmoo@users.noreply.github.com dev/bots: improve service worker test code (flutter/flutter#169231) 2025-05-28 magder@google.com Make Android team platform view TESTOWNERS (flutter/flutter#169297) 2025-05-28 engine-flutter-autoroll@skia.org Roll Skia from 044f58f78a73 to 92311f2ba0b7 (9 revisions) (flutter/flutter#169542) 2025-05-27 engine-flutter-autoroll@skia.org Roll Skia from 443f5257f382 to 044f58f78a73 (16 revisions) (flutter/flutter#169530) 2025-05-27 sokolovskyi.konstantin@gmail.com [web] Fix unresponsive input above SelectionArea in Safari and Firefox. (flutter/flutter#167275) 2025-05-27 58529443+srujzs@users.noreply.github.com Set pause_isolates_on_start flag if --start-paused (flutter/flutter#169392) 2025-05-27 engine-flutter-autoroll@skia.org Roll Packages from af0b9a9 to 6eebe72 (24 revisions) (flutter/flutter#169514) 2025-05-27 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 5mpmPsuD8rpeiJizT... to nC9hLWjYVlChDTEPh... (flutter/flutter#169498) 2025-05-27 zhongliu88889@gmail.com Split hint from label and expose it via aria-description or aria-describedby (flutter/flutter#169157) 2025-05-26 github@alexv525.com 🐛 Normalize generated file paths for the l10n generator (flutter/flutter#169467) 2025-05-26 engine-flutter-autoroll@skia.org Roll Dart SDK from d811152316e4 to 6aeb798bdbe2 (2 revisions) (flutter/flutter#169478) 2025-05-26 dkwingsmt@users.noreply.github.com [Cupertino] Apply RSuperellipse to most Cupertino widgets (flutter/flutter#167784) 2025-05-26 bkonyi@google.com Roll `package:dds` to 5.0.2 (flutter/flutter#169471) 2025-05-26 matanlurey@users.noreply.github.com Use `.flutter-plugins-dependencies` for crash reporting. (flutter/flutter#169319) 2025-05-26 matanlurey@users.noreply.github.com Remove now disabled code that would generate `.flutter-plugins`. (flutter/flutter#169320) 2025-05-26 engine-flutter-autoroll@skia.org Roll Dart SDK from 7dab9bffe1f7 to d811152316e4 (1 revision) (flutter/flutter#169473) 2025-05-26 munsocket@protonmail.com Precise browser resizing with integration_test and driver (flutter/flutter#160678) 2025-05-26 matanlurey@users.noreply.github.com Add `/coverage/` to `.gitignore.tmp` (flutter/flutter#169387) 2025-05-26 matanlurey@users.noreply.github.com Make test output with encoded `dart-defines=...` human readable. (flutter/flutter#169353) 2025-05-26 matanlurey@users.noreply.github.com Use at most `PROC~/2` tasks to transform assets. (flutter/flutter#169386) 2025-05-26 32538273+ValentinVignal@users.noreply.github.com Forward exit code from dart test to flutter test (flutter/flutter#168604) 2025-05-26 51940183+Sameri11@users.noreply.github.com Fix warning when building for macOS desktop (flutter/flutter#165996) 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
…r#9334) flutter/flutter@4372bfb...0e536eb 2025-05-28 30870216+gaaclarke@users.noreply.github.com Introduces FlutterPluginRegistrant protocol. (flutter/flutter#169399) 2025-05-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Initialize `default-flavor` in `FlutterCommand`, adds integration test. (#169298)" (flutter/flutter#169581) 2025-05-28 matanlurey@users.noreply.github.com Initialize `default-flavor` in `FlutterCommand`, adds integration test. (flutter/flutter#169298) 2025-05-28 jakemac@google.com Update DEPS to add dart-lang/ai repo (flutter/flutter#169540) 2025-05-28 engine-flutter-autoroll@skia.org Roll Skia from 92311f2ba0b7 to 82d326fc2148 (1 revision) (flutter/flutter#169552) 2025-05-28 kevmoo@users.noreply.github.com dev/bots: improve service worker test code (flutter/flutter#169231) 2025-05-28 magder@google.com Make Android team platform view TESTOWNERS (flutter/flutter#169297) 2025-05-28 engine-flutter-autoroll@skia.org Roll Skia from 044f58f78a73 to 92311f2ba0b7 (9 revisions) (flutter/flutter#169542) 2025-05-27 engine-flutter-autoroll@skia.org Roll Skia from 443f5257f382 to 044f58f78a73 (16 revisions) (flutter/flutter#169530) 2025-05-27 sokolovskyi.konstantin@gmail.com [web] Fix unresponsive input above SelectionArea in Safari and Firefox. (flutter/flutter#167275) 2025-05-27 58529443+srujzs@users.noreply.github.com Set pause_isolates_on_start flag if --start-paused (flutter/flutter#169392) 2025-05-27 engine-flutter-autoroll@skia.org Roll Packages from af0b9a9 to 6eebe72 (24 revisions) (flutter/flutter#169514) 2025-05-27 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 5mpmPsuD8rpeiJizT... to nC9hLWjYVlChDTEPh... (flutter/flutter#169498) 2025-05-27 zhongliu88889@gmail.com Split hint from label and expose it via aria-description or aria-describedby (flutter/flutter#169157) 2025-05-26 github@alexv525.com 🐛 Normalize generated file paths for the l10n generator (flutter/flutter#169467) 2025-05-26 engine-flutter-autoroll@skia.org Roll Dart SDK from d811152316e4 to 6aeb798bdbe2 (2 revisions) (flutter/flutter#169478) 2025-05-26 dkwingsmt@users.noreply.github.com [Cupertino] Apply RSuperellipse to most Cupertino widgets (flutter/flutter#167784) 2025-05-26 bkonyi@google.com Roll `package:dds` to 5.0.2 (flutter/flutter#169471) 2025-05-26 matanlurey@users.noreply.github.com Use `.flutter-plugins-dependencies` for crash reporting. (flutter/flutter#169319) 2025-05-26 matanlurey@users.noreply.github.com Remove now disabled code that would generate `.flutter-plugins`. (flutter/flutter#169320) 2025-05-26 engine-flutter-autoroll@skia.org Roll Dart SDK from 7dab9bffe1f7 to d811152316e4 (1 revision) (flutter/flutter#169473) 2025-05-26 munsocket@protonmail.com Precise browser resizing with integration_test and driver (flutter/flutter#160678) 2025-05-26 matanlurey@users.noreply.github.com Add `/coverage/` to `.gitignore.tmp` (flutter/flutter#169387) 2025-05-26 matanlurey@users.noreply.github.com Make test output with encoded `dart-defines=...` human readable. (flutter/flutter#169353) 2025-05-26 matanlurey@users.noreply.github.com Use at most `PROC~/2` tasks to transform assets. (flutter/flutter#169386) 2025-05-26 32538273+ValentinVignal@users.noreply.github.com Forward exit code from dart test to flutter test (flutter/flutter#168604) 2025-05-26 51940183+Sameri11@users.noreply.github.com Fix warning when building for macOS desktop (flutter/flutter#165996) 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
…r#9334) flutter/flutter@4372bfb...0e536eb 2025-05-28 30870216+gaaclarke@users.noreply.github.com Introduces FlutterPluginRegistrant protocol. (flutter/flutter#169399) 2025-05-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Initialize `default-flavor` in `FlutterCommand`, adds integration test. (#169298)" (flutter/flutter#169581) 2025-05-28 matanlurey@users.noreply.github.com Initialize `default-flavor` in `FlutterCommand`, adds integration test. (flutter/flutter#169298) 2025-05-28 jakemac@google.com Update DEPS to add dart-lang/ai repo (flutter/flutter#169540) 2025-05-28 engine-flutter-autoroll@skia.org Roll Skia from 92311f2ba0b7 to 82d326fc2148 (1 revision) (flutter/flutter#169552) 2025-05-28 kevmoo@users.noreply.github.com dev/bots: improve service worker test code (flutter/flutter#169231) 2025-05-28 magder@google.com Make Android team platform view TESTOWNERS (flutter/flutter#169297) 2025-05-28 engine-flutter-autoroll@skia.org Roll Skia from 044f58f78a73 to 92311f2ba0b7 (9 revisions) (flutter/flutter#169542) 2025-05-27 engine-flutter-autoroll@skia.org Roll Skia from 443f5257f382 to 044f58f78a73 (16 revisions) (flutter/flutter#169530) 2025-05-27 sokolovskyi.konstantin@gmail.com [web] Fix unresponsive input above SelectionArea in Safari and Firefox. (flutter/flutter#167275) 2025-05-27 58529443+srujzs@users.noreply.github.com Set pause_isolates_on_start flag if --start-paused (flutter/flutter#169392) 2025-05-27 engine-flutter-autoroll@skia.org Roll Packages from af0b9a9 to 6eebe72 (24 revisions) (flutter/flutter#169514) 2025-05-27 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 5mpmPsuD8rpeiJizT... to nC9hLWjYVlChDTEPh... (flutter/flutter#169498) 2025-05-27 zhongliu88889@gmail.com Split hint from label and expose it via aria-description or aria-describedby (flutter/flutter#169157) 2025-05-26 github@alexv525.com 🐛 Normalize generated file paths for the l10n generator (flutter/flutter#169467) 2025-05-26 engine-flutter-autoroll@skia.org Roll Dart SDK from d811152316e4 to 6aeb798bdbe2 (2 revisions) (flutter/flutter#169478) 2025-05-26 dkwingsmt@users.noreply.github.com [Cupertino] Apply RSuperellipse to most Cupertino widgets (flutter/flutter#167784) 2025-05-26 bkonyi@google.com Roll `package:dds` to 5.0.2 (flutter/flutter#169471) 2025-05-26 matanlurey@users.noreply.github.com Use `.flutter-plugins-dependencies` for crash reporting. (flutter/flutter#169319) 2025-05-26 matanlurey@users.noreply.github.com Remove now disabled code that would generate `.flutter-plugins`. (flutter/flutter#169320) 2025-05-26 engine-flutter-autoroll@skia.org Roll Dart SDK from 7dab9bffe1f7 to d811152316e4 (1 revision) (flutter/flutter#169473) 2025-05-26 munsocket@protonmail.com Precise browser resizing with integration_test and driver (flutter/flutter#160678) 2025-05-26 matanlurey@users.noreply.github.com Add `/coverage/` to `.gitignore.tmp` (flutter/flutter#169387) 2025-05-26 matanlurey@users.noreply.github.com Make test output with encoded `dart-defines=...` human readable. (flutter/flutter#169353) 2025-05-26 matanlurey@users.noreply.github.com Use at most `PROC~/2` tasks to transform assets. (flutter/flutter#169386) 2025-05-26 32538273+ValentinVignal@users.noreply.github.com Forward exit code from dart test to flutter test (flutter/flutter#168604) 2025-05-26 51940183+Sameri11@users.noreply.github.com Fix warning when building for macOS desktop (flutter/flutter#165996) 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
Summary
This PR improves web accessibility in Flutter by separating hint from label in the semantics engine and exposing them via ARIA attributes. The hint is set using aria-description (or aria-describedby as a fallback for browsers that do not support aria-description).
Details
Hint separation:
The hint is exposed via aria-description if supported, or via aria-describedby with a hidden node as a fallback.
Browser compatibility:
Uses feature detection to choose between aria-description and aria-describedby.
Test coverage:
Added/updated tests to verify correct ARIA attribute behavior and fallback logic.
How to verify
All relevant tests pass (semantics_test.dart, semantics_text_test.dart).
Motivation
This change brings Flutter web closer to accessibility best practices and ARIA standards, improving the experience for users of assistive technologies.
Before/After Change
before: https://before-change-hint.web.app/
after: https://after-change-hint.web.app/
after(fallback to aria-describedby): https://after-change-hint-fallback.web.app/
Issues fixed
#162140
Note
Some focus-related tests (e.g., incrementable sends focus events) are failing, but these failures are also present on the main branch and are unrelated to the ARIA label/hint changes in this PR.
Pre-launch Checklist
///
).