diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/checkable.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/checkable.dart index 7ee0a067c9..8704c771b2 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/checkable.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/checkable.dart @@ -84,6 +84,7 @@ class Checkable extends RoleManager { @override void dispose() { + super.dispose(); switch (_kind) { case _CheckableKind.checkbox: semanticsObject.setAriaRole('checkbox', false); diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/dialog.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/dialog.dart index 5f9c8bb869..df2c743d4a 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/dialog.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/dialog.dart @@ -13,26 +13,86 @@ class Dialog extends RoleManager { Dialog(SemanticsObject semanticsObject) : super(Role.dialog, semanticsObject); @override - void dispose() { - semanticsObject.element.removeAttribute('aria-label'); - semanticsObject.clearAriaRole(); + void update() { + // If semantic object corresponding to the dialog also provides the label + // for itself it is applied as `aria-label`. See also [describeBy]. + if (semanticsObject.namesRoute) { + final String? label = semanticsObject.label; + assert(() { + if (label == null || label.trim().isEmpty) { + printWarning( + 'Semantic node ${semanticsObject.id} had both scopesRoute and ' + 'namesRoute set, indicating a self-labelled dialog, but it is ' + 'missing the label. A dialog should be labelled either by setting ' + 'namesRoute on itself and providing a label, or by containing a ' + 'child node with namesRoute that can describe it with its content.' + ); + } + return true; + }()); + semanticsObject.element.setAttribute('aria-label', label ?? ''); + semanticsObject.setAriaRole('dialog', true); + } } + /// Sets the description of this dialog based on a [RouteName] descendant + /// node, unless the dialog provides its own label. + void describeBy(RouteName routeName) { + if (semanticsObject.namesRoute) { + // The dialog provides its own label, which takes precedence. + return; + } + + semanticsObject.setAriaRole('dialog', true); + semanticsObject.element.setAttribute( + 'aria-describedby', + routeName.semanticsObject.element.id, + ); + } +} + +/// Supplies a description for the nearest ancestor [Dialog]. +class RouteName extends RoleManager { + RouteName(SemanticsObject semanticsObject) : super(Role.routeName, semanticsObject); + + Dialog? _dialog; + @override void update() { - final String? label = semanticsObject.label; - assert(() { - if (label == null || label.trim().isEmpty) { - printWarning( - 'Semantic node ${semanticsObject.id} was assigned dialog role, but ' - 'is missing a label. A dialog should contain a label so that a ' - 'screen reader can communicate to the user that a dialog appeared ' - 'and a user action is requested.' - ); + // NOTE(yjbanov): this does not handle the case when the node structure + // changes such that this RouteName is no longer attached to the same + // dialog. While this is technically expressible using the semantics API, + // after discussing this case with customers I decided that this case is not + // interesting enough to support. A tree restructure like this is likely to + // confuse screen readers, and it would add complexity to the engine's + // semantics code. Since reparenting can be done with no update to either + // the Dialog or RouteName we'd have to scan intermediate nodes for + // structural changes. + if (semanticsObject.isLabelDirty) { + final Dialog? dialog = _dialog; + if (dialog != null) { + // Already attached to a dialog, just update the description. + dialog.describeBy(this); + } else { + // Setting the label for the first time. Wait for the DOM tree to be + // established, then find the nearest dialog and update its label. + semanticsObject.owner.addOneTimePostUpdateCallback(() { + if (!isDisposed) { + _lookUpNearestAncestorDialog(); + _dialog?.describeBy(this); + } + }); } - return true; - }()); - semanticsObject.element.setAttribute('aria-label', label ?? ''); - semanticsObject.setAriaRole('dialog', true); + } + } + + void _lookUpNearestAncestorDialog() { + SemanticsObject? parent = semanticsObject.parent; + while (parent != null && !parent.hasRole(Role.dialog)) { + parent = parent.parent; + } + if (parent != null && parent.hasRole(Role.dialog)) { + _dialog = parent.getRole(Role.dialog); + } } } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart index 2f1e871280..4de57af1c6 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/focusable.dart @@ -43,6 +43,7 @@ class Focusable extends RoleManager { @override void dispose() { + super.dispose(); _focusManager.stopManaging(); } } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/image.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/image.dart index e1a4de392d..e867544439 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/image.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/image.dart @@ -73,6 +73,7 @@ class ImageRoleManager extends RoleManager { @override void dispose() { + super.dispose(); _cleanUpAuxiliaryElement(); _cleanupElement(); } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart index b75cd06476..f58e2a8edd 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart @@ -140,6 +140,7 @@ class Incrementable extends RoleManager { @override void dispose() { assert(_gestureModeListener != null); + super.dispose(); _focusManager.stopManaging(); semanticsObject.owner.removeGestureModeListener(_gestureModeListener); _gestureModeListener = null; diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/label_and_value.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/label_and_value.dart index 748ec58f08..087b30dc83 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/label_and_value.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/label_and_value.dart @@ -102,6 +102,7 @@ class LabelAndValue extends RoleManager { @override void dispose() { + super.dispose(); _cleanUpDom(); } } 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 2d17cff0a1..35685cfb0e 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 @@ -31,8 +31,4 @@ class LiveRegion extends RoleManager { } } } - - @override - void dispose() { - } } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart index cafd8cbfff..dd0d66bb6d 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart @@ -224,6 +224,7 @@ class Scrollable extends RoleManager { @override void dispose() { + super.dispose(); final DomCSSStyleDeclaration style = semanticsObject.element.style; assert(_gestureModeListener != null); style.removeProperty('overflowY'); diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart index 942a505b05..51044508fc 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -5,6 +5,7 @@ import 'dart:math' as math; import 'dart:typed_data'; +import 'package:meta/meta.dart'; import 'package:ui/ui.dart' as ui; import '../../engine.dart' show registerHotRestartListener; @@ -365,15 +366,35 @@ enum Role { /// Adds the "dialog" ARIA role to the node. /// - /// This corresponds to a semantics node that has both `scopesRoute` and - /// `namesRoute` bits set. While in Flutter a named route is not necessarily a - /// dialog, this is the closest analog on the web. + /// This corresponds to a semantics node that has `scopesRoute` bit set. While + /// in Flutter a named route is not necessarily a dialog, this is the closest + /// analog on the web. /// - /// Why is `scopesRoute` alone not sufficient? Because Flutter can create - /// routes that are not logically dialogs and there's nothing interesting to - /// announce to the user. For example, a modal barrier has `scopesRoute` set - /// but marking it as a dialog would be wrong. + /// There are 3 possible situations: + /// + /// * The node also has the `namesRoute` bit set. This means that the node's + /// `label` describes the dialog, which can be expressed by adding the + /// `aria-label` attribute. + /// * A descendant node has the `namesRoute` bit set. This means that the + /// child's content describes the dialog. The child may simply be labelled, + /// or it may be a subtree of nodes that describe the dialog together. The + /// nearest HTML equivalent is `aria-describedby`. The child acquires the + /// [routeName] role, which manages the relevant ARIA attributes. + /// * There is no `namesRoute` bit anywhere in the sub-tree rooted at the + /// current node. In this case it's likely not a dialog at all, and the node + /// should not get a label or the "dialog" role. It's just a group of + /// children. For example, a modal barrier has `scopesRoute` set but marking + /// it as a dialog would be wrong. dialog, + + /// Provides a description for an ancestor dialog. + /// + /// This role is assigned to nodes that have `namesRoute` set but not + /// `scopesRoute`. When both flags are set the node only gets the dialog + /// role (see [dialog]). + /// + /// If the ancestor dialog is missing, this role does nothing useful. + routeName, } /// A function that creates a [RoleManager] for a [SemanticsObject]. @@ -390,6 +411,7 @@ final Map _roleFactories = { Role.image: (SemanticsObject object) => ImageRoleManager(object), Role.liveRegion: (SemanticsObject object) => LiveRegion(object), Role.dialog: (SemanticsObject object) => Dialog(object), + Role.routeName: (SemanticsObject object) => RouteName(object), }; /// Provides the functionality associated with the role of the given @@ -416,6 +438,10 @@ abstract class RoleManager { /// minimum DOM updates. void update(); + /// Whether this role manager was disposed of. + bool get isDisposed => _isDisposed; + bool _isDisposed = false; + /// Called when [semanticsObject] is removed, or when it changes its role such /// that this role is no longer relevant. /// @@ -423,7 +449,10 @@ abstract class RoleManager { /// DOM. In particular, this method is the appropriate place to call /// [EngineSemanticsOwner.removeGestureModeListener] if this role reponds to /// gesture mode changes. - void dispose(); + @mustCallSuper + void dispose() { + _isDisposed = true; + } } /// Instantiation of a framework-side semantics node in the DOM. @@ -827,6 +856,15 @@ class SemanticsObject { DomElement? _childContainerElement; /// The parent of this semantics object. + /// + /// This value is not final until the tree is finalized. It is not safe to + /// rely on this value in the middle of a semantics tree update. It is safe to + /// use this value in post-update callback (see [SemanticsUpdatePhase] and + /// [EngineSemanticsOwner.addOneTimePostUpdateCallback]). + SemanticsObject? get parent { + assert(owner.phase == SemanticsUpdatePhase.postUpdate); + return _parent; + } SemanticsObject? _parent; /// Whether this node currently has a given [SemanticsFlag]. @@ -881,14 +919,15 @@ class SemanticsObject { !hasAction(ui.SemanticsAction.tap) && !hasFlag(ui.SemanticsFlag.isButton); - /// Whether this node should be treated as an ARIA dialog. + /// Whether this node defines a scope for a route. /// /// See also [Role.dialog]. - bool get isDialog { - final bool scopesRoute = hasFlag(ui.SemanticsFlag.scopesRoute); - final bool namesRoute = hasFlag(ui.SemanticsFlag.namesRoute); - return scopesRoute && namesRoute; - } + bool get scopesRoute => hasFlag(ui.SemanticsFlag.scopesRoute); + + /// Whether this node describes a route. + /// + /// See also [Role.dialog]. + bool get namesRoute => hasFlag(ui.SemanticsFlag.namesRoute); /// Whether this object carry enabled/disabled state (and if so whether it is /// enabled). @@ -1276,7 +1315,20 @@ class SemanticsObject { /// spec: /// /// > A map literal is ordered: iterating over the keys and/or values of the maps always happens in the order the keys appeared in the source code. - final Map _roleManagers = {}; + final Map _roleManagers = {}; + + /// The mapping of roles to role managers. + /// + /// This getter is only meant for testing. + Map get debugRoleManagers => _roleManagers; + + /// Returns if this node has the given [role]. + bool hasRole(Role role) => _roleManagers.containsKey(role); + + /// Returns the role manager for the given [role] attached to this node. + /// + /// If [hasRole] is false for the given [role], throws an error. + R getRole(Role role) => _roleManagers[role]! as R; /// Returns the role manager for the given [role]. /// @@ -1287,10 +1339,11 @@ class SemanticsObject { /// the lifecycles of [RoleManager] objects. void _updateRoles() { // Some role managers manage labels themselves for various role-specific reasons. - final bool managesOwnLabel = isTextField || isDialog || isVisualOnly; + final bool managesOwnLabel = isTextField || scopesRoute || isVisualOnly; _updateRole(Role.labelAndValue, (hasLabel || hasValue || hasTooltip) && !managesOwnLabel); - _updateRole(Role.dialog, isDialog); + _updateRole(Role.dialog, scopesRoute); + _updateRole(Role.routeName, namesRoute && !scopesRoute); _updateRole(Role.textField, isTextField); // The generic `Focusable` role manager can be used for everything except @@ -1500,6 +1553,30 @@ enum GestureMode { browserGestures, } +/// The current phase of the semantic update. +enum SemanticsUpdatePhase { + /// No update is in progress. + /// + /// When the semantics owner receives an update, it enters the [updating] + /// phase from the idle phase. + idle, + + /// Updating individual [SemanticsObject] nodes by calling + /// [RoleManager.update] and fixing parent-child relationships. + /// + /// After this phase is done, the owner enters the [postUpdate] phase. + updating, + + /// Post-update callbacks are being called. + /// + /// At this point all nodes have been updated, the parent child hierarchy has + /// been established, the DOM tree is in sync with the semantics tree, and + /// [RoleManager.dispose] has been called on removed nodes. + /// + /// After this phase is done, the owner switches back to [idle]. + postUpdate, +} + /// The top-level service that manages everything semantics-related. class EngineSemanticsOwner { EngineSemanticsOwner._() { @@ -1528,6 +1605,10 @@ class EngineSemanticsOwner { _instance = null; } + /// The current update phase of this semantics owner. + SemanticsUpdatePhase get phase => _phase; + SemanticsUpdatePhase _phase = SemanticsUpdatePhase.idle; + final Map _semanticsTree = {}; /// Map [SemanticsObject.id] to parent [SemanticsObject] it was attached to @@ -1604,11 +1685,16 @@ class EngineSemanticsOwner { _detachments = []; _attachments = {}; - if (_oneTimePostUpdateCallbacks.isNotEmpty) { - for (final ui.VoidCallback callback in _oneTimePostUpdateCallbacks) { - callback(); + _phase = SemanticsUpdatePhase.postUpdate; + try { + if (_oneTimePostUpdateCallbacks.isNotEmpty) { + for (final ui.VoidCallback callback in _oneTimePostUpdateCallbacks) { + callback(); + } + _oneTimePostUpdateCallbacks = []; } - _oneTimePostUpdateCallbacks = []; + } finally { + _phase = SemanticsUpdatePhase.idle; } } @@ -1876,6 +1962,7 @@ class EngineSemanticsOwner { } } + _phase = SemanticsUpdatePhase.updating; final SemanticsUpdate update = uiUpdate as SemanticsUpdate; // First, update each object's information about itself. This information is diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/tappable.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/tappable.dart index 16f4be118f..9352e4c52a 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/tappable.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/tappable.dart @@ -66,6 +66,7 @@ class Tappable extends RoleManager { @override void dispose() { + super.dispose(); _stopListening(); semanticsObject.setAriaRole('button', false); } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/text_field.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/text_field.dart index c2244378a7..f6b739925c 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/text_field.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/text_field.dart @@ -450,6 +450,7 @@ class TextField extends RoleManager { @override void dispose() { + super.dispose(); _positionInputElementTimer?.cancel(); _positionInputElementTimer = null; // on iOS, the `blur` event listener callback will remove the element. 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 d09ab4ef64..026a8b6cb5 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 @@ -441,6 +441,88 @@ void _testEngineSemanticsOwner() { mockSemanticsEnabler.shouldEnableSemanticsReturnValue = true; expect(semantics().receiveGlobalEvent(pointerEvent), isTrue); }); + + test('semantics owner update phases', () async { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + expect( + reason: 'Should start in idle phase', + semantics().phase, + SemanticsUpdatePhase.idle, + ); + + void pumpSemantics({ required String label }) { + final SemanticsTester tester = SemanticsTester(semantics()); + tester.updateNode( + id: 0, + children: [ + tester.updateNode(id: 1, label: label), + ], + ); + tester.apply(); + } + + SemanticsUpdatePhase? capturedPostUpdateCallbackPhase; + semantics().addOneTimePostUpdateCallback(() { + capturedPostUpdateCallbackPhase = semantics().phase; + }); + + pumpSemantics(label: 'Hello'); + + final SemanticsObject semanticsObject = semantics().debugSemanticsTree![1]!; + + expect( + reason: 'Should be in postUpdate phase while calling post-update callbacks', + capturedPostUpdateCallbackPhase, + SemanticsUpdatePhase.postUpdate, + ); + expect( + reason: 'After the update is done, should go back to idle', + semantics().phase, + SemanticsUpdatePhase.idle, + ); + + // Rudely replace the role manager with a mock, and trigger an update. + final MockRoleManager mockRoleManager = MockRoleManager(Role.labelAndValue, semanticsObject); + semanticsObject.debugRoleManagers[Role.labelAndValue] = mockRoleManager; + + pumpSemantics(label: 'World'); + + expect( + reason: 'While updating must be in SemanticsUpdatePhase.updating phase', + mockRoleManager.log, + [ + (method: 'update', phase: SemanticsUpdatePhase.updating), + ], + ); + + semantics().semanticsEnabled = false; + }); +} + +typedef MockRoleManagerLogEntry = ({ + String method, + SemanticsUpdatePhase phase, +}); + +class MockRoleManager extends RoleManager { + MockRoleManager(super.role, super.semanticsObject); + + final List log = []; + + void _log(String method) { + log.add(( + method: method, + phase: semanticsObject.owner.phase, + )); + } + + @override + void update() { + _log('update'); + } } class MockSemanticsEnabler implements SemanticsEnabler { @@ -2365,8 +2447,8 @@ void _testDialog() { semantics().updateSemantics(builder.build()); expectSemanticsTree(''' - -'''); + + '''); expect( semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.dialog), @@ -2404,14 +2486,14 @@ void _testDialog() { expect( warnings, [ - 'Semantic node 0 was assigned dialog role, but is missing a label. A dialog should contain a label so that a screen reader can communicate to the user that a dialog appeared and a user action is requested.', + 'Semantic node 0 had both scopesRoute and namesRoute set, indicating a self-labelled dialog, but it is missing the label. A dialog should be labelled either by setting namesRoute on itself and providing a label, or by containing a child node with namesRoute that can describe it with its content.', ], ); // But still sets the dialog role. expectSemanticsTree(''' - -'''); + + '''); expect( semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.dialog), @@ -2420,6 +2502,141 @@ void _testDialog() { semantics().semanticsEnabled = false; }); + + test('dialog can be described by a descendant', () { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + void pumpSemantics({ required String label }) { + final SemanticsTester tester = SemanticsTester(semantics()); + tester.updateNode( + id: 0, + scopesRoute: true, + transform: Matrix4.identity().toFloat64(), + children: [ + tester.updateNode( + id: 1, + children: [ + tester.updateNode( + id: 2, + namesRoute: true, + label: label, + ), + ], + ), + ], + ); + tester.apply(); + + expectSemanticsTree(''' + + + + + + + + + + '''); + } + + pumpSemantics(label: 'Dialog label'); + + expect( + semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.dialog), + isA(), + ); + expect( + semantics().debugSemanticsTree![2]!.debugRoleManagerFor(Role.routeName), + isA(), + ); + + pumpSemantics(label: 'Updated dialog label'); + + semantics().semanticsEnabled = false; + }); + + test('scopesRoute alone sets the dialog role with no label', () { + final List warnings = []; + printWarning = warnings.add; + + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + final SemanticsTester tester = SemanticsTester(semantics()); + tester.updateNode( + id: 0, + scopesRoute: true, + transform: Matrix4.identity().toFloat64(), + ); + tester.apply(); + + expectSemanticsTree(''' + + '''); + + expect( + semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.dialog), + isA(), + ); + expect( + semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.routeName), + isNull, + ); + + semantics().semanticsEnabled = false; + }); + + test('namesRoute alone has no effect', () { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + final SemanticsTester tester = SemanticsTester(semantics()); + tester.updateNode( + id: 0, + transform: Matrix4.identity().toFloat64(), + children: [ + tester.updateNode( + id: 1, + children: [ + tester.updateNode( + id: 2, + namesRoute: true, + label: 'Hello', + ), + ], + ), + ], + ); + tester.apply(); + + expectSemanticsTree(''' + + + + + + + + + + '''); + + expect( + semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.dialog), + isNull, + ); + expect( + semantics().debugSemanticsTree![2]!.debugRoleManagerFor(Role.routeName), + isA(), + ); + + semantics().semanticsEnabled = false; + }); } typedef CapturedAction = (int nodeId, ui.SemanticsAction action, ByteData? args);