Skip to content

Commit

Permalink
[web:a11y] introduce primary role responsible for ARIA roles (flutter…
Browse files Browse the repository at this point in the history
…#43159)

This PR fixes flutter/flutter#128468 by changing the relationship between semantics nodes and their roles from this:

```
SemanticsNode one-to-many RoleManager
```

To this:

```
SemanticsNode one-to-one PrimaryRoleManager one-to-many RoleManager
```

Previously a node would simply have multiple role managers, some of which would be responsible for setting the `role` attribute. It wasn't clear which role manager should be doing this. It also wasn't clear which role managers were safe to reuse across multiple types of nodes. This led to the unfortunate situation in flutter/flutter#128468 where `LabelAndValue` ended up overriding the role assigned by `Checkable`.

With this PR, a `SemanticsNode` has exactly one `PrimaryRoleManager`. A primary role manager is responsible for setting the `role` attribute, and importantly, it's the _only_ thing responsible for it. It's _not safe_ to share primary role managers across different kinds of nodes. They are meant to provide very specific functionality for the widget's main role. OTOH, a non-primary `RoleManager` provides a piece of functionality that's safe to share.

A `Checkable` is a `PrimaryRoleManager` and is the only thing that decides on the `role` attribute. `LabelAndValue` is now a `RoleManager` that's not responsible for setting the role. It's only responsible for `aria-label`. No more confusion.

This also drastically simplifies the logic for role assignment. There's no more [logical soup](https://github.com/flutter/engine/blob/eca910dd5e3f1d8e18b10f3a46ce8d1454a232c8/lib/web_ui/lib/src/engine/semantics/semantics.dart#L1340) attempting to find a good subset of roles to assign to a node. [Finding](https://github.com/yjbanov/engine/blob/93df91df9575f8fc212aac115ccccc23f8fba26f/lib/web_ui/lib/src/engine/semantics/semantics.dart#L1477) and [instantiating](https://github.com/yjbanov/engine/blob/93df91df9575f8fc212aac115ccccc23f8fba26f/lib/web_ui/lib/src/engine/semantics/semantics.dart#L1498) primary roles are very linear steps, as is [assigning a set of secondary roles](https://github.com/yjbanov/engine/blob/93df91df9575f8fc212aac115ccccc23f8fba26f/lib/web_ui/lib/src/engine/semantics/image.dart#L16).
  • Loading branch information
yjbanov authored and kjlubick committed Jul 14, 2023
1 parent 359e572 commit efb35e8
Show file tree
Hide file tree
Showing 13 changed files with 454 additions and 289 deletions.
20 changes: 7 additions & 13 deletions lib/web_ui/lib/src/engine/semantics/checkable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,25 @@ _CheckableKind _checkableKindFromSemanticsFlag(
/// See also [ui.SemanticsFlag.hasCheckedState], [ui.SemanticsFlag.isChecked],
/// [ui.SemanticsFlag.isInMutuallyExclusiveGroup], [ui.SemanticsFlag.isToggled],
/// [ui.SemanticsFlag.hasToggledState]
class Checkable extends RoleManager {
class Checkable extends PrimaryRoleManager {
Checkable(SemanticsObject semanticsObject)
: _kind = _checkableKindFromSemanticsFlag(semanticsObject),
super(Role.checkable, semanticsObject);
super.withBasics(PrimaryRole.checkable, semanticsObject);

final _CheckableKind _kind;

@override
void update() {
super.update();

if (semanticsObject.isFlagsDirty) {
switch (_kind) {
case _CheckableKind.checkbox:
semanticsObject.setAriaRole('checkbox', true);
semanticsObject.setAriaRole('checkbox');
case _CheckableKind.radio:
semanticsObject.setAriaRole('radio', true);
semanticsObject.setAriaRole('radio');
case _CheckableKind.toggle:
semanticsObject.setAriaRole('switch', true);
semanticsObject.setAriaRole('switch');
}

/// Adding disabled and aria-disabled attribute to notify the assistive
Expand All @@ -85,14 +87,6 @@ class Checkable extends RoleManager {
@override
void dispose() {
super.dispose();
switch (_kind) {
case _CheckableKind.checkbox:
semanticsObject.setAriaRole('checkbox', false);
case _CheckableKind.radio:
semanticsObject.setAriaRole('radio', false);
case _CheckableKind.toggle:
semanticsObject.setAriaRole('switch', false);
}
_removeDisabledAttribute();
}

Expand Down
22 changes: 15 additions & 7 deletions lib/web_ui/lib/src/engine/semantics/dialog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,19 @@ import '../util.dart';
/// Provides accessibility for dialogs.
///
/// See also [Role.dialog].
class Dialog extends RoleManager {
Dialog(SemanticsObject semanticsObject) : super(Role.dialog, semanticsObject);
class Dialog extends PrimaryRoleManager {
Dialog(SemanticsObject semanticsObject) : super.blank(PrimaryRole.dialog, semanticsObject) {
// The following secondary roles can coexist with dialog. Generic `RouteName`
// and `LabelAndValue` are not used by this role because when the dialog
// names its own route an `aria-label` is used instead of `aria-describedby`.
addFocusManagement();
addLiveRegion();
}

@override
void update() {
super.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) {
Expand All @@ -31,7 +39,7 @@ class Dialog extends RoleManager {
return true;
}());
semanticsObject.element.setAttribute('aria-label', label ?? '');
semanticsObject.setAriaRole('dialog', true);
semanticsObject.setAriaRole('dialog');
}
}

Expand All @@ -43,7 +51,7 @@ class Dialog extends RoleManager {
return;
}

semanticsObject.setAriaRole('dialog', true);
semanticsObject.setAriaRole('dialog');
semanticsObject.element.setAttribute(
'aria-describedby',
routeName.semanticsObject.element.id,
Expand Down Expand Up @@ -88,11 +96,11 @@ class RouteName extends RoleManager {

void _lookUpNearestAncestorDialog() {
SemanticsObject? parent = semanticsObject.parent;
while (parent != null && !parent.hasRole(Role.dialog)) {
while (parent != null && parent.primaryRole?.role != PrimaryRole.dialog) {
parent = parent.parent;
}
if (parent != null && parent.hasRole(Role.dialog)) {
_dialog = parent.getRole<Dialog>(Role.dialog);
if (parent != null && parent.primaryRole?.role == PrimaryRole.dialog) {
_dialog = parent.primaryRole! as Dialog;
}
}
}
18 changes: 14 additions & 4 deletions lib/web_ui/lib/src/engine/semantics/image.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,18 @@ import 'semantics.dart';
/// Uses aria img role to convey this semantic information to the element.
///
/// Screen-readers takes advantage of "aria-label" to describe the visual.
class ImageRoleManager extends RoleManager {
class ImageRoleManager extends PrimaryRoleManager {
ImageRoleManager(SemanticsObject semanticsObject)
: super(Role.image, semanticsObject);
: super.blank(PrimaryRole.image, semanticsObject) {
// The following secondary roles can coexist with images. `LabelAndValue` is
// not used because this role manager uses special auxiliary elements to
// supply ARIA labels.
// TODO(yjbanov): reevaluate usage of aux elements, https://github.com/flutter/flutter/issues/129317
addFocusManagement();
addLiveRegion();
addRouteName();
addTappable();
}

/// The element with role="img" and aria-label could block access to all
/// children elements, therefore create an auxiliary element and describe the
Expand All @@ -21,6 +30,8 @@ class ImageRoleManager extends RoleManager {

@override
void update() {
super.update();

if (semanticsObject.isVisualOnly && semanticsObject.hasChildren) {
if (_auxiliaryImageElement == null) {
_auxiliaryImageElement = domDocument.createElement('flt-semantics-img');
Expand All @@ -44,7 +55,7 @@ class ImageRoleManager extends RoleManager {
_auxiliaryImageElement!.setAttribute('role', 'img');
_setLabel(_auxiliaryImageElement);
} else if (semanticsObject.isVisualOnly) {
semanticsObject.setAriaRole('img', true);
semanticsObject.setAriaRole('img');
_setLabel(semanticsObject.element);
_cleanUpAuxiliaryElement();
} else {
Expand All @@ -67,7 +78,6 @@ class ImageRoleManager extends RoleManager {
}

void _cleanupElement() {
semanticsObject.setAriaRole('img', false);
semanticsObject.element.removeAttribute('aria-label');
}

Expand Down
13 changes: 11 additions & 2 deletions lib/web_ui/lib/src/engine/semantics/incrementable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,17 @@ import 'semantics.dart';
/// The input element is disabled whenever the gesture mode switches to pointer
/// events. This is to prevent the browser from taking over drag gestures. Drag
/// gestures must be interpreted by the Flutter framework.
class Incrementable extends RoleManager {
class Incrementable extends PrimaryRoleManager {
Incrementable(SemanticsObject semanticsObject)
: _focusManager = AccessibilityFocusManager(semanticsObject.owner),
super(Role.incrementable, semanticsObject) {
super.blank(PrimaryRole.incrementable, semanticsObject) {
// The following generic roles can coexist with incrementables. Generic focus
// management is not used by this role because the root DOM element is not
// the one being focused on, but the internal `<input>` element.
addLiveRegion();
addRouteName();
addLabelAndValue();

semanticsObject.element.append(_element);
_element.type = 'range';
_element.setAttribute('role', 'slider');
Expand Down Expand Up @@ -80,6 +87,8 @@ class Incrementable extends RoleManager {

@override
void update() {
super.update();

switch (semanticsObject.owner.gestureMode) {
case GestureMode.browserGestures:
_enableBrowserGestureHandling();
Expand Down
29 changes: 0 additions & 29 deletions lib/web_ui/lib/src/engine/semantics/label_and_value.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'package:ui/ui.dart' as ui;

import '../dom.dart';
import 'semantics.dart';

Expand Down Expand Up @@ -66,33 +64,6 @@ class LabelAndValue extends RoleManager {

semanticsObject.element
.setAttribute('aria-label', combinedValue.toString());

// Assign one of three roles to the element: heading, group, text.
//
// - "group" is used when the node has children, irrespective of whether the
// node is marked as a header or not. This is because marking a group
// as a "heading" will prevent the AT from reaching its children.
// - "heading" is used when the framework explicitly marks the node as a
// heading and the node does not have children.
// - "text" is used by default.
//
// As of October 24, 2022, "text" only has effect on Safari. Other browsers
// ignore it. Setting role="text" prevents Safari from treating the element
// as a "group" or "empty group". Other browsers still announce it as
// "group" or "empty group". However, other options considered produced even
// worse results, such as:
//
// - Ignore the size of the element and size the focus ring to the text
// content, which is wrong. The HTML text size is irrelevant because
// Flutter renders into canvas, so the focus ring looks wrong.
// - Read out the same label multiple times.
if (semanticsObject.hasChildren) {
semanticsObject.setAriaRole('group', true);
} else if (semanticsObject.hasFlag(ui.SemanticsFlag.isHeader)) {
semanticsObject.setAriaRole('heading', true);
} else {
semanticsObject.setAriaRole('text', true);
}
}

void _cleanUpDom() {
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/semantics/live_region.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import 'semantics.dart';
/// no content will be read.
class LiveRegion extends RoleManager {
LiveRegion(SemanticsObject semanticsObject)
: super(Role.labelAndValue, semanticsObject);
: super(Role.liveRegion, semanticsObject);

String? _lastAnnouncement;

Expand Down
6 changes: 4 additions & 2 deletions lib/web_ui/lib/src/engine/semantics/scrollable.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ import 'package:ui/ui.dart' as ui;
/// contents is less than the size of the viewport the browser snaps
/// "scrollTop" back to zero. If there is more content than available in the
/// viewport "scrollTop" may take positive values.
class Scrollable extends RoleManager {
class Scrollable extends PrimaryRoleManager {
Scrollable(SemanticsObject semanticsObject)
: super(Role.scrollable, semanticsObject) {
: super.withBasics(PrimaryRole.scrollable, semanticsObject) {
_scrollOverflowElement.style
..position = 'absolute'
..transformOrigin = '0 0 0'
Expand Down Expand Up @@ -95,6 +95,8 @@ class Scrollable extends RoleManager {

@override
void update() {
super.update();

semanticsObject.owner.addOneTimePostUpdateCallback(() {
_neutralizeDomScrollPosition();
semanticsObject.recomputePositionAndSize();
Expand Down

0 comments on commit efb35e8

Please sign in to comment.