[web] Update a11y announcements to append divs instead of setting content. by marcianx · Pull Request #42258 · flutter/engine

marcianx · 2023-05-23T20:01:34Z

This also removes the appended divs after a short time so that screen readers don't navigate to it, especially when users are entering the DOM to enable accessiblity.

Fixes flutter/flutter#127335.

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] and the [C++, Objective-C, Java style guides].
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the [CLA].
All existing and new tests are passing.

…tent. This also removes the appended divs after a short time so that screen readers don't navigate to it, especially when users are entering the DOM to enable accessiblity. Fixes #127335.

yjbanov · 2023-05-23T20:38:14Z

lib/web_ui/lib/src/engine/semantics/accessibility.dart


+/// Duration for which a live message will be present in the DOM for the screen
+/// reader to announce it.
+const Duration liveMessageDuration = Duration(milliseconds: 300);


Can you please add a comment explaining the 300 value? Even if it's just "I tried with X, Y, and Z screen readers and they were all happy with this value". This would provide some context to a future maintainer who may doubt this value in the future for whatever reason.

… live region.

yjbanov · 2023-05-23T20:51:45Z

lib/web_ui/lib/src/engine/semantics/accessibility.dart


+/// Duration for which a live message will be present in the DOM for the screen
+/// reader to announce it.
+const Duration liveMessageDuration = Duration(milliseconds: 300);


Instead of making this a const can this be mutable so that tests can set a smaller duration than 300ms? I don't know how many tests we currently have, but if the union of all tests have to await on this 10 times, it lengthens the runtime of the test by 3 seconds. I think we could use 10 times smaller delays in the tests.

yjbanov

auto-submit · 2023-05-23T22:19:09Z

auto label is removed for flutter/engine, pr: 42258, due to This PR has not met approval requirements for merging. You are not a member of flutter-hackers and need 1 more review(s) in order to merge this PR.

Merge guidelines: You need at least one approved review if you are already part of flutter-hackers or two member reviews if you are not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

htoor3

Just left some minor test comments, but lgtm otherwise

htoor3 · 2023-05-24T19:20:13Z

lib/web_ui/test/engine/semantics/accessibility_test.dart

-      accessibilityAnnouncements.handleMessage(codec, codec.encodeMessage(testInput));
-      expect(accessibilityAnnouncements.ariaLiveElementFor(Assertiveness.polite).text, 'polite message');
-      expect(accessibilityAnnouncements.ariaLiveElementFor(Assertiveness.assertive).text, '');
+    test('aria-live is polite when assertiveness is set to 0', () async {


Do we know/care what happens if assertiveness is a negative int? Or is this not a scenario we'll run into

It's not a scenario we should run into - it would give an index out of bounds. This is a serialization of the Assertiveness enum. It's converted back to the enum using Assertiveness.values[deserializedAssertiveness]:

engine/lib/web_ui/lib/src/engine/semantics/accessibility.dart

Line 108 in df37f0c

final Assertiveness assertiveness = Assertiveness.values[assertivenessIndex];

htoor3 · 2023-05-24T19:21:36Z

lib/web_ui/test/engine/semantics/accessibility_test.dart

-      accessibilityAnnouncements.handleMessage(codec, codec.encodeMessage(testInput3));
-      expect(accessibilityAnnouncements.ariaLiveElementFor(Assertiveness.polite).text, 'Hello');
-      expect(accessibilityAnnouncements.ariaLiveElementFor(Assertiveness.assertive).text, '');
+    test('Rapid-fire messages are each announced.', () async {


Does this behavior change if mixed assertive/polite rapid-fire messages are sent, rather than just polite?

Nope. Nothing fundamentally changes. They go into different live divs, each of which is handled independently. These tests ensure that multiple messages of the same type don't clobber each other in any way.

…ting content. (flutter/engine#42258)

lib/web_ui/test/engine/semantics/accessibility_test.dart

…ting content. (flutter/engine#42258)

flutter/engine@c641f63...9ba461e 2023-05-24 [email protected] [web] Remove comment about dart:html migration (flutter/engine#42290) 2023-05-24 [email protected] [web] Hide JS types from dart:ui_web (flutter/engine#42252) 2023-05-24 [email protected] Roll Fuchsia Mac SDK from qoLy9E5PjnAlICjUb... to RSSC61ubl9JXmn4JO... (flutter/engine#42287) 2023-05-24 [email protected] [web] Update a11y announcements to append divs instead of setting content. (flutter/engine#42258) 2023-05-24 [email protected] [Impeller] fix Xcode frame capture. (flutter/engine#42289) 2023-05-24 [email protected] [Impeller] Create an autorelease pool for Impeller tests running on macOS. (flutter/engine#42265) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from qoLy9E5PjnAl to RSSC61ubl9JX If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

flutter-dashboard bot added the platform-web Code specifically for the web engine label May 23, 2023

[web] Update a11y announcements to append divs instead of setting con…

db0412c

…tent. This also removes the appended divs after a short time so that screen readers don't navigate to it, especially when users are entering the DOM to enable accessiblity. Fixes #127335.

marcianx force-pushed the main branch from 32a00d4 to db0412c Compare May 23, 2023 20:23

yjbanov reviewed May 23, 2023

View reviewed changes

Document rationale for length of delay before message is cleared from…

5af78f6

… live region.

yjbanov reviewed May 23, 2023

View reviewed changes

Make [liveMessageDuration] settable to reduce delays for tests.

ac5c7bb

yjbanov approved these changes May 23, 2023

View reviewed changes

yjbanov added the autosubmit Merge PR when tree becomes green via auto submit App label May 23, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 23, 2023

htoor3 approved these changes May 24, 2023

View reviewed changes

yjbanov added the autosubmit Merge PR when tree becomes green via auto submit App label May 24, 2023

mdebbar approved these changes May 24, 2023

View reviewed changes

auto-submit bot merged commit 6e55f46 into flutter:main May 24, 2023

engine-flutter-autoroll mentioned this pull request May 24, 2023

Roll Flutter Engine from c641f6307f08 to 50bfa8115c41 (5 revisions) flutter/flutter#127532

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 24, 2023

6e55f46 [web] Update a11y announcements to append divs instead of set…

8e1f660

…ting content. (flutter/engine#42258)

engine-flutter-autoroll mentioned this pull request May 24, 2023

Roll Flutter Engine from c641f6307f08 to 9ba461efd3fe (6 revisions) flutter/flutter#127536

Merged

ditman reviewed May 24, 2023

View reviewed changes

lib/web_ui/test/engine/semantics/accessibility_test.dart Show resolved Hide resolved

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 24, 2023

6e55f46 [web] Update a11y announcements to append divs instead of set…

e3d30f0

…ting content. (flutter/engine#42258)

tugorez mentioned this pull request Jan 25, 2024

Flutter Web Semantics does not announce the same announcement multiple times in a row flutter/flutter#142250

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[web] Update a11y announcements to append divs instead of setting content.#42258

[web] Update a11y announcements to append divs instead of setting content.#42258
auto-submit[bot] merged 3 commits intoflutter:mainfrom
marcianx:main

marcianx commented May 23, 2023 •

edited by yjbanov

Loading

Uh oh!

yjbanov May 23, 2023

Uh oh!

marcianx May 23, 2023

Uh oh!

yjbanov May 23, 2023

Uh oh!

marcianx May 23, 2023

Uh oh!

yjbanov left a comment

Uh oh!

auto-submit bot commented May 23, 2023

Uh oh!

htoor3 left a comment

Uh oh!

htoor3 May 24, 2023

Uh oh!

marcianx May 24, 2023 •

edited

Loading

Uh oh!

htoor3 May 24, 2023

Uh oh!

marcianx May 24, 2023

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

marcianx commented May 23, 2023 • edited by yjbanov Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

yjbanov May 23, 2023

Choose a reason for hiding this comment

Uh oh!

marcianx May 23, 2023

Choose a reason for hiding this comment

Uh oh!

yjbanov May 23, 2023

Choose a reason for hiding this comment

Uh oh!

marcianx May 23, 2023

Choose a reason for hiding this comment

Uh oh!

yjbanov left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented May 23, 2023

Uh oh!

htoor3 left a comment

Choose a reason for hiding this comment

Uh oh!

htoor3 May 24, 2023

Choose a reason for hiding this comment

Uh oh!

marcianx May 24, 2023 • 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!

htoor3 May 24, 2023

Choose a reason for hiding this comment

Uh oh!

marcianx May 24, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

marcianx commented May 23, 2023 •

edited by yjbanov

Loading

marcianx May 24, 2023 •

edited

Loading