Skip to content

Commit

Permalink
Fix a11y tab traversal (flutter#25797)
Browse files Browse the repository at this point in the history
* fix race with framework when using tab to traverse in a11y mode
  • Loading branch information
yjbanov authored and christopherfujino committed May 25, 2021
1 parent f8172c4 commit a9b004a
Show file tree
Hide file tree
Showing 10 changed files with 1,508 additions and 736 deletions.
4 changes: 2 additions & 2 deletions lib/web_ui/lib/src/engine/dom_renderer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ class DomRenderer {
/// This getter calls the `hasFocus` method of the `Document` interface.
/// See for more details:
/// https://developer.mozilla.org/en-US/docs/Web/API/Document/hasFocus
bool? get windowHasFocus =>
js_util.callMethod(html.document, 'hasFocus', <dynamic>[]);
bool get windowHasFocus =>
js_util.callMethod(html.document, 'hasFocus', <dynamic>[]) ?? false;

void _setupHotRestart() {
// This persists across hot restarts to clear stale DOM.
Expand Down
9 changes: 3 additions & 6 deletions lib/web_ui/lib/src/engine/semantics/label_and_value.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,9 @@ class LabelAndValue extends RoleManager {
final bool hasValue = semanticsObject.hasValue;
final bool hasLabel = semanticsObject.hasLabel;

// If the node is incrementable or a text field the value is reported to the
// browser via the respective role managers. We do not need to also render
// it again here.
final bool shouldDisplayValue = hasValue &&
!semanticsObject.isIncrementable &&
!semanticsObject.isTextField;
// If the node is incrementable the value is reported to the browser via
// the respective role manager. We do not need to also render it again here.
final bool shouldDisplayValue = hasValue && !semanticsObject.isIncrementable;

if (!hasLabel && !shouldDisplayValue) {
_cleanUpDom();
Expand Down
31 changes: 20 additions & 11 deletions lib/web_ui/lib/src/engine/semantics/semantics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -270,8 +270,8 @@ class SemanticsObject {
}

/// See [ui.SemanticsUpdateBuilder.updateNode].
int? get flags => _flags;
int? _flags;
int get flags => _flags;
int _flags = 0;

/// Whether the [flags] field has been updated but has not been applied to the
/// DOM yet.
Expand Down Expand Up @@ -584,7 +584,7 @@ class SemanticsObject {
SemanticsObject? _parent;

/// Whether this node currently has a given [SemanticsFlag].
bool hasFlag(ui.SemanticsFlag flag) => _flags! & flag.index != 0;
bool hasFlag(ui.SemanticsFlag flag) => _flags & flag.index != 0;

/// Whether [actions] contains the given action.
bool hasAction(ui.SemanticsAction action) => (_actions! & action.index) != 0;
Expand Down Expand Up @@ -785,15 +785,24 @@ class SemanticsObject {
/// > 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<Role, RoleManager?> _roleManagers = <Role, RoleManager?>{};

/// Returns the role manager for the given [role].
///
/// If a role manager does not exist for the given role, returns null.
RoleManager? debugRoleManagerFor(Role role) => _roleManagers[role];

/// Detects the roles that this semantics object corresponds to and manages
/// the lifecycles of [SemanticsObjectRole] objects.
void _updateRoles() {
_updateRole(Role.labelAndValue, (hasLabel || hasValue) && !isVisualOnly);
_updateRole(Role.labelAndValue, (hasLabel || hasValue) && !isTextField && !isVisualOnly);
_updateRole(Role.textField, isTextField);
_updateRole(
Role.tappable,
hasAction(ui.SemanticsAction.tap) ||
hasFlag(ui.SemanticsFlag.isButton));

bool shouldUseTappableRole =
(hasAction(ui.SemanticsAction.tap) || hasFlag(ui.SemanticsFlag.isButton)) &&
// Text fields manage their own focus/tap interactions. We don't need the
// tappable role manager. It only confuses AT.
!isTextField;

_updateRole(Role.tappable, shouldUseTappableRole);
_updateRole(Role.incrementable, isIncrementable);
_updateRole(Role.scrollable,
isVerticalScrollContainer || isHorizontalScrollContainer);
Expand Down Expand Up @@ -1145,7 +1154,7 @@ class EngineSemanticsOwner {
_instance = null;
}

final Map<int?, SemanticsObject?> _semanticsTree = <int?, SemanticsObject?>{};
final Map<int, SemanticsObject> _semanticsTree = <int, SemanticsObject>{};

/// Map [SemanticsObject.id] to parent [SemanticsObject] it was attached to
/// this frame.
Expand Down Expand Up @@ -1222,8 +1231,8 @@ class EngineSemanticsOwner {
/// Returns the entire semantics tree for testing.
///
/// Works only in debug mode.
Map<int?, SemanticsObject?>? get debugSemanticsTree {
Map<int?, SemanticsObject?>? result;
Map<int, SemanticsObject>? get debugSemanticsTree {
Map<int, SemanticsObject>? result;
assert(() {
result = _semanticsTree;
return true;
Expand Down
9 changes: 9 additions & 0 deletions lib/web_ui/lib/src/engine/semantics/tappable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ class Tappable extends RoleManager {
void update() {
final html.Element element = semanticsObject.element;

// "tab-index=0" is used to allow keyboard traversal of non-form elements.
// See also: https://developer.mozilla.org/en-US/docs/Web/Accessibility/Keyboard-navigable_JavaScript_widgets
element.tabIndex = 0;

semanticsObject.setAriaRole(
'button', semanticsObject.hasFlag(ui.SemanticsFlag.isButton));

Expand Down Expand Up @@ -49,6 +53,11 @@ class Tappable extends RoleManager {
_stopListening();
}
}

// Request focus so that the AT shifts a11y focus to this node.
if (semanticsObject.isFlagsDirty && semanticsObject.hasFocus) {
element.focus();
}
}

void _stopListening() {
Expand Down
Loading

0 comments on commit a9b004a

Please sign in to comment.