diff --git a/lib/web_ui/lib/src/engine/semantics/dialog.dart b/lib/web_ui/lib/src/engine/semantics/dialog.dart index 7e8f58ec6c73d..8155ba4838415 100644 --- a/lib/web_ui/lib/src/engine/semantics/dialog.dart +++ b/lib/web_ui/lib/src/engine/semantics/dialog.dart @@ -76,6 +76,10 @@ class RouteName extends RoleManager { // 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.namesRoute) { + return; + } + if (semanticsObject.isLabelDirty) { final Dialog? dialog = _dialog; if (dialog != null) { diff --git a/lib/web_ui/lib/src/engine/semantics/focusable.dart b/lib/web_ui/lib/src/engine/semantics/focusable.dart index 4de57af1c6948..4b3285dbdf399 100644 --- a/lib/web_ui/lib/src/engine/semantics/focusable.dart +++ b/lib/web_ui/lib/src/engine/semantics/focusable.dart @@ -30,15 +30,20 @@ import 'semantics.dart'; class Focusable extends RoleManager { Focusable(SemanticsObject semanticsObject) : _focusManager = AccessibilityFocusManager(semanticsObject.owner), - super(Role.focusable, semanticsObject) { - _focusManager.manage(semanticsObject.id, semanticsObject.element); - } + super(Role.focusable, semanticsObject); final AccessibilityFocusManager _focusManager; @override void update() { - _focusManager.changeFocus(semanticsObject.hasFocus && (!semanticsObject.hasEnabledState || semanticsObject.isEnabled)); + if (semanticsObject.isFocusable) { + if (!_focusManager.isManaging) { + _focusManager.manage(semanticsObject.id, semanticsObject.element); + } + _focusManager.changeFocus(semanticsObject.hasFocus && (!semanticsObject.hasEnabledState || semanticsObject.isEnabled)); + } else { + _focusManager.stopManaging(); + } } @override @@ -79,6 +84,9 @@ class AccessibilityFocusManager { _FocusTarget? _target; + /// Whether this focus manager is managing a focusable target. + bool get isManaging => _target != null; + /// Starts managing the focus of the given [element]. /// /// The "focus" and "blur" DOM events are forwarded to the framework-side @@ -127,6 +135,7 @@ class AccessibilityFocusManager { /// Stops managing the focus of the current element, if any. void stopManaging() { final _FocusTarget? target = _target; + _target = null; if (target == null) { /// Nothing is being managed. Just return. @@ -135,7 +144,11 @@ class AccessibilityFocusManager { target.element.removeEventListener('focus', target.domFocusListener); target.element.removeEventListener('blur', target.domBlurListener); - _target = null; + + // Blur the element after removing listeners. If this method is being called + // it indicates that the framework already knows that this node should not + // have focus, and there's no need to notify it. + target.element.blur(); } void _setFocusFromDom(bool acquireFocus) { diff --git a/lib/web_ui/lib/src/engine/semantics/label_and_value.dart b/lib/web_ui/lib/src/engine/semantics/label_and_value.dart index 643f305fc7287..23421e13590d1 100644 --- a/lib/web_ui/lib/src/engine/semantics/label_and_value.dart +++ b/lib/web_ui/lib/src/engine/semantics/label_and_value.dart @@ -68,7 +68,6 @@ class LabelAndValue extends RoleManager { void _cleanUpDom() { semanticsObject.element.removeAttribute('aria-label'); - semanticsObject.clearAriaRole(); } @override diff --git a/lib/web_ui/lib/src/engine/semantics/live_region.dart b/lib/web_ui/lib/src/engine/semantics/live_region.dart index 239b9b600a13d..c922cddd717b7 100644 --- a/lib/web_ui/lib/src/engine/semantics/live_region.dart +++ b/lib/web_ui/lib/src/engine/semantics/live_region.dart @@ -22,6 +22,10 @@ class LiveRegion extends RoleManager { @override void update() { + if (!semanticsObject.isLiveRegion) { + return; + } + // Avoid announcing the same message over and over. if (_lastAnnouncement != semanticsObject.label) { _lastAnnouncement = semanticsObject.label; diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index 8e3971b6dfc1b..8f19b2ef10e72 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -449,39 +449,29 @@ abstract class PrimaryRoleManager { @visibleForTesting List get debugSecondaryRoles => _secondaryRoleManagers?.map((RoleManager manager) => manager.role).toList() ?? const []; - /// Adds generic focus management features, if applicable. + /// Adds generic focus management features. void addFocusManagement() { - if (semanticsObject.isFocusable) { - addSecondaryRole(Focusable(semanticsObject)); - } + addSecondaryRole(Focusable(semanticsObject)); } - /// Adds generic live region features, if applicable. + /// Adds generic live region features. void addLiveRegion() { - if (semanticsObject.isLiveRegion) { - addSecondaryRole(LiveRegion(semanticsObject)); - } + addSecondaryRole(LiveRegion(semanticsObject)); } - /// Adds generic route name features, if applicable. + /// Adds generic route name features. void addRouteName() { - if (semanticsObject.namesRoute) { - addSecondaryRole(RouteName(semanticsObject)); - } + addSecondaryRole(RouteName(semanticsObject)); } - /// Adds generic label features, if applicable. + /// Adds generic label features. void addLabelAndValue() { - if (semanticsObject.hasLabel || semanticsObject.hasValue || semanticsObject.hasTooltip) { - addSecondaryRole(LabelAndValue(semanticsObject)); - } + addSecondaryRole(LabelAndValue(semanticsObject)); } /// Adds generic functionality for handling taps and clicks. void addTappable() { - if (semanticsObject.isTappable) { - addSecondaryRole(Tappable(semanticsObject)); - } + addSecondaryRole(Tappable(semanticsObject)); } /// Adds a secondary role to this primary role manager. @@ -531,7 +521,7 @@ abstract class PrimaryRoleManager { /// gesture mode changes. @mustCallSuper void dispose() { - semanticsObject.clearAriaRole(); + semanticsObject.element.removeAttribute('role'); _isDisposed = true; } } @@ -1464,11 +1454,6 @@ class SemanticsObject { element.setAttribute('role', ariaRoleName); } - /// Removes the `role` HTML attribue, if any. - void clearAriaRole() { - element.removeAttribute('role'); - } - /// The primary role of this node. /// /// The primary role is assigned by [updateSelf] based on the combination of diff --git a/lib/web_ui/lib/src/engine/semantics/tappable.dart b/lib/web_ui/lib/src/engine/semantics/tappable.dart index 39362f9976a0a..259a9c7d55669 100644 --- a/lib/web_ui/lib/src/engine/semantics/tappable.dart +++ b/lib/web_ui/lib/src/engine/semantics/tappable.dart @@ -40,8 +40,7 @@ class Tappable extends RoleManager { @override void update() { - final DomElement element = semanticsObject.element; - if (semanticsObject.enabledState() == EnabledState.disabled || !semanticsObject.isTappable) { + if (!semanticsObject.isTappable || semanticsObject.enabledState() == EnabledState.disabled) { _stopListening(); } else { if (_clickListener == null) { @@ -52,7 +51,7 @@ class Tappable extends RoleManager { EnginePlatformDispatcher.instance.invokeOnSemanticsAction( semanticsObject.id, ui.SemanticsAction.tap, null); }); - element.addEventListener('click', _clickListener); + semanticsObject.element.addEventListener('click', _clickListener); } } } diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index 03a030358a74c..4c4f51eb0a49b 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -44,6 +44,9 @@ void runSemanticsTests() { group('longestIncreasingSubsequence', () { _testLongestIncreasingSubsequence(); }); + group('Role managers', () { + _testRoleManagerLifecycle(); + }); group('container', () { _testContainer(); }); @@ -91,6 +94,60 @@ void runSemanticsTests() { }); } +void _testRoleManagerLifecycle() { + test('Secondary role managers are added upon node initialization', () { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + // Check that roles are initialized immediately + { + final SemanticsTester tester = SemanticsTester(semantics()); + tester.updateNode( + id: 0, + isButton: true, + rect: const ui.Rect.fromLTRB(0, 0, 100, 50), + ); + tester.apply(); + + expectSemanticsTree(''); + + final SemanticsObject node = semantics().debugSemanticsTree![0]!; + expect(node.primaryRole?.role, PrimaryRole.button); + expect( + node.primaryRole?.debugSecondaryRoles, + containsAll([Role.focusable, Role.tappable, Role.labelAndValue]), + ); + expect(tester.getSemanticsObject(0).element.tabIndex, -1); + } + + // Check that roles apply their functionality upon update. + { + final SemanticsTester tester = SemanticsTester(semantics()); + tester.updateNode( + id: 0, + label: 'a label', + isFocusable: true, + isButton: true, + rect: const ui.Rect.fromLTRB(0, 0, 100, 50), + ); + tester.apply(); + + expectSemanticsTree(''); + + final SemanticsObject node = semantics().debugSemanticsTree![0]!; + expect(node.primaryRole?.role, PrimaryRole.button); + expect( + node.primaryRole?.debugSecondaryRoles, + containsAll([Role.focusable, Role.tappable, Role.labelAndValue]), + ); + expect(tester.getSemanticsObject(0).element.tabIndex, 0); + } + + semantics().semanticsEnabled = false; + }); +} + void _testEngineAccessibilityBuilder() { final EngineAccessibilityFeaturesBuilder builder = EngineAccessibilityFeaturesBuilder(0); @@ -2681,28 +2738,62 @@ void _testFocusable() { expect(element.tabIndex, -1); domDocument.body!.append(element); + // Start managing element manager.manage(1, element); expect(element.tabIndex, 0); expect(capturedActions, isEmpty); + expect(domDocument.activeElement, isNot(element)); + // Request focus manager.changeFocus(true); pumpSemantics(); // triggers post-update callbacks + expect(domDocument.activeElement, element); expect(capturedActions, [ (1, ui.SemanticsAction.didGainAccessibilityFocus, null), ]); capturedActions.clear(); + // Give up focus manager.changeFocus(false); pumpSemantics(); // triggers post-update callbacks expect(capturedActions, [ (1, ui.SemanticsAction.didLoseAccessibilityFocus, null), ]); capturedActions.clear(); + expect(domDocument.activeElement, isNot(element)); + + // Request focus again + manager.changeFocus(true); + pumpSemantics(); // triggers post-update callbacks + expect(domDocument.activeElement, element); + expect(capturedActions, [ + (1, ui.SemanticsAction.didGainAccessibilityFocus, null), + ]); + capturedActions.clear(); + // Stop managing manager.stopManaging(); + pumpSemantics(); // triggers post-update callbacks + expect( + reason: 'Even though the element was blurred after stopManaging there ' + 'should be no notification to the framework because the framework ' + 'should already know. Otherwise, it would not have asked to stop ' + 'managing the node.', + capturedActions, + isEmpty, + ); + expect(domDocument.activeElement, isNot(element)); + + // Attempt to request focus when not managing an element. manager.changeFocus(true); pumpSemantics(); // triggers post-update callbacks - expect(capturedActions, isEmpty); + expect( + reason: 'Attempting to request focus on a node that is not managed should ' + 'not result in any notifications to the framework.', + capturedActions, + isEmpty, + ); + expect(domDocument.activeElement, isNot(element)); semantics().semanticsEnabled = false; });