Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[web:a11y] support dialogs described by descendants #42108

Merged
merged 5 commits into from
May 19, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
90 changes: 74 additions & 16 deletions lib/web_ui/lib/src/engine/semantics/dialog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,84 @@ 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 ?? '');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where the aria-label is set for the case where namesRoute is a descendant of scopesRoute.

Does it happen through the LabelAndValue role manager? If yes, then why are we doing it in the Dialog role manager too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't set aria-label when a descendant provides a description. We use aria-describedby="ID_OF_DESCENDANT" instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And then that descendant is supposed to set the aria-label, right? I don't see RouteName doing it, so I'm assuming that LabelAndValue is doing it. But LabelAndValue should be doing it for Dialog as well. Maybe I'm missing something here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, the final state is like this:

<div role="dialog" aria-describedby="describer">
  <div id="describer">Dialog description</div>
</div>

No aria-label.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see. I was confused by this test:

expectSemanticsTree('''
  <sem aria-describedby="flt-semantic-node-2" style="$rootSemanticStyle">
    <sem-c>
      <sem>
        <sem-c>
          <sem aria-label="$label"></sem>
        </sem-c>
      </sem>
    </sem-c>
  </sem>
''');

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it's because the descendant can provide the description by applying aria-label to itself, but it can also be a group of elements that provide a description together.

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(() {
_lookUpNearestAncestorDialog();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check that this hasn't been disposed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

_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<Dialog>(Role.dialog);
}
}
}
71 changes: 54 additions & 17 deletions lib/web_ui/lib/src/engine/semantics/semantics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -365,15 +365,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.
///
/// 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.
/// 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.
///
/// 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].
Expand All @@ -390,6 +410,7 @@ final Map<Role, RoleManagerFactory> _roleFactories = <Role, RoleManagerFactory>{
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
Expand Down Expand Up @@ -423,7 +444,7 @@ 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();
void dispose() {}
}

/// Instantiation of a framework-side semantics node in the DOM.
Expand Down Expand Up @@ -827,6 +848,12 @@ 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
/// [EngineSemanticsOwner.addOneTimePostUpdateCallback]).
SemanticsObject? get parent => _parent;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to have an assert that throws if we are in the middle of an update. Not sure what's the best way to do that though. Maybe a flag EngineSemanticsOwner.isUpdating and then:

Suggested change
SemanticsObject? get parent => _parent;
SemanticsObject? get parent {
assert(!owner.isUpdating);
return _parent;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

SemanticsObject? _parent;

/// Whether this node currently has a given [SemanticsFlag].
Expand Down Expand Up @@ -881,14 +908,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).
Expand Down Expand Up @@ -1278,6 +1306,14 @@ 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 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<R extends RoleManager>(Role role) => _roleManagers[role]! as R;

/// Returns the role manager for the given [role].
///
/// If a role manager does not exist for the given role, returns null.
Expand All @@ -1287,10 +1323,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
Expand Down
145 changes: 140 additions & 5 deletions lib/web_ui/test/engine/semantics/semantics_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2365,8 +2365,8 @@ void _testDialog() {

semantics().updateSemantics(builder.build());
expectSemanticsTree('''
<sem role="dialog" aria-label="this is a dialog label" style="$rootSemanticStyle"><sem-c><sem></sem></sem-c></sem>
''');
<sem role="dialog" aria-label="this is a dialog label" style="$rootSemanticStyle"><sem-c><sem></sem></sem-c></sem>
''');

expect(
semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.dialog),
Expand Down Expand Up @@ -2404,19 +2404,154 @@ void _testDialog() {
expect(
warnings,
<String>[
'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('''
<sem role="dialog" aria-label="" style="$rootSemanticStyle"><sem-c><sem></sem></sem-c></sem>
''');
<sem role="dialog" aria-label="" style="$rootSemanticStyle"><sem-c><sem></sem></sem-c></sem>
''');

expect(
semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.dialog),
isA<Dialog>(),
);

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: <SemanticsNodeUpdate>[
tester.updateNode(
id: 1,
children: <SemanticsNodeUpdate>[
tester.updateNode(
id: 2,
namesRoute: true,
label: label,
),
],
),
],
);
tester.apply();

expectSemanticsTree('''
<sem aria-describedby="flt-semantic-node-2" style="$rootSemanticStyle">
<sem-c>
<sem>
<sem-c>
<sem aria-label="$label"></sem>
</sem-c>
</sem>
</sem-c>
</sem>
''');
}

pumpSemantics(label: 'Dialog label');

expect(
semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.dialog),
isA<Dialog>(),
);
expect(
semantics().debugSemanticsTree![2]!.debugRoleManagerFor(Role.routeName),
isA<RouteName>(),
);

pumpSemantics(label: 'Updated dialog label');

semantics().semanticsEnabled = false;
});

test('scopesRoute alone sets the dialog role with no label', () {
final List<String> warnings = <String>[];
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('''
<sem style="$rootSemanticStyle"></sem>
''');

expect(
semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.dialog),
isA<Dialog>(),
);
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: <SemanticsNodeUpdate>[
tester.updateNode(
id: 1,
children: <SemanticsNodeUpdate>[
tester.updateNode(
id: 2,
namesRoute: true,
label: 'Hello',
),
],
),
],
);
tester.apply();

expectSemanticsTree('''
<sem style="$rootSemanticStyle">
<sem-c>
<sem>
<sem-c>
<sem aria-label="Hello"></sem>
</sem-c>
</sem>
</sem-c>
</sem>
''');

expect(
semantics().debugSemanticsTree![0]!.debugRoleManagerFor(Role.dialog),
isNull,
);
expect(
semantics().debugSemanticsTree![2]!.debugRoleManagerFor(Role.routeName),
isA<RouteName>(),
);

semantics().semanticsEnabled = false;
});
Expand Down