From a612a1dfd1fb8051e1feeb38e6aaf41e0d4db859 Mon Sep 17 00:00:00 2001 From: Ashish Myles Date: Fri, 2 Jun 2023 14:33:24 -0400 Subject: [PATCH] [web] Move announcement live elements to the end of the DOM and make them `div`s instead of `label`s. (flutter/engine#42432) - Moving them to the end prevents the screen reader from landing on them before the relevant content. - Making them `div`s instead of `label`s prevents some screen readers (ChromeVox in particular) from landing on the live elements when the live elements are empty. Fixes https://github.com/flutter/flutter/issues/127862. --- .../lib/web_ui/lib/src/engine/embedder.dart | 13 +++- .../web_ui/lib/src/engine/initialization.dart | 1 - .../lib/src/engine/platform_dispatcher.dart | 5 +- .../src/engine/semantics/accessibility.dart | 61 ++----------------- .../lib/src/engine/semantics/live_region.dart | 3 +- .../engine/semantics/accessibility_test.dart | 45 +++++--------- .../test/engine/semantics/semantics_test.dart | 12 +--- 7 files changed, 41 insertions(+), 99 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/embedder.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/embedder.dart index e79254fe57b..e949fdd0d2e 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/embedder.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/embedder.dart @@ -135,6 +135,9 @@ class FlutterViewEmbedder { DomElement get textEditingHostNode => _textEditingHostNode; late DomElement _textEditingHostNode; + AccessibilityAnnouncements get accessibilityAnnouncements => _accessibilityAnnouncements; + late AccessibilityAnnouncements _accessibilityAnnouncements; + static const String defaultFontStyle = 'normal'; static const String defaultFontWeight = 'normal'; static const double defaultFontSize = 14; @@ -163,7 +166,6 @@ class FlutterViewEmbedder { _flutterViewElement = domDocument.createElement(flutterViewTagName); _glassPaneElement = domDocument.createElement(glassPaneTagName); - // This must be attached to the DOM now, so the engine can create a host // node (ShadowDOM or a fallback) next. // @@ -216,8 +218,12 @@ class FlutterViewEmbedder { .instance.semanticsHelper .prepareAccessibilityPlaceholder(); + final DomElement announcementsElement = createDomElement('flt-announcement-host'); + _accessibilityAnnouncements = AccessibilityAnnouncements(hostElement: announcementsElement); + shadowRoot.append(accessibilityPlaceholder); shadowRoot.append(_sceneHostElement!); + shadowRoot.append(announcementsElement); // The semantic host goes last because hit-test order-wise it must be // first. If semantics goes under the scene host, platform views will @@ -246,6 +252,11 @@ class FlutterViewEmbedder { window.onResize.listen(_metricsDidChange); } + /// For tests only. + void debugOverrideAccessibilityAnnouncements(AccessibilityAnnouncements override) { + _accessibilityAnnouncements = override; + } + /// The framework specifies semantics in physical pixels, but CSS uses /// logical pixels. To compensate, an inverse scale is injected at the root /// level. diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/initialization.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/initialization.dart index bf89ca81c32..c4fb28b82b1 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/initialization.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/initialization.dart @@ -223,7 +223,6 @@ Future initializeEngineUi() async { } _initializationState = DebugEngineInitializationState.initializingUi; - initializeAccessibilityAnnouncements(); RawKeyboard.initialize(onMacOs: operatingSystem == OperatingSystem.macOs); MouseCursor.initialize(); ensureFlutterViewEmbedderInitialized(); diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 0d54cba677d..4101440a22a 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -11,10 +11,9 @@ import 'package:ui/src/engine/canvaskit/renderer.dart'; import 'package:ui/src/engine/renderer.dart'; import 'package:ui/ui.dart' as ui; -import '../engine.dart' show platformViewManager, registerHotRestartListener; +import '../engine.dart' show flutterViewEmbedder, platformViewManager, registerHotRestartListener; import 'clipboard.dart'; import 'dom.dart'; -import 'embedder.dart'; import 'mouse_cursor.dart'; import 'platform_views/message_handler.dart'; import 'plugins.dart'; @@ -645,7 +644,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { case 'flutter/accessibility': // In widget tests we want to bypass processing of platform messages. const StandardMessageCodec codec = StandardMessageCodec(); - accessibilityAnnouncements.handleMessage(codec, data); + flutterViewEmbedder.accessibilityAnnouncements.handleMessage(codec, data); replyToPlatformMessage(callback, codec.encodeMessage(true)); return; diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/accessibility.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/accessibility.dart index 216b7615964..c4803098ee0 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/accessibility.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/accessibility.dart @@ -5,7 +5,6 @@ import 'dart:async'; import 'dart:typed_data'; -import '../../engine.dart' show registerHotRestartListener; import '../dom.dart'; import '../services.dart'; import '../util.dart'; @@ -20,37 +19,6 @@ enum Assertiveness { assertive, } -/// Singleton for accessing accessibility announcements from the platform. -AccessibilityAnnouncements get accessibilityAnnouncements { - assert( - _accessibilityAnnouncements != null, - 'AccessibilityAnnouncements not initialized. Call initializeAccessibilityAnnouncements() to initialize it.', - ); - return _accessibilityAnnouncements!; -} -AccessibilityAnnouncements? _accessibilityAnnouncements; - -void debugOverrideAccessibilityAnnouncements(AccessibilityAnnouncements override) { - _accessibilityAnnouncements = override; -} - -/// Initializes the [accessibilityAnnouncements] singleton. -/// -/// It is an error to attempt to initialize the singleton more than once. Call -/// [AccessibilityAnnouncements.dispose] prior to calling this function again. -void initializeAccessibilityAnnouncements() { - assert( - _accessibilityAnnouncements == null, - 'AccessibilityAnnouncements is already initialized. This is likely a bug in ' - 'Flutter Web engine initialization. Please file an issue at ' - 'https://github.com/flutter/flutter/issues/new/choose', - ); - _accessibilityAnnouncements = AccessibilityAnnouncements(); - registerHotRestartListener(() { - accessibilityAnnouncements.dispose(); - }); -} - /// Duration for which a live message will be present in the DOM for the screen /// reader to announce it. /// @@ -65,11 +33,11 @@ void setLiveMessageDurationForTest(Duration duration) { /// Makes accessibility announcements using `aria-live` DOM elements. class AccessibilityAnnouncements { /// Creates a new instance with its own DOM elements used for announcements. - factory AccessibilityAnnouncements() { + factory AccessibilityAnnouncements({required DomElement hostElement}) { final DomHTMLElement politeElement = _createElement(Assertiveness.polite); final DomHTMLElement assertiveElement = _createElement(Assertiveness.assertive); - domDocument.body!.append(politeElement); - domDocument.body!.append(assertiveElement); + hostElement.append(politeElement); + hostElement.append(assertiveElement); return AccessibilityAnnouncements._(politeElement, assertiveElement); } @@ -85,32 +53,17 @@ class AccessibilityAnnouncements { /// Looks up the element used to announce messages of the given [assertiveness]. DomHTMLElement ariaLiveElementFor(Assertiveness assertiveness) { - assert(!_isDisposed); switch (assertiveness) { case Assertiveness.polite: return _politeElement; case Assertiveness.assertive: return _assertiveElement; } } - bool _isDisposed = false; - - /// Disposes of the resources used by this object. - /// - /// This object's methods must not be called after calling this method. - void dispose() { - assert(!_isDisposed); - _isDisposed = true; - _politeElement.remove(); - _assertiveElement.remove(); - _accessibilityAnnouncements = null; - } - /// Makes an accessibity announcement from a message sent by the framework /// over the 'flutter/accessibility' channel. /// /// The encoded message is passed as [data], and will be decoded using [codec]. void handleMessage(StandardMessageCodec codec, ByteData? data) { - assert(!_isDisposed); final Map inputMap = codec.decodeMessage(data) as Map; final Map dataMap = inputMap.readDynamicJson('data'); final String? message = dataMap.tryString('message'); @@ -128,19 +81,17 @@ class AccessibilityAnnouncements { /// /// [assertiveness] controls how interruptive the announcement is. void announce(String message, Assertiveness assertiveness) { - assert(!_isDisposed); final DomHTMLElement ariaLiveElement = ariaLiveElementFor(assertiveness); - final DomElement messageElement = createDomElement('div'); + final DomHTMLDivElement messageElement = createDomHTMLDivElement(); messageElement.text = message; ariaLiveElement.append(messageElement); Timer(liveMessageDuration, () => messageElement.remove()); } - static DomHTMLLabelElement _createElement(Assertiveness assertiveness) { + static DomHTMLElement _createElement(Assertiveness assertiveness) { final String ariaLiveValue = (assertiveness == Assertiveness.assertive) ? 'assertive' : 'polite'; - final DomHTMLLabelElement liveRegion = createDomHTMLLabelElement(); - liveRegion.setAttribute('id', 'ftl-announcement-$ariaLiveValue'); + final DomHTMLElement liveRegion = createDomElement('flt-announcement-$ariaLiveValue') as DomHTMLElement; liveRegion.style ..position = 'fixed' ..overflow = 'hidden' diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/live_region.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/live_region.dart index 35685cfb0ec..1be14a49e75 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/live_region.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/live_region.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import '../embedder.dart' show flutterViewEmbedder; import 'accessibility.dart'; import 'semantics.dart'; @@ -25,7 +26,7 @@ class LiveRegion extends RoleManager { if (_lastAnnouncement != semanticsObject.label) { _lastAnnouncement = semanticsObject.label; if (semanticsObject.hasLabel) { - accessibilityAnnouncements.announce( + flutterViewEmbedder.accessibilityAnnouncements.announce( _lastAnnouncement! , Assertiveness.polite ); } diff --git a/engine/src/flutter/lib/web_ui/test/engine/semantics/accessibility_test.dart b/engine/src/flutter/lib/web_ui/test/engine/semantics/accessibility_test.dart index 91972cd3b91..f39236b8c9e 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/semantics/accessibility_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/semantics/accessibility_test.dart @@ -8,7 +8,7 @@ import 'dart:typed_data'; import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; import 'package:ui/src/engine/dom.dart'; -import 'package:ui/src/engine/initialization.dart'; +import 'package:ui/src/engine/embedder.dart'; import 'package:ui/src/engine/semantics.dart'; import 'package:ui/src/engine/services.dart'; @@ -19,42 +19,29 @@ void main() { } void testMain() { - setUpAll(() async { - await initializeEngine(); + late FlutterViewEmbedder embedder; + late AccessibilityAnnouncements accessibilityAnnouncements; + + setUp(() { + embedder = FlutterViewEmbedder(); + accessibilityAnnouncements = embedder.accessibilityAnnouncements; setLiveMessageDurationForTest(const Duration(milliseconds: 10)); + expect( + embedder.glassPaneShadow.querySelector('flt-announcement-polite'), + accessibilityAnnouncements.ariaLiveElementFor(Assertiveness.polite), + ); + expect( + embedder.glassPaneShadow.querySelector('flt-announcement-assertive'), + accessibilityAnnouncements.ariaLiveElementFor(Assertiveness.assertive), + ); }); - void expectAnnouncementElements({required bool present}) { - expect( - domDocument.getElementById('ftl-announcement-polite'), - present ? isNotNull : isNull, - ); - expect( - domDocument.getElementById('ftl-announcement-assertive'), - present ? isNotNull : isNull, - ); - } - tearDown(() async { - // Completely reset accessibility announcements for subsequent tests. - accessibilityAnnouncements.dispose(); await Future.delayed(liveMessageDuration * 2); - initializeAccessibilityAnnouncements(); - expectAnnouncementElements(present: true); + embedder.glassPaneElement.remove(); }); group('$AccessibilityAnnouncements', () { - test('Initialization and disposal', () { - // Elements should be there right after engine initialization. - expectAnnouncementElements(present: true); - - accessibilityAnnouncements.dispose(); - expectAnnouncementElements(present: false); - - initializeAccessibilityAnnouncements(); - expectAnnouncementElements(present: true); - }); - ByteData? encodeMessageOnly({required String message}) { return codec.encodeMessage({ 'data': {'message': message}, diff --git a/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart b/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart index 1ea261d7af4..89fe404a3ea 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -2122,12 +2122,6 @@ class MockAccessibilityAnnouncements implements AccessibilityAnnouncements { 'ariaLiveElementFor is not supported in MockAccessibilityAnnouncements'); } - @override - void dispose() { - throw UnsupportedError( - 'dispose is not supported in MockAccessibilityAnnouncements!'); - } - @override void handleMessage(StandardMessageCodec codec, ByteData? data) { throw UnsupportedError( @@ -2143,7 +2137,7 @@ void _testLiveRegion() { final MockAccessibilityAnnouncements mockAccessibilityAnnouncements = MockAccessibilityAnnouncements(); - debugOverrideAccessibilityAnnouncements(mockAccessibilityAnnouncements); + flutterViewEmbedder.debugOverrideAccessibilityAnnouncements(mockAccessibilityAnnouncements); final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); updateNode( @@ -2166,7 +2160,7 @@ void _testLiveRegion() { final MockAccessibilityAnnouncements mockAccessibilityAnnouncements = MockAccessibilityAnnouncements(); - debugOverrideAccessibilityAnnouncements(mockAccessibilityAnnouncements); + flutterViewEmbedder.debugOverrideAccessibilityAnnouncements(mockAccessibilityAnnouncements); final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); updateNode( @@ -2188,7 +2182,7 @@ void _testLiveRegion() { final MockAccessibilityAnnouncements mockAccessibilityAnnouncements = MockAccessibilityAnnouncements(); - debugOverrideAccessibilityAnnouncements(mockAccessibilityAnnouncements); + flutterViewEmbedder.debugOverrideAccessibilityAnnouncements(mockAccessibilityAnnouncements); ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); updateNode(