From 8a56925414fe1adb986a89d21aa5feda9eccbcbf Mon Sep 17 00:00:00 2001 From: nturgut Date: Tue, 28 Jul 2020 10:29:02 -0700 Subject: [PATCH] [a11y-web] Semantics fix (flutter/engine#19898) * fixinf enable semantics * fix unit tests * fix indentation * addressing reviewer comments * fix comment * make commnt starts upper case * fix grammar in comment * Add one more comment --- .../lib/src/engine/semantics/checkable.dart | 5 +- .../lib/src/engine/semantics/semantics.dart | 39 ++++++ .../lib/src/engine/semantics/tappable.dart | 3 +- .../test/engine/semantics/semantics_test.dart | 120 ++++++++++++------ 4 files changed, 126 insertions(+), 41 deletions(-) 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 5826fa6086a..9c7c2c101d8 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 @@ -28,7 +28,8 @@ enum _CheckableKind { toggle, } -_CheckableKind _checkableKindFromSemanticsFlag(SemanticsObject semanticsObject) { +_CheckableKind _checkableKindFromSemanticsFlag( + SemanticsObject semanticsObject) { if (semanticsObject.hasFlag(ui.SemanticsFlag.isInMutuallyExclusiveGroup)) { return _CheckableKind.radio; } else if (semanticsObject.hasFlag(ui.SemanticsFlag.hasToggledState)) { @@ -99,7 +100,7 @@ class Checkable extends RoleManager { } void _updateDisabledAttribute() { - if (!semanticsObject.hasFlag(ui.SemanticsFlag.isEnabled)) { + if (semanticsObject.enabledState() == EnabledState.disabled) { final html.Element element = semanticsObject.element; element ..setAttribute('aria-disabled', 'true') 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 01bdf99eac9..57c02b5bf22 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 @@ -605,6 +605,22 @@ class SemanticsObject { !hasAction(ui.SemanticsAction.tap) && !hasFlag(ui.SemanticsFlag.isButton); + /// Whether this object carry enabled/disabled state (and if so whether it is + /// enabled). + /// + /// See [EnabledState] for more details. + EnabledState enabledState() { + if (hasFlag(ui.SemanticsFlag.hasEnabledState)) { + if (hasFlag(ui.SemanticsFlag.isEnabled)) { + return EnabledState.enabled; + } else { + return EnabledState.disabled; + } + } else { + return EnabledState.noOpinion; + } + } + /// Updates this object from data received from a semantics [update]. /// /// This method creates [SemanticsObject]s for the direct children of this @@ -1542,3 +1558,26 @@ List longestIncreasingSubsequence(List list) { } return seq; } + +/// States that a [ui.SemanticsNode] can have. +/// +/// SemanticsNodes can be in three distinct states (enabled, disabled, +/// no opinion). +enum EnabledState { + /// Flag [ui.SemanticsFlag.hasEnabledState] is not set. + /// + /// The node does not have enabled/disabled state. + noOpinion, + + /// Flag [ui.SemanticsFlag.hasEnabledState] and [ui.SemanticsFlag.isEnabled] + /// are set. + /// + /// The node is enabled. + enabled, + + /// Flag [ui.SemanticsFlag.hasEnabledState] is set and + /// [ui.SemanticsFlag.isEnabled] is not set. + /// + /// The node is disabled. + disabled, +} 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 b18f5dc4308..3f58195b1d4 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 @@ -24,7 +24,8 @@ class Tappable extends RoleManager { semanticsObject.setAriaRole( 'button', semanticsObject.hasFlag(ui.SemanticsFlag.isButton)); - if (!semanticsObject.hasFlag(ui.SemanticsFlag.isEnabled) && + // Add `aria-disabled` for disabled buttons. + if (semanticsObject.enabledState() == EnabledState.disabled && semanticsObject.hasFlag(ui.SemanticsFlag.isButton)) { semanticsObject.element.setAttribute('aria-disabled', 'true'); _stopListening(); 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 186b7b71ac8..016c94f0c5f 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 @@ -100,7 +100,8 @@ void _testEngineSemanticsOwner() { void renderLabel(String label) { final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0, flags: 0, @@ -109,7 +110,8 @@ void _testEngineSemanticsOwner() { childrenInHitTestOrder: Int32List.fromList([1]), childrenInTraversalOrder: Int32List.fromList([1]), ); - updateNode(builder, + updateNode( + builder, id: 1, actions: 0, flags: 0, @@ -230,7 +232,8 @@ void _testHeader() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0, flags: 0 | ui.SemanticsFlag.isHeader.index, @@ -306,7 +309,8 @@ void _testContainer() { final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); const ui.Rect zeroOffsetRect = ui.Rect.fromLTRB(0, 0, 20, 20); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0, flags: 0, @@ -345,7 +349,8 @@ void _testContainer() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0, flags: 0, @@ -386,7 +391,8 @@ void _testVerticalScrolling() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.scrollUp.index, flags: 0, @@ -412,7 +418,8 @@ void _testVerticalScrolling() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.scrollUp.index, flags: 0, @@ -470,7 +477,8 @@ void _testVerticalScrolling() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.scrollUp.index | @@ -483,7 +491,8 @@ void _testVerticalScrolling() { ); for (int id = 1; id <= 3; id++) { - updateNode(builder, + updateNode( + builder, id: id, actions: 0, flags: 0, @@ -538,7 +547,8 @@ void _testHorizontalScrolling() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.scrollLeft.index, flags: 0, @@ -564,7 +574,8 @@ void _testHorizontalScrolling() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.scrollLeft.index, flags: 0, @@ -603,7 +614,8 @@ void _testHorizontalScrolling() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.scrollLeft.index | @@ -616,7 +628,8 @@ void _testHorizontalScrolling() { ); for (int id = 1; id <= 3; id++) { - updateNode(builder, + updateNode( + builder, id: id, actions: 0, flags: 0, @@ -671,7 +684,8 @@ void _testIncrementables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.increase.index, flags: 0, @@ -698,7 +712,8 @@ void _testIncrementables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.increase.index, flags: 0, @@ -734,7 +749,8 @@ void _testIncrementables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.decrease.index, flags: 0, @@ -769,7 +785,8 @@ void _testIncrementables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.decrease.index | @@ -801,7 +818,8 @@ void _testTextField() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | ui.SemanticsFlag.isTextField.index, @@ -830,7 +848,8 @@ void _testTextField() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | ui.SemanticsFlag.isTextField.index, @@ -867,11 +886,13 @@ void _testCheckables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | ui.SemanticsFlag.isEnabled.index | + ui.SemanticsFlag.hasEnabledState.index | ui.SemanticsFlag.hasToggledState.index | ui.SemanticsFlag.isToggled.index, transform: Matrix4.identity().toFloat64(), @@ -894,12 +915,14 @@ void _testCheckables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | ui.SemanticsFlag.hasToggledState.index | - ui.SemanticsFlag.isToggled.index, + ui.SemanticsFlag.isToggled.index | + ui.SemanticsFlag.hasEnabledState.index, transform: Matrix4.identity().toFloat64(), rect: const ui.Rect.fromLTRB(0, 0, 100, 50), ); @@ -920,12 +943,14 @@ void _testCheckables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | ui.SemanticsFlag.hasToggledState.index | - ui.SemanticsFlag.isEnabled.index, + ui.SemanticsFlag.isEnabled.index | + ui.SemanticsFlag.hasEnabledState.index, transform: Matrix4.identity().toFloat64(), rect: const ui.Rect.fromLTRB(0, 0, 100, 50), ); @@ -946,11 +971,13 @@ void _testCheckables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | ui.SemanticsFlag.isEnabled.index | + ui.SemanticsFlag.hasEnabledState.index | ui.SemanticsFlag.hasCheckedState.index | ui.SemanticsFlag.isChecked.index, transform: Matrix4.identity().toFloat64(), @@ -973,11 +1000,13 @@ void _testCheckables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | ui.SemanticsFlag.hasCheckedState.index | + ui.SemanticsFlag.hasEnabledState.index | ui.SemanticsFlag.isChecked.index, transform: Matrix4.identity().toFloat64(), rect: const ui.Rect.fromLTRB(0, 0, 100, 50), @@ -999,12 +1028,14 @@ void _testCheckables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | ui.SemanticsFlag.hasCheckedState.index | - ui.SemanticsFlag.isEnabled.index, + ui.SemanticsFlag.isEnabled.index | + ui.SemanticsFlag.hasEnabledState.index, transform: Matrix4.identity().toFloat64(), rect: const ui.Rect.fromLTRB(0, 0, 100, 50), ); @@ -1025,11 +1056,13 @@ void _testCheckables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | ui.SemanticsFlag.isEnabled.index | + ui.SemanticsFlag.hasEnabledState.index | ui.SemanticsFlag.hasCheckedState.index | ui.SemanticsFlag.isInMutuallyExclusiveGroup.index | ui.SemanticsFlag.isChecked.index, @@ -1053,10 +1086,12 @@ void _testCheckables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | + ui.SemanticsFlag.hasEnabledState.index | ui.SemanticsFlag.hasCheckedState.index | ui.SemanticsFlag.isInMutuallyExclusiveGroup.index | ui.SemanticsFlag.isChecked.index, @@ -1080,11 +1115,13 @@ void _testCheckables() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | ui.SemanticsFlag.isEnabled.index | + ui.SemanticsFlag.hasEnabledState.index | ui.SemanticsFlag.hasCheckedState.index | ui.SemanticsFlag.isInMutuallyExclusiveGroup.index, transform: Matrix4.identity().toFloat64(), @@ -1109,7 +1146,8 @@ void _testTappable() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | @@ -1136,7 +1174,8 @@ void _testTappable() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0 | ui.SemanticsAction.tap.index, flags: 0 | @@ -1166,7 +1205,8 @@ void _testImage() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0, flags: 0 | ui.SemanticsFlag.isImage.index, @@ -1191,7 +1231,8 @@ void _testImage() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0, flags: 0 | ui.SemanticsFlag.isImage.index, @@ -1223,7 +1264,8 @@ void _testImage() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0, flags: 0 | ui.SemanticsFlag.isImage.index, @@ -1246,7 +1288,8 @@ void _testImage() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0, flags: 0 | ui.SemanticsFlag.isImage.index, @@ -1279,7 +1322,8 @@ void _testLiveRegion() { ..semanticsEnabled = true; final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); - updateNode(builder, + updateNode( + builder, id: 0, actions: 0, label: 'This is a snackbar',