From de5ce1e4dc466ddaaf93ca5db801e23a3a42ee6e Mon Sep 17 00:00:00 2001 From: Yegor Date: Thu, 9 May 2024 15:23:22 -0700 Subject: [PATCH] [web] scale semantic text elements to match the desired focus ring size (flutter/engine#52586) Due to https://g-issues.chromium.org/issues/40875151?pli=1&authuser=0 and a lack of an ARIA role for plain text nodes (after the removal of `role="text"` in WebKit recently), there is no way to customize the size of the screen reader focus ring for a plain text element. The focus ring always tightly hugs the text itself. A workaround implemented in this PR is to match the size of the text exactly to the desired focus ring size. This is done by measuring the size of the text in the DOM, then scaling it both vertically and horizontally to match the size requested by the framework for the corresponding semantics node. To avoid serious performance penalty, the following optimizations were included: - Nodes that are satisfiable by just an `aria-label` do not need this workaround, and are skipped. - Nodes that must use DOM text (e.g. links, buttons) but have ARIA roles that size them based on the element, do not need this workaround, and are skipped. - Nodes that do need the workaround are first measured in a single batch, incurring only one page reflow. Then they are all updated in a single batch without taking any further DOM measurements. This ensures that no matter how many text spans are rendered, only one reflow is needed to measure them all. - Nodes that need the workaround cache the previous label and size, and if they do not change, the size is not updated. Other changes: - Rename `LeafLabelRepresentation` to `LabelRepresentation`, because labels also apply to non-leaf nodes (e.g. `aria-label` may be applied to a container). - Rename `labelRepresentation` to `preferredLabelRepresentation`, because a particular label representation cannot be guaranteed. A node that currently looks like a leaf text node may turn out to be an empty container, and after adding children to it must switch from using DOM text to an `aria-label`. Therefore, role manager only specify a preference, but `LabelAndValue` ultimately decides which representation is usable. - Introduce `void initState()` in `PrimaryRoleManager` to be used for one-time initialization of state and DOM structure after all objects that are in a one-to-one relationship with each other create all the references needed to establish that relationship (`PrimaryRoleManager`, `SemanticsObject`, `element`, `owner`, etc). This is not available at the time the constructors are called. Fixes https://github.com/flutter/flutter/issues/146774. --- .../lib/web_ui/lib/src/engine/dom.dart | 8 + .../lib/src/engine/semantics/checkable.dart | 2 +- .../src/engine/semantics/incrementable.dart | 2 +- .../src/engine/semantics/label_and_value.dart | 472 +++++++++++++++++- .../web_ui/lib/src/engine/semantics/link.dart | 2 +- .../src/engine/semantics/platform_view.dart | 2 +- .../lib/src/engine/semantics/scrollable.dart | 25 +- .../lib/src/engine/semantics/semantics.dart | 81 +-- .../lib/src/engine/semantics/tappable.dart | 2 +- .../lib/web_ui/test/common/matchers.dart | 62 ++- .../lib/web_ui/test/engine/matchers_test.dart | 51 +- .../test/engine/semantics/semantics_test.dart | 203 +++++--- .../engine/semantics/semantics_tester.dart | 5 - ...ler_test.dart => semantics_text_test.dart} | 116 ++++- .../engine/semantics/text_field_test.dart | 2 +- 15 files changed, 852 insertions(+), 183 deletions(-) rename engine/src/flutter/lib/web_ui/test/engine/semantics/{semantics_crawler_test.dart => semantics_text_test.dart} (58%) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart index 465d66703a..d008d5ae1c 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/dom.dart @@ -589,6 +589,14 @@ extension DomElementExtension on DomElement { external JSNumber get _clientWidth; double get clientWidth => _clientWidth.toDartDouble; + @JS('offsetHeight') + external JSNumber get _offsetHeight; + double get offsetHeight => _offsetHeight.toDartDouble; + + @JS('offsetWidth') + external JSNumber get _offsetWidth; + double get offsetWidth => _offsetWidth.toDartDouble; + @JS('id') external JSString get _id; String get id => _id.toDart; 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 cf37b2423b..30b7928a13 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 @@ -55,7 +55,7 @@ class Checkable extends PrimaryRoleManager { super.withBasics( PrimaryRole.checkable, semanticsObject, - labelRepresentation: LeafLabelRepresentation.ariaLabel, + preferredLabelRepresentation: LabelRepresentation.ariaLabel, ) { addTappable(); } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart index 5a4abbbfd0..12d4468cd2 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/incrementable.dart @@ -28,7 +28,7 @@ class Incrementable extends PrimaryRoleManager { // the one being focused on, but the internal `` element. addLiveRegion(); addRouteName(); - addLabelAndValue(labelRepresentation: LeafLabelRepresentation.ariaLabel); + addLabelAndValue(preferredRepresentation: LabelRepresentation.ariaLabel); append(_element); _element.type = 'range'; diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/label_and_value.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/label_and_value.dart index 1d8045bf70..9601074785 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/label_and_value.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/label_and_value.dart @@ -2,6 +2,8 @@ // 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'; @@ -14,12 +16,413 @@ import 'semantics.dart'; /// respect the `aria-label` without a [DomText] node. Crawlers typically do not /// need this information, as they primarily scan visible text, which is /// communicated in semantics as leaf text and heading nodes. -enum LeafLabelRepresentation { +enum LabelRepresentation { /// Represents the label as an `aria-label` attribute. + /// + /// This representation is the most efficient as all it does is pass a string + /// to the browser that does not incur any DOM costs. + /// + /// The drawback of this representation is that it is not compatible with most + /// web crawlers, and for some ARIA roles (including the implicit "generic" + /// role) JAWS on Windows. However, this role is still the most common, as it + /// applies to all container nodes, and many ARIA roles (e.g. checkboxes, + /// radios, scrollables, sliders). ariaLabel, /// Represents the label as a [DomText] node. + /// + /// This is the second fastest way to represent a label in the DOM. It has a + /// small cost because the browser lays out the text (in addition to Flutter + /// having already done it). + /// + /// This representation is compatible with most web crawlers, and it is the + /// best option for certain ARIA roles, such as buttons, links, and headings. domText, + + /// Represents the label as a sized span. + /// + /// This representation is the costliest as it uses an extra element that + /// need to be laid out to compute the right size. It is compatible with most + /// web crawlers, and it is the best options for certain ARIA roles, such as + /// the implicit "generic" role used for plain text (not headings). + sizedSpan; + + /// Creates the behavior for this label representation. + LabelRepresentationBehavior createBehavior(PrimaryRoleManager owner) { + return switch (this) { + ariaLabel => AriaLabelRepresentation._(owner), + domText => DomTextRepresentation._(owner), + sizedSpan => SizedSpanRepresentation._(owner), + }; + } +} + +/// Provides a DOM behavior for a [LabelRepresentation]. +abstract final class LabelRepresentationBehavior { + LabelRepresentationBehavior(this.kind, this.owner); + + final LabelRepresentation kind; + + /// The role manager that this label representation is attached to. + final PrimaryRoleManager owner; + + /// Convenience getter for the corresponding semantics object. + SemanticsObject get semanticsObject => owner.semanticsObject; + + /// Updates the label displayed to the user. + void update(String label); + + /// Removes the DOM associated with this label. + /// + /// This can happen when the representation is changed from one type to + /// another. + void cleanUp(); + + /// The element that gets focus when [focusAsRouteDefault] is called. + /// + /// Each label behavior decides which element should be focused on based on + /// its own bespoke DOM structure. + DomElement get focusTarget; + + /// Move the accessibility focus to the element the carries the label assuming + /// the node is not [Focusable]. + /// + /// Since normally, plain text is not focusable (e.g. it doesn't have explicit + /// or implicit `tabindex`), `tabindex` must be added artificially. + /// + /// Plain text nodes should not be focusable via keyboard or mouse. They are + /// only focusable for the purposes of focusing the screen reader. To achieve + /// this the -1 value is used. + /// + /// See also: + /// + /// https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex + void focusAsRouteDefault() { + focusTarget.tabIndex = -1; + focusTarget.focus(); + } +} + +/// Sets the label as `aria-label`. +/// +/// Example: +/// +/// +final class AriaLabelRepresentation extends LabelRepresentationBehavior { + AriaLabelRepresentation._(PrimaryRoleManager owner) : super(LabelRepresentation.ariaLabel, owner); + + String? _previousLabel; + + @override + void update(String label) { + if (label == _previousLabel) { + return; + } + owner.setAttribute('aria-label', label); + } + + @override + void cleanUp() { + owner.removeAttribute('aria-label'); + } + + // ARIA label does not introduce extra DOM elements, so focus should go to the + // semantic node's host element. + @override + DomElement get focusTarget => owner.element; +} + +/// Sets the label as text inside the DOM element. +/// +/// Example: +/// +/// Hello, World! +/// +/// This representation is used when the ARIA role of the element already sizes +/// the element and therefore no extra sizing assistance is needed. If there is +/// no ARIA role set, or the role does not size the element, then the +/// [SizedSpanRepresentation] representation can be used. +final class DomTextRepresentation extends LabelRepresentationBehavior { + DomTextRepresentation._(PrimaryRoleManager owner) : super(LabelRepresentation.domText, owner); + + DomText? _domText; + String? _previousLabel; + + @override + void update(String label) { + if (label == _previousLabel) { + return; + } + + _domText?.remove(); + final DomText domText = domDocument.createTextNode(label); + _domText = domText; + semanticsObject.element.appendChild(domText); + } + + @override + void cleanUp() { + _domText?.remove(); + } + + // DOM text does not introduce extra DOM elements, so focus should go to the + // semantic node's host element. + @override + DomElement get focusTarget => owner.element; +} + +/// A span queue for a size update. +typedef _QueuedSizeUpdate = ({ + // The span to be sized. + SizedSpanRepresentation representation, + + // The desired size. + ui.Size targetSize, +}); + +/// The size of a span as measured in the DOM. +typedef _Measurement = ({ + // The span that was measured. + SizedSpanRepresentation representation, + + // The measured size of the DOM element before the size adjustment. + ui.Size domSize, + + // The size of the element that the screen reader should observe after the + // size adjustment. + ui.Size targetSize, +}); + +/// Sets the label as the text of a `` child element. +/// +/// The span element is scaled to match the size of the semantic node. +/// +/// Example: +/// +/// +/// Hello, World! +/// +/// +/// Text scaling is used to control the size of the screen reader focus ring. +/// This is used for plain text nodes (e.g. paragraphs of text). +/// +/// ## Why use scaling rather than another method? +/// +/// Due to https://g-issues.chromium.org/issues/40875151?pli=1&authuser=0 and a +/// lack of an ARIA role for plain text nodes (expecially after the removal of +/// ARIA role "text" in WebKit, starting with Safari 17), there is no way to +/// customize the size of the screen reader focus ring for a plain text element. +/// The focus ring always tightly hugs the text itself. The following approaches +/// were tried, and all failed: +/// +/// * `text-align` + dummy text to force text align to span the width of the +/// element. This does affect the screen reader focus size, but this method is +/// limited to width only. There's no way to control the height. Also, using +/// dummy text at the end feels extremely hacky, and risks failing due to +/// proprietary screen reader behaviors - they may not consistency react to +/// the dummy text (e.g. some may read it out loud). +/// * The following methods did not have the desired effect: +/// - Different `display` values. +/// - Adding visual/layout features to the element: border, outline, padding, +/// box-sizing, text-shadow. +/// * `role="text"` was used previously and worked, but only in Safari pre-17. +/// * `role="group"` sizes the element correctly, but breaks the message to the +/// user (reads "empty group", requires multi-step traversal). +/// * Adding `aria-hidden` contents to the element. This results in "group" +/// behavior. +/// * Use an existing non-text role, e.g. "heading". Sizes correctly, but breaks +/// the message (reads "heading"). +final class SizedSpanRepresentation extends LabelRepresentationBehavior { + SizedSpanRepresentation._(PrimaryRoleManager owner) : super(LabelRepresentation.sizedSpan, owner) { + _domText.style + // `inline-block` is needed for two reasons: + // - It supports measuring the true size of the text. Pure `block` would + // disassociate the size of the text from the size of the element. + // - It supports the `transform` and `transform-origin` properties. Pure + // `inline` does not support them. + ..display = 'inline-block' + + // Do not wrap text based on parent constraints. Instead, to fit in the + // parent's box the text will be scaled. + ..whiteSpace = 'nowrap' + + // The origin of the coordinate system is the top-left corner of the + // parent element. + ..transformOrigin = '0 0 0'; + semanticsObject.element.appendChild(_domText); + } + + final DomElement _domText = domDocument.createElement('span'); + String? _previousLabel; + ui.Size? _previousSize; + + @override + void update(String label) { + final ui.Size? size = semanticsObject.rect?.size; + final bool labelChanged = label != _previousLabel; + final bool sizeChanged = size != _previousSize; + + // Label must be updated before sizing because the size depends on text + // content. + if (labelChanged) { + _domText.text = label; + } + + // This code makes the assumption that the DOM size of the element depends + // solely on the text of the label. This is because text in the semantics + // tree is unstyled. If this ever changes, this assumption will no longer + // hold, and this code will need to be updated. + if (labelChanged || sizeChanged) { + _updateSize(size); + } + + // Remember the last used data to shut off unnecessary updates. + _previousLabel = label; + _previousSize = size; + } + + // Scales the text span (if any), such that the text matches the size of the + // node. This is important because screen reader focus sizes itself tightly + // around the text. Frequently, Flutter wants the focus to be different from + // the text itself. For example, when you focus on a card widget containing a + // piece of text, it is desirable that the focus covers the whole card, and + // not just the text inside. + // + // The scaling may cause the text to become distorted, but that doesn't matter + // because the semantic DOM is invisible. + // + // See: https://github.com/flutter/flutter/issues/146774 + void _updateSize(ui.Size? size) { + if (size == null) { + // There's no size to match => remove whatever stale sizing information was there. + // Note, it is not necessary to always reset the transform before measuring, + // as transform does not affect the offset size of the element. We do not + // reset it unnecessarily to reduce the cost of setting properties + // unnecessarily. + _domText.style.transform = ''; + return; + } + + if (_resizeQueue == null) { + _resizeQueue = <_QueuedSizeUpdate>[]; + + // Perform the adjustment in a post-update callback because the DOM layout + // can only be performed when the elements are attached to the document, + // but at this point the DOM tree is not yet finalized, and the element + // corresponding to the semantic node may still be detached. + semanticsObject.owner.addOneTimePostUpdateCallback(_updateSizes); + } + _resizeQueue!.add(( + representation: this, + targetSize: size, + )); + } + + @override + void cleanUp() { + _domText.remove(); + } + + static List<_QueuedSizeUpdate>? _resizeQueue; + + static void _updateSizes() { + final List<_QueuedSizeUpdate>? queue = _resizeQueue; + + // Eagerly reset the queue before doing any work. This ensures that if there + // is an unexpected error while processing the queue, we don't end up in a + // cycle that grows the queue indefinitely. Worst case, some text nodes end + // up incorrectly sized, but that's a smaller problem compared to running + // out of memory. + _resizeQueue = null; + + assert( + queue != null && queue.isNotEmpty, + '_updateSizes was called with an empty _resizeQueue. This should never ' + 'happend. If it does, please file an issue at ' + 'https://github.com/flutter/flutter/issues/new/choose', + ); + + if (queue == null || queue.isEmpty) { + // This should not happen, but if it does (e.g. something else fails and + // caused the post-update callback to be called with an empty queue), do + // not crash. + return; + } + + final List<_Measurement> measurements = <_Measurement>[]; + + // Step 1: set `display` to `inline` so that the measurement measures the + // true size of the text. Update all spans in a batch so that the + // measurement can be done without changing CSS properties that + // trigger reflow. + for (final _QueuedSizeUpdate update in queue) { + update.representation._domText.style.display = 'inline'; + } + + // Step 2: measure all spans in a single batch prior to updating their CSS + // styles. This way, all measurements are taken with a single reflow. + // Interleaving measurements with updates, will cause the browser to + // reflow the page between measurements. + for (final _QueuedSizeUpdate update in queue) { + // Both clientWidth/Height and offsetWidth/Height provide a good + // approximation for the purposes of sizing the focus ring of the text, + // since there's no borders or scrollbars. The `offset` variant was chosen + // mostly because it rounds the value to `int`, so the value is less + // volatile and therefore would need fewer updates. + // + // getBoundingClientRect() was considered and rejected, because it provides + // the rect in screen coordinates but this scale adjustment needs to be + // local. + final double domWidth = update.representation._domText.offsetWidth; + final double domHeight = update.representation._domText.offsetHeight; + measurements.add(( + representation: update.representation, + domSize: ui.Size(domWidth, domHeight), + targetSize: update.targetSize, + )); + } + + // Step 3: update all spans at a batch without taking any further DOM + // measurements, which avoids additional reflows. + for (final _Measurement measurement in measurements) { + final SizedSpanRepresentation representation = measurement.representation; + final double domWidth = measurement.domSize.width; + final double domHeight = measurement.domSize.height; + final ui.Size targetSize = measurement.targetSize; + + // Reset back to `inline-block` (it was set to `inline` in Step 1). + representation._domText.style.display = 'inline-block'; + + if (domWidth < 1 && domHeight < 1) { + // Don't bother dealing with divisions by tiny numbers. This probably means + // the label is empty or doesn't contain anything that would be visible to + // the user. + representation._domText.style.transform = ''; + } else { + final double scaleX = targetSize.width / domWidth; + final double scaleY = targetSize.height / domHeight; + representation._domText.style.transform = 'scale($scaleX, $scaleY)'; + } + } + + assert(_resizeQueue == null, '_resizeQueue must be empty after it is processed.'); + } + + // The structure of the sized span label looks like this: + // + // + // Here goes the label + // + // + // The target of the focus should be the , not the . + // Otherwise the browser will report the node as two separate nodes to the + // screen reader. It would require the user to make an additional navigation + // action to "step over" the to reach the where the + // text is. However, logically this DOM structure is just "one thing" as far + // as the user is concerned, so both `tabindex` and the text of the label + // should go on the same element. + @override + DomElement get focusTarget => _domText; } /// Renders [SemanticsObject.label] and/or [SemanticsObject.value] to the semantics DOM. @@ -28,46 +431,50 @@ enum LeafLabelRepresentation { /// interactive controls. In such case the value is reported via that element's /// `value` attribute rather than rendering it separately. class LabelAndValue extends RoleManager { - LabelAndValue(SemanticsObject semanticsObject, PrimaryRoleManager owner, { required this.labelRepresentation }) + LabelAndValue(SemanticsObject semanticsObject, PrimaryRoleManager owner, { required this.preferredRepresentation }) : super(Role.labelAndValue, semanticsObject, owner); - /// Configures the representation of the label in the DOM. - final LeafLabelRepresentation labelRepresentation; + /// The preferred representation of the label in the DOM. + /// + /// This value may be changed. Calling [update] after changing it will apply + /// the new preference. + /// + /// If the node contains children, [LabelRepresentation.ariaLabel] is used + /// instead. + LabelRepresentation preferredRepresentation; @override void update() { final String? computedLabel = _computeLabel(); if (computedLabel == null) { - _oldLabel = null; _cleanUpDom(); return; } - _updateLabel(computedLabel); + _getEffectiveRepresentation().update(computedLabel); } - DomText? _domText; - String? _oldLabel; + LabelRepresentationBehavior? _representation; - void _updateLabel(String label) { - if (label == _oldLabel) { - return; - } - _oldLabel = label; - - final bool needsDomText = labelRepresentation == LeafLabelRepresentation.domText && !semanticsObject.hasChildren; - - _domText?.remove(); - if (needsDomText) { - owner.removeAttribute('aria-label'); - final DomText domText = domDocument.createTextNode(label); - _domText = domText; - semanticsObject.element.appendChild(domText); - } else { - owner.setAttribute('aria-label', label); - _domText = null; + /// Return the representation that should be used based on the current + /// parameters of the semantic node. + /// + /// If the node has children always use an `aria-label`. Using extra child + /// nodes to represent the label will cause layout shifts and confuse the + /// screen reader. If the are no children, use the representation preferred + /// by the primary role manager. + LabelRepresentationBehavior _getEffectiveRepresentation() { + final LabelRepresentation effectiveRepresentation = semanticsObject.hasChildren + ? LabelRepresentation.ariaLabel + : preferredRepresentation; + + LabelRepresentationBehavior? representation = _representation; + if (representation == null || representation.kind != effectiveRepresentation) { + representation?.cleanUp(); + _representation = representation = effectiveRepresentation.createBehavior(owner); } + return representation; } /// Computes the final label to be assigned to the node. @@ -88,8 +495,7 @@ class LabelAndValue extends RoleManager { } void _cleanUpDom() { - owner.removeAttribute('aria-label'); - _domText?.remove(); + _representation?.cleanUp(); } @override @@ -97,6 +503,18 @@ class LabelAndValue extends RoleManager { super.dispose(); _cleanUpDom(); } + + /// Moves the focus to the element that carries the semantic label. + /// + /// Typically a node would be [Focusable] and focus request would be satisfied + /// by transfering focus through the normal focusability features. However, + /// sometimes accessibility focus needs to be moved to a non-focusable node, + /// such as the title of a dialog. This method handles that situation. + /// Different label representations use different DOM structures, so the + /// actual work is delegated to [LabelRepresentationBehavior]. + void focusAsRouteDefault() { + _getEffectiveRepresentation().focusAsRouteDefault(); + } } String? computeDomSemanticsLabel({ diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/link.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/link.dart index d9f93078f0..87a2a47a07 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/link.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/link.dart @@ -10,7 +10,7 @@ class Link extends PrimaryRoleManager { Link(SemanticsObject semanticsObject) : super.withBasics( PrimaryRole.link, semanticsObject, - labelRepresentation: LeafLabelRepresentation.domText, + preferredLabelRepresentation: LabelRepresentation.domText, ) { addTappable(); } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/platform_view.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/platform_view.dart index 029a7dd71f..89b61507a7 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/platform_view.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/platform_view.dart @@ -25,7 +25,7 @@ class PlatformViewRoleManager extends PrimaryRoleManager { : super.withBasics( PrimaryRole.platformView, semanticsObject, - labelRepresentation: LeafLabelRepresentation.ariaLabel, + preferredLabelRepresentation: LabelRepresentation.ariaLabel, ); @override diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart index bb42bbc822..45699c0fa1 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart @@ -27,15 +27,8 @@ class Scrollable extends PrimaryRoleManager { : super.withBasics( PrimaryRole.scrollable, semanticsObject, - labelRepresentation: LeafLabelRepresentation.ariaLabel, - ) { - _scrollOverflowElement.style - ..position = 'absolute' - ..transformOrigin = '0 0 0' - // Ignore pointer events since this is a dummy element. - ..pointerEvents = 'none'; - append(_scrollOverflowElement); - } + preferredLabelRepresentation: LabelRepresentation.ariaLabel, + ); /// Disables browser-driven scrolling in the presence of pointer events. GestureModeCallback? _gestureModeListener; @@ -97,6 +90,20 @@ class Scrollable extends PrimaryRoleManager { } } + @override + void initState() { + // Scrolling is controlled by setting overflow-y/overflow-x to 'scroll`. The + // default overflow = "visible" needs to be unset. + semanticsObject.element.style.overflow = ''; + + _scrollOverflowElement.style + ..position = 'absolute' + ..transformOrigin = '0 0 0' + // Ignore pointer events since this is a dummy element. + ..pointerEvents = 'none'; + append(_scrollOverflowElement); + } + @override void update() { super.update(); 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 a62f99eef1..505531afcc 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 @@ -434,12 +434,12 @@ abstract class PrimaryRoleManager { /// /// If `labelRepresentation` is true, configures the [LabelAndValue] role with /// [LabelAndValue.labelRepresentation] set to true. - PrimaryRoleManager.withBasics(this.role, this.semanticsObject, { required LeafLabelRepresentation labelRepresentation }) { + PrimaryRoleManager.withBasics(this.role, this.semanticsObject, { required LabelRepresentation preferredLabelRepresentation }) { element = _initElement(createElement(), semanticsObject); addFocusManagement(); addLiveRegion(); addRouteName(); - addLabelAndValue(labelRepresentation: labelRepresentation); + addLabelAndValue(preferredRepresentation: preferredLabelRepresentation); } /// Initializes a blank role for a [semanticsObject]. @@ -475,7 +475,9 @@ abstract class PrimaryRoleManager { static DomElement _initElement(DomElement element, SemanticsObject semanticsObject) { // DOM nodes created for semantics objects are positioned absolutely using // transforms. - element.style.position = 'absolute'; + element.style + ..position = 'absolute' + ..overflow = 'visible'; element.setAttribute('id', 'flt-semantic-node-${semanticsObject.id}'); // The root node has some properties that other nodes do not. @@ -502,6 +504,20 @@ abstract class PrimaryRoleManager { return element; } + /// A lifecycle method called after the DOM [element] for this role manager + /// is initialized, and the association with the corresponding + /// [SemanticsObject] established. + /// + /// Override this method to implement expensive one-time initialization of a + /// role manager's state. It is more efficient to do such work in this method + /// compared to [update], because [update] can be called many times during the + /// lifecycle of the semantic node. + /// + /// It is safe to access [element], [semanticsObject], [secondaryRoleManagers] + /// and all helper methods that access these fields, such as [append], + /// [focusable], etc. + void initState() {} + /// Sets the `role` ARIA attribute. void setAriaRole(String ariaRoleName) { setAttribute('role', ariaRoleName); @@ -541,9 +557,13 @@ abstract class PrimaryRoleManager { addSecondaryRole(RouteName(semanticsObject, this)); } + /// Convenience getter for the [LabelAndValue] role manager, if any. + LabelAndValue? get labelAndValue => _labelAndValue; + LabelAndValue? _labelAndValue; + /// Adds generic label features. - void addLabelAndValue({ required LeafLabelRepresentation labelRepresentation }) { - addSecondaryRole(LabelAndValue(semanticsObject, this, labelRepresentation: labelRepresentation)); + void addLabelAndValue({ required LabelRepresentation preferredRepresentation }) { + addSecondaryRole(_labelAndValue = LabelAndValue(semanticsObject, this, preferredRepresentation: preferredRepresentation)); } /// Adds generic functionality for handling taps and clicks. @@ -624,7 +644,10 @@ final class GenericRole extends PrimaryRoleManager { GenericRole(SemanticsObject semanticsObject) : super.withBasics( PrimaryRole.generic, semanticsObject, - labelRepresentation: LeafLabelRepresentation.domText, + // Prefer sized span because if this is a leaf it is frequently a Text widget. + // But if it turns out to be a container, then LabelAndValue will automatically + // switch to `aria-label`. + preferredLabelRepresentation: LabelRepresentation.sizedSpan, ) { // Typically a tappable widget would have a more specific role, such as // "link", "button", "checkbox", etc. However, there are situations when a @@ -639,42 +662,40 @@ final class GenericRole extends PrimaryRoleManager { @override void update() { - super.update(); - if (!semanticsObject.hasLabel) { // The node didn't get a more specific role, and it has no label. It is // likely that this node is simply there for positioning its children and // has no other role for the screen reader to be aware of. In this case, // the element does not need a `role` attribute at all. + super.update(); return; } - // Assign one of three roles to the element: heading, group, text. + // Assign one of three roles to the element: group, heading, 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 a node has a label and no children, assume is a paragraph of text. + // In HTML text has no ARIA role. It's just a DOM node with text inside + // it. Previously, role="text" was used, but it was only supported by + // Safari, and it was removed starting Safari 17. if (semanticsObject.hasChildren) { + labelAndValue!.preferredRepresentation = LabelRepresentation.ariaLabel; setAriaRole('group'); } else if (semanticsObject.hasFlag(ui.SemanticsFlag.isHeader)) { + labelAndValue!.preferredRepresentation = LabelRepresentation.domText; setAriaRole('heading'); } else { - setAriaRole('text'); + labelAndValue!.preferredRepresentation = LabelRepresentation.sizedSpan; + removeAttribute('role'); } + + // Call super.update last so the role is established before applying + // specific behaviors. + super.update(); } @override @@ -694,18 +715,9 @@ final class GenericRole extends PrimaryRoleManager { return false; } - // Case 3: current node is visual/informational. Move just the - // accessibility focus. - - // Plain text nodes should not be focusable via keyboard or mouse. They are - // only focusable for the purposes of focusing the screen reader. To achieve - // this the -1 value is used. - // - // See also: - // - // https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/tabindex - element.tabIndex = -1; - element.focus(); + // Case 3: current node is visual/informational. Move just the accessibility + // focus. + labelAndValue!.focusAsRouteDefault(); return true; } } @@ -1645,6 +1657,7 @@ class SemanticsObject { if (currentPrimaryRole == null) { currentPrimaryRole = _createPrimaryRole(roleId); primaryRole = currentPrimaryRole; + currentPrimaryRole.initState(); currentPrimaryRole.update(); } 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 f99faab8f5..d5f58d6a6c 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 @@ -10,7 +10,7 @@ class Button extends PrimaryRoleManager { Button(SemanticsObject semanticsObject) : super.withBasics( PrimaryRole.button, semanticsObject, - labelRepresentation: LeafLabelRepresentation.domText, + preferredLabelRepresentation: LabelRepresentation.domText, ) { addTappable(); setAriaRole('button'); diff --git a/engine/src/flutter/lib/web_ui/test/common/matchers.dart b/engine/src/flutter/lib/web_ui/test/common/matchers.dart index a94f5b480b..2f1c19ce20 100644 --- a/engine/src/flutter/lib/web_ui/test/common/matchers.dart +++ b/engine/src/flutter/lib/web_ui/test/common/matchers.dart @@ -398,18 +398,68 @@ class HtmlPatternMatcher extends Matcher { } } + // Removes nodes that are not interesting for comparison purposes. + // + // In particular, removes non-leaf white space Text nodes between elements, as + // these are typically not interesting to test for. It's strictly not correct + // to ignore it entirely. For example, in the presence of a
 tag or CSS
+  // `white-space: pre` white space does matter, but Flutter Web doesn't use
+  // them, at least not in tests, so it's OK to ignore.
+  List _cleanUpNodeList(html.NodeList nodeList) {
+    final List cleanNodes = [];
+    for (int i = 0; i < nodeList.length; i++) {
+      final html.Node node = nodeList[i];
+      assert(
+        node is html.Element || node is html.Text,
+        'Unsupported node type ${node.runtimeType}. Only Element and Text nodes are supported',
+      );
+
+      final bool hasSiblings = nodeList.length > 1;
+      final bool isWhitespace = node is html.Text && node.data.trim().isEmpty;
+
+      if (hasSiblings && isWhitespace) {
+        // Ignore white space between elements, e.g. 
+ // | | | + // ignore | | + // | | + // compare | + // ignore + continue; + } + + cleanNodes.add(node); + } + return cleanNodes; + } + void matchChildren(_Breadcrumbs parent, List mismatches, html.Element element, html.Element pattern) { - if (element.children.length != pattern.children.length) { + final List actualChildNodes = _cleanUpNodeList(element.nodes); + final List expectedChildNodes = _cleanUpNodeList(pattern.nodes); + + if (actualChildNodes.length != expectedChildNodes.length) { mismatches.add( - '$parent: expected ${pattern.children.length} children, but found ${element.children.length}.' + '$parent: expected ${expectedChildNodes.length} child nodes, but found ${actualChildNodes.length}.' ); return; } - for (int i = 0; i < pattern.children.length; i++) { - final html.Element expectedChild = pattern.children[i]; - final html.Element actualChild = element.children[i]; - matchElements(parent, mismatches, actualChild, expectedChild); + for (int i = 0; i < expectedChildNodes.length; i++) { + final html.Node expectedChild = expectedChildNodes[i]; + final html.Node actualChild = actualChildNodes[i]; + + if (expectedChild is html.Element && actualChild is html.Element) { + matchElements(parent, mismatches, actualChild, expectedChild); + } else if (expectedChild is html.Text && actualChild is html.Text) { + if (expectedChild.data != actualChild.data) { + mismatches.add( + '$parent: expected text content "${expectedChild.data}", but found "${actualChild.data}".' + ); + } + } else { + mismatches.add( + '$parent: expected child type ${expectedChild.runtimeType}, but found ${actualChild.runtimeType}.' + ); + } } } diff --git a/engine/src/flutter/lib/web_ui/test/engine/matchers_test.dart b/engine/src/flutter/lib/web_ui/test/engine/matchers_test.dart index 4c2ce65ac6..b305d92553 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/matchers_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/matchers_test.dart @@ -44,6 +44,53 @@ Specifically: ); }); + test('trivial equal text content', () { + expectDom( + '
hello
', + hasHtml('
hello
'), + ); + }); + + test('trivial unequal text content', () { + expectDom( + '
hello
', + expectMismatch( + hasHtml('
world
'), + ''' +The following DOM structure did not match the expected pattern: +
hello
+ +Specifically: + - @div: expected text content "world", but found "hello".''', + ), + ); + }); + + test('white space between elements', () { + expectDom( + ' ', + hasHtml(' '), + ); + + expectDom( + ' ', + hasHtml(' '), + ); + + expectDom( + ' ', + expectMismatch( + hasHtml(' '), + ''' +The following DOM structure did not match the expected pattern: + + +Specifically: + - @a > b: expected text content " ", but found " ".''', + ), + ); + }); + test('trivial equal attributes', () { expectDom( '
', @@ -192,7 +239,7 @@ The following DOM structure did not match the expected pattern:

Specifically: - - @div: expected 3 children, but found 2.''', + - @div: expected 3 child nodes, but found 2.''', ), ); }); @@ -207,7 +254,7 @@ The following DOM structure did not match the expected pattern:

Specifically: - - @div: expected 2 children, but found 3.''', + - @div: expected 2 child nodes, but found 3.''', ), ); }); 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 98ce4c9b62..2b0ce16119 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 @@ -123,7 +123,7 @@ void _testRoleManagerLifecycle() { ); tester.apply(); - tester.expectSemantics(''); + tester.expectSemantics(''); final SemanticsObject node = owner().debugSemanticsTree![0]!; expect(node.primaryRole?.role, PrimaryRole.button); @@ -146,7 +146,7 @@ void _testRoleManagerLifecycle() { ); tester.apply(); - tester.expectSemantics('a label'); + tester.expectSemantics('a label'); final SemanticsObject node = owner().debugSemanticsTree![0]!; expect(node.primaryRole?.role, PrimaryRole.button); @@ -320,6 +320,30 @@ void _testEngineSemanticsOwner() { expect(copy.reduceMotion, true); }); + test('makes the semantic DOM tree invisible', () { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + final SemanticsTester tester = SemanticsTester(owner()); + tester.updateNode( + id: 0, + label: 'I am root', + rect: const ui.Rect.fromLTRB(0, 0, 100, 50), + ); + tester.apply(); + + expectSemanticsTree( + owner(), + ''' + + I am root +''', + ); + + semantics().semanticsEnabled = false; + }); + void renderSemantics({String? label, String? tooltip, Set flags = const {}}) { int flagValues = 0; for (final ui.SemanticsFlag flag in flags) { @@ -363,9 +387,9 @@ void _testEngineSemanticsOwner() { expect(tree[1]!.label, 'Hello'); expectSemanticsTree(owner(), ''' - + - Hello + Hello '''); @@ -373,9 +397,9 @@ void _testEngineSemanticsOwner() { renderLabel('World'); expectSemanticsTree(owner(), ''' - + - World + World '''); @@ -383,9 +407,9 @@ void _testEngineSemanticsOwner() { renderLabel(''); expectSemanticsTree(owner(), ''' - + - + '''); @@ -406,9 +430,9 @@ void _testEngineSemanticsOwner() { final DomElement existingParent = tree[1]!.element.parent!; expectSemanticsTree(owner(), ''' - + - Hello + Hello '''); @@ -421,7 +445,7 @@ void _testEngineSemanticsOwner() { expect(tree[1]!.label, 'Hello'); expect(tree[1]!.element.tagName.toLowerCase(), 'a'); expectSemanticsTree(owner(), ''' - + Hello @@ -445,9 +469,9 @@ void _testEngineSemanticsOwner() { expect(tree[1]!.tooltip, 'tooltip'); expectSemanticsTree(owner(), ''' - + - tooltip + tooltip '''); @@ -455,9 +479,9 @@ void _testEngineSemanticsOwner() { renderSemantics(label: 'Hello', tooltip: 'tooltip'); expectSemanticsTree(owner(), ''' - + - tooltip\nHello + tooltip\nHello '''); @@ -465,9 +489,9 @@ void _testEngineSemanticsOwner() { renderSemantics(); expectSemanticsTree(owner(), ''' - + - + '''); @@ -660,7 +684,7 @@ void _testHeader() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' -Header of the page +Header of the page '''); semantics().semanticsEnabled = false; @@ -696,7 +720,7 @@ void _testHeader() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -764,7 +788,7 @@ void _testText() { expectSemanticsTree( owner(), - '''plain text''', + '''plain text''', ); final SemanticsObject node = owner().debugSemanticsTree![0]!; @@ -797,7 +821,7 @@ void _testText() { expectSemanticsTree( owner(), - '''tappable text''', + '''tappable text''', ); final SemanticsObject node = owner().debugSemanticsTree![0]!; @@ -925,7 +949,7 @@ void _testContainer() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -976,7 +1000,7 @@ void _testContainer() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1021,7 +1045,7 @@ void _testContainer() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1072,7 +1096,7 @@ void _testContainer() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1092,7 +1116,7 @@ void _testContainer() { ); owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1112,7 +1136,7 @@ void _testContainer() { ); owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1132,7 +1156,7 @@ void _testContainer() { ); owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1163,7 +1187,7 @@ void _testContainer() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1215,7 +1239,7 @@ void _testContainer() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1252,7 +1276,7 @@ void _testContainer() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1287,7 +1311,7 @@ void _testVerticalScrolling() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); @@ -1319,7 +1343,7 @@ void _testVerticalScrolling() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1376,7 +1400,7 @@ void _testVerticalScrolling() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1440,7 +1464,7 @@ void _testHorizontalScrolling() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); @@ -1470,7 +1494,7 @@ void _testHorizontalScrolling() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1527,7 +1551,7 @@ void _testHorizontalScrolling() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -1591,7 +1615,7 @@ void _testIncrementables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); @@ -1624,7 +1648,7 @@ void _testIncrementables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); @@ -1657,7 +1681,7 @@ void _testIncrementables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); @@ -1692,7 +1716,7 @@ void _testIncrementables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); @@ -1770,7 +1794,7 @@ void _testTextField() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); @@ -1856,7 +1880,7 @@ void _testCheckables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); final SemanticsObject node = owner().debugSemanticsTree![0]!; @@ -1889,7 +1913,7 @@ void _testCheckables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -1914,7 +1938,7 @@ void _testCheckables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -1940,7 +1964,7 @@ void _testCheckables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -1965,7 +1989,7 @@ void _testCheckables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -1990,7 +2014,7 @@ void _testCheckables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -2017,7 +2041,7 @@ void _testCheckables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -2043,7 +2067,7 @@ void _testCheckables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -2069,7 +2093,7 @@ void _testCheckables() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -2154,7 +2178,7 @@ void _testTappable() { tester.apply(); expectSemanticsTree(owner(), ''' - + '''); final SemanticsObject node = owner().debugSemanticsTree![0]!; @@ -2186,7 +2210,7 @@ void _testTappable() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -2213,25 +2237,25 @@ void _testTappable() { updateTappable(enabled: false); expectSemanticsTree( owner(), - '' + '' ); updateTappable(enabled: true); expectSemanticsTree( owner(), - '', + '', ); updateTappable(enabled: false); expectSemanticsTree( owner(), - '', + '', ); updateTappable(enabled: true); expectSemanticsTree( owner(), - '', + '', ); semantics().semanticsEnabled = false; @@ -2349,7 +2373,7 @@ void _testTappable() { tester.apply(); expectSemanticsTree(owner(), ''' - + @@ -2411,7 +2435,7 @@ void _testImage() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -2441,9 +2465,8 @@ void _testImage() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - - - + + @@ -2468,7 +2491,7 @@ void _testImage() { owner().updateSemantics(builder.build()); expectSemanticsTree( owner(), - '', + '', ); semantics().semanticsEnabled = false; @@ -2497,9 +2520,8 @@ void _testImage() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - - - + + @@ -2632,7 +2654,7 @@ void _testPlatformView() { owner().updateSemantics(builder.build()); expectSemanticsTree( owner(), - '', + '', ); } @@ -2647,7 +2669,7 @@ void _testPlatformView() { owner().updateSemantics(builder.build()); expectSemanticsTree( owner(), - '', + '', ); } @@ -2669,7 +2691,7 @@ void _testPlatformView() { expectSemanticsTree( owner(), - '', + '', ); final DomElement element = owner().semanticsHost.querySelector('flt-semantics')!; expect(element.style.pointerEvents, 'none'); @@ -2751,7 +2773,7 @@ void _testPlatformView() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + @@ -2855,7 +2877,7 @@ void _testGroup() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); semantics().semanticsEnabled = false; @@ -2887,7 +2909,7 @@ void _testDialog() { owner().updateSemantics(builder.build()); expectSemanticsTree(owner(), ''' - + '''); expect( @@ -2932,7 +2954,7 @@ void _testDialog() { // But still sets the dialog role. expectSemanticsTree(owner(), ''' - + '''); expect( @@ -2970,11 +2992,11 @@ void _testDialog() { tester.apply(); expectSemanticsTree(owner(), ''' - + - $label + $label @@ -3019,7 +3041,7 @@ void _testDialog() { tester.apply(); expectSemanticsTree(owner(), ''' - + '''); expect( @@ -3059,11 +3081,11 @@ void _testDialog() { tester.apply(); expectSemanticsTree(owner(), ''' - + - Hello + Hello @@ -3255,9 +3277,24 @@ void _testDialog() { expect(capturedActions, isEmpty); // However, the element should have gotten the focus. - final DomElement element = owner().debugSemanticsTree![2]!.element; - expect(element.tabIndex, -1); - expect(domDocument.activeElement, element); + + tester.expectSemantics(''' + + + + + + Heading + + Click me! + + + +'''); + + final DomElement span = owner().debugSemanticsTree![2]!.element.querySelectorAll('span').single; + expect(span.tabIndex, -1); + expect(domDocument.activeElement, span); semantics().semanticsEnabled = false; }); @@ -3420,9 +3457,9 @@ void _testFocusable() { } expectSemanticsTree(owner(), ''' - + - focusable text + focusable text '''); diff --git a/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_tester.dart b/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_tester.dart index 6c2fcdcb6c..aedda526b9 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_tester.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_tester.dart @@ -13,11 +13,6 @@ import 'package:ui/ui.dart' as ui; import '../../common/matchers.dart'; -/// CSS style applied to the root of the semantics tree. -// TODO(yjbanov): this should be handled internally by [expectSemanticsTree]. -// No need for every test to inject it. -const String rootSemanticStyle = 'filter: opacity(0%); color: rgba(0, 0, 0, 0)'; - /// A convenience wrapper of the semantics API for building and inspecting the /// semantics tree in unit tests. class SemanticsTester { diff --git a/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_crawler_test.dart b/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_text_test.dart similarity index 58% rename from engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_crawler_test.dart rename to engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_text_test.dart index 78b4c581ef..6f9bc43f63 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_crawler_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_text_test.dart @@ -14,7 +14,6 @@ import '../../common/rendering.dart'; import '../../common/test_initialization.dart'; import 'semantics_tester.dart'; -const String _rootStyle = 'style="filter: opacity(0%); color: rgba(0, 0, 0, 0)"'; DateTime _testTime = DateTime(2023, 2, 17); EngineSemantics semantics() => EngineSemantics.instance; EngineSemanticsOwner owner() => EnginePlatformDispatcher.instance.implicitView!.semantics; @@ -46,7 +45,7 @@ Future testMain() async { tester.apply(); expectSemanticsTree(owner(), ''' - Hello''' + Hello''' ); final SemanticsObject node = owner().debugSemanticsTree![0]!; @@ -58,7 +57,7 @@ Future testMain() async { ); } - // Change label - expect both and aria-label to be updated. + // Change label - expect the to be updated. { final SemanticsTester tester = SemanticsTester(owner()); tester.updateNode( @@ -70,7 +69,7 @@ Future testMain() async { tester.apply(); expectSemanticsTree(owner(), ''' - World''' + World''' ); } @@ -85,7 +84,7 @@ Future testMain() async { ); tester.apply(); - expectSemanticsTree(owner(), ''); + expectSemanticsTree(owner(), ''); } semantics().semanticsEnabled = false; @@ -114,9 +113,9 @@ Future testMain() async { tester.apply(); expectSemanticsTree(owner(), ''' - + - I am a child + I am a child ''' ); @@ -141,7 +140,7 @@ Future testMain() async { tester.apply(); expectSemanticsTree(owner(), ''' - I am a leaf''' + I am a leaf''' ); } @@ -165,9 +164,9 @@ Future testMain() async { tester.apply(); expectSemanticsTree(owner(), ''' - + - I am a child + I am a child ''' ); @@ -185,10 +184,105 @@ Future testMain() async { tester.apply(); expectSemanticsTree(owner(), ''' - I am a leaf again''' + I am a leaf again''' ); } semantics().semanticsEnabled = false; }); + + test('focusAsRouteDefault focuses on when sized span is used', () async { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + final SemanticsTester tester = SemanticsTester(owner()); + tester.updateNode( + id: 0, + label: 'Hello', + transform: Matrix4.identity().toFloat64(), + rect: const ui.Rect.fromLTRB(0, 0, 100, 50), + ); + tester.apply(); + + expectSemanticsTree(owner(), ''' + Hello''' + ); + + final SemanticsObject node = owner().debugSemanticsTree![0]!; + final DomElement span = node.element.querySelector('span')!; + + expect(span.getAttribute('tabindex'), isNull); + node.primaryRole!.focusAsRouteDefault(); + expect(span.getAttribute('tabindex'), '-1'); + expect(domDocument.activeElement, span); + + semantics().semanticsEnabled = false; + }); + + test('focusAsRouteDefault focuses on when DOM text is used', () async { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + final SemanticsTester tester = SemanticsTester(owner()); + tester.updateNode( + id: 0, + label: 'Hello', + transform: Matrix4.identity().toFloat64(), + rect: const ui.Rect.fromLTRB(0, 0, 100, 50), + ); + tester.apply(); + + final SemanticsObject node = owner().debugSemanticsTree![0]!; + + // Set DOM text as preferred representation + final LabelAndValue lav = node.primaryRole!.labelAndValue!; + lav.preferredRepresentation = LabelRepresentation.domText; + lav.update(); + + expectSemanticsTree(owner(), ''' + Hello''' + ); + + expect(node.element.getAttribute('tabindex'), isNull); + node.primaryRole!.focusAsRouteDefault(); + expect(node.element.getAttribute('tabindex'), '-1'); + expect(domDocument.activeElement, node.element); + + semantics().semanticsEnabled = false; + }); + + test('focusAsRouteDefault focuses on when aria-label is used', () async { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + final SemanticsTester tester = SemanticsTester(owner()); + tester.updateNode( + id: 0, + label: 'Hello', + transform: Matrix4.identity().toFloat64(), + rect: const ui.Rect.fromLTRB(0, 0, 100, 50), + ); + tester.apply(); + + final SemanticsObject node = owner().debugSemanticsTree![0]!; + + // Set DOM text as preferred representation + final LabelAndValue lav = node.primaryRole!.labelAndValue!; + lav.preferredRepresentation = LabelRepresentation.ariaLabel; + lav.update(); + + expectSemanticsTree(owner(), ''' + ''' + ); + + expect(node.element.getAttribute('tabindex'), isNull); + node.primaryRole!.focusAsRouteDefault(); + expect(node.element.getAttribute('tabindex'), '-1'); + expect(domDocument.activeElement, node.element); + + semantics().semanticsEnabled = false; + }); } diff --git a/engine/src/flutter/lib/web_ui/test/engine/semantics/text_field_test.dart b/engine/src/flutter/lib/web_ui/test/engine/semantics/text_field_test.dart index e78e219b41..aee5e1b331 100644 --- a/engine/src/flutter/lib/web_ui/test/engine/semantics/text_field_test.dart +++ b/engine/src/flutter/lib/web_ui/test/engine/semantics/text_field_test.dart @@ -93,7 +93,7 @@ void testMain() { createTextFieldSemantics(value: 'hello'); expectSemanticsTree(owner(), ''' - + ''');