From 3b7f799a99f350afc52385c5b973b99d17b02741 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Fri, 15 Apr 2022 10:40:53 -0700 Subject: [PATCH 1/5] [web:a11y] implement traversal and hit-test orders --- lib/web_ui/lib/src/engine/embedder.dart | 18 +- lib/web_ui/lib/src/engine/html/picture.dart | 15 +- .../lib/src/engine/semantics/semantics.dart | 381 +++++++++++------- .../test/engine/semantics/semantics_test.dart | 211 +++++++++- .../engine/semantics/semantics_tester.dart | 3 +- lib/web_ui/test/matchers.dart | 14 +- 6 files changed, 472 insertions(+), 170 deletions(-) diff --git a/lib/web_ui/lib/src/engine/embedder.dart b/lib/web_ui/lib/src/engine/embedder.dart index 834af36a8570d..c07a48e4a64de 100644 --- a/lib/web_ui/lib/src/engine/embedder.dart +++ b/lib/web_ui/lib/src/engine/embedder.dart @@ -290,9 +290,20 @@ class FlutterViewEmbedder { .prepareAccessibilityPlaceholder(); glassPaneElementHostNode.nodes.addAll([ - semanticsHostElement, _accessibilityPlaceholder, _sceneHostElement!, + + // The semantic host goes last because hit-test order-wise we want it to + // be first. If semantics goes under the scene host, platform views will + // obscure semantic elements. + // + // You may be wondering: wouldn't semantics obscure platform views and + // make then not accessible? At least with some careful planning, that + // should not be the case. The semantics tree makes all of its non-leaf + // elements transparent. This way, if a platform view appears among other + // interactive Flutter widgets, as long as those widgets do not intersect + // with the platform view, the platform view will be reachable. + semanticsHostElement, ]); // When debugging semantics, make the scene semi-transparent so that the @@ -304,11 +315,6 @@ class FlutterViewEmbedder { PointerBinding.initInstance(glassPaneElement); KeyboardBinding.initInstance(glassPaneElement); - // Hide the DOM nodes used to render the scene from accessibility, because - // the accessibility tree is built from the SemanticsNode tree as a parallel - // DOM tree. - _sceneHostElement!.setAttribute('aria-hidden', 'true'); - if (html.window.visualViewport == null && isWebKit) { // Older Safari versions sometimes give us bogus innerWidth/innerHeight // values when the page loads. When it changes the values to correct ones diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index b105c998e81ac..925e93025a1a7 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -122,7 +122,20 @@ class PersistedPicture extends PersistedLeafSurface { @override html.Element createElement() { - return defaultCreateElement('flt-picture'); + final html.Element element = defaultCreateElement('flt-picture'); + + // The DOM elements used to render pictures are used purely to put pixels on + // the screen. They have no semantic information. If an assistive technology + // attempts to scan picture content it will look like garbage and confuse + // users. UI semantics are exported as a separate DOM tree rendered parallel + // to pictures. + // + // Why do we not hide layer and scene elements from ARIA? Because those + // elements may contain platform views, and we do want platform views to be + // accessible. + element.setAttribute('aria-hidden', 'true'); + + return element; } @override diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index 5ae98324c7a94..c71a655e0fac8 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -274,6 +274,7 @@ class SemanticsObject { // DOM nodes created for semantics objects are positioned absolutely using // transforms. element.style.position = 'absolute'; + element.setAttribute('id', 'flt-semantic-node-$id'); // The root node has some properties that other nodes do not. if (id == 0 && !configuration.debugShowSemanticsNodes) { @@ -601,6 +602,19 @@ class SemanticsObject { _dirtyFields |= _tooltipIndex; } + /// See [ui.SemanticsUpdateBuilder.updateNode]. + int get platformViewId => _platformViewId; + int _platformViewId = -1; + + /// Whether this object represents a platform view. + bool get isPlatformView => _platformViewId != -1; + + static const int _platformViewIdIndex = 1 << 23; + + void _markPlatformViewIdDirty() { + _dirtyFields |= _platformViewIdIndex; + } + /// A unique permanent identifier of the semantics node in the tree. final int id; @@ -632,7 +646,11 @@ class SemanticsObject { html.Element? getOrCreateChildContainer() { if (_childContainerElement == null) { _childContainerElement = html.Element.tag('flt-semantics-container'); - _childContainerElement!.style.position = 'absolute'; + _childContainerElement!.style + ..position = 'absolute' + // Ignore pointer events on child container so that platform views + // behind it can be reached. + ..pointerEvents = 'none'; element.append(_childContainerElement!); } return _childContainerElement; @@ -702,9 +720,9 @@ class SemanticsObject { /// Updates this object from data received from a semantics [update]. /// - /// This method creates [SemanticsObject]s for the direct children of this - /// object. However, it does not recursively populate them. - void updateWith(SemanticsNodeUpdate update) { + /// Does not update children. Children are updated in a separate pass because + /// at this point children's self information is not ready yet. + void updateSelf(SemanticsNodeUpdate update) { // Update all field values and their corresponding dirty flags before // applying the updates to the DOM. assert(update.flags != null); // ignore: unnecessary_null_comparison @@ -838,9 +856,13 @@ class SemanticsObject { _markAdditionalActionsDirty(); } + if (_platformViewId != update.platformViewId) { + _platformViewId = update.platformViewId; + _markPlatformViewIdDirty(); + } + // Apply updates to the DOM. _updateRoles(); - _updateChildrenInTraversalOrder(); // All properties that affect positioning and sizing are checked together // any one of them triggers position and size recomputation. @@ -848,9 +870,187 @@ class SemanticsObject { recomputePositionAndSize(); } - // Make sure we create a child container only when there are children. - assert(_childContainerElement == null || hasChildren); - _dirtyFields = 0; + // Ignore pointer events on all container nodes and all platform view nodes. + // This is so that the platform views are not obscured by semantic elements + // and can be reached by inspecting the web page. + if (!hasChildren && !isPlatformView) { + element.style.pointerEvents = 'all'; + } else { + element.style.pointerEvents = 'none'; + } + } + + /// The order children are currently rendered in. + List? _currentChildrenInRenderOrder; + + /// Updates direct children of this node, if any. + /// + /// Specifies two orders of direct children: + /// + /// * Traversal order: the logical order of child nodes that establishes the + /// next and previous relationship between UI widgets. When the user + /// traverses the UI using next/previous gestures the accessibility focus + /// follows the traversal order. + /// * Hit-test order: determines the top/bottom relationship between widgets. + /// When the user is inspecting the UI using the drag gesture, the widgets + /// that appear "on top" hit-test order wise take the focus. This order is + /// communicated in the DOM using the inverse paint order, specified by the + /// z-index CSS style attribute. + void updateChildren() { + // Trivial case: remove all children. + if (_childrenInHitTestOrder == null || + _childrenInHitTestOrder!.isEmpty) { + if (_currentChildrenInRenderOrder == null || + _currentChildrenInRenderOrder!.isEmpty) { + // We must not have created a container element when child list is empty. + assert(_childContainerElement == null); + _currentChildrenInRenderOrder = null; + return; + } + + // We must have created a container element when child list is not empty. + assert(_childContainerElement != null); + + // Remove all children from this semantics object. + final int len = _currentChildrenInRenderOrder!.length; + for (int i = 0; i < len; i++) { + owner._detachObject(_currentChildrenInRenderOrder![i].id); + } + _childContainerElement!.remove(); + _childContainerElement = null; + _currentChildrenInRenderOrder = null; + return; + } + + // At this point we're guaranteed to have at least one child. + final Int32List childrenInTraversalOrder = _childrenInTraversalOrder!; + final Int32List childrenInHitTestOrder = _childrenInHitTestOrder!; + final int childCount = childrenInHitTestOrder.length; + final html.Element? containerElement = getOrCreateChildContainer(); + + assert(childrenInTraversalOrder.length == childrenInHitTestOrder.length); + + // We always render in traversal order, because the accessibility traversal + // is determined by the DOM order of elements. + final List childrenInRenderOrder = []; + for (int i = 0; i < childCount; i++) { + childrenInRenderOrder.add(owner._semanticsTree[childrenInTraversalOrder[i]]!); + } + + // The z-index determines hit testing. Technically, it also affects paint + // order. However, this does not matter because our ARIA tree is invisible. + // On top of that, it is a bad UI practice when hit test order does not match + // paint order, because human eye must be able to predict hit test order + // simply by looking at the UI (if a dialog is painted on top of a dismiss + // barrier, then tapping on anything inside the dialog should not land on + // the barrier). + final bool zIndexMatters = childCount > 1; + if (zIndexMatters) { + for (int i = 0; i < childCount; i++) { + final SemanticsObject child = owner._semanticsTree[childrenInHitTestOrder[i]]!; + + // Invert the z-index because hit-test order is inverted with respect to + // paint order. + child.element.style.zIndex = '${childCount - i}'; + } + } + + // Trivial case: previous list was empty => just populate the container. + if (_currentChildrenInRenderOrder == null || + _currentChildrenInRenderOrder!.isEmpty) { + for (final SemanticsObject child in childrenInRenderOrder) { + containerElement!.append(child.element); + owner._attachObject(parent: this, child: child); + } + _currentChildrenInRenderOrder = childrenInRenderOrder; + return; + } + + // At this point we're guaranteed to have had a non-empty previous child list. + final List previousChildrenInRenderOrder = _currentChildrenInRenderOrder!; + final int previousCount = previousChildrenInRenderOrder.length; + + // Both non-empty case. + + // Indices into the new child list pointing at children that also exist in + // the old child list. + final List intersectionIndicesNew = []; + + // Indices into the old child list pointing at children that also exist in + // the new child list. + final List intersectionIndicesOld = []; + + int newIndex = 0; + + // The smallest of the two child list lengths. + final int minLength = math.min(previousCount, childCount); + + // Scan forward until first discrepancy. + while (newIndex < minLength && + previousChildrenInRenderOrder[newIndex] == + childrenInRenderOrder[newIndex]) { + intersectionIndicesNew.add(newIndex); + intersectionIndicesOld.add(newIndex); + newIndex += 1; + } + + // Trivial case: child lists are identical both in length and order => do nothing. + if (previousCount == childrenInRenderOrder.length && newIndex == childCount) { + return; + } + + // If child lists are not identical, continue computing the intersection + // between the two lists. + while (newIndex < childCount) { + for (int oldIndex = 0; + oldIndex < previousCount; + oldIndex += 1) { + if (previousChildrenInRenderOrder[oldIndex] == + childrenInRenderOrder[newIndex]) { + intersectionIndicesNew.add(newIndex); + intersectionIndicesOld.add(oldIndex); + break; + } + } + newIndex += 1; + } + + // The longest sub-sequence in the old list maximizes the number of children + // that do not need to be moved. + final List longestSequence = longestIncreasingSubsequence(intersectionIndicesOld); + final List stationaryIds = []; + for (int i = 0; i < longestSequence.length; i += 1) { + stationaryIds.add( + previousChildrenInRenderOrder[intersectionIndicesOld[longestSequence[i]!]].id + ); + } + + // Remove children that are no longer in the list. + for (int i = 0; i < previousCount; i++) { + if (!intersectionIndicesOld.contains(i)) { + // Child not in the intersection. Must be removed. + final int childId = previousChildrenInRenderOrder[i].id; + owner._detachObject(childId); + } + } + + html.Element? refNode; + for (int i = childCount - 1; i >= 0; i -= 1) { + final SemanticsObject child = childrenInRenderOrder[i]; + if (!stationaryIds.contains(child.id)) { + if (refNode == null) { + containerElement!.append(child.element); + } else { + containerElement!.insertBefore(child.element, refNode); + } + owner._attachObject(parent: this, child: child); + } else { + assert(child._parent == this); + } + refNode = child.element; + } + + _currentChildrenInRenderOrder = childrenInRenderOrder; } /// Populates the HTML "role" attribute based on a [condition]. @@ -1040,144 +1240,6 @@ class SemanticsObject { } } - Int32List? _previousChildrenInTraversalOrder; - - /// Updates the traversal child list of [object] from the given [update]. - /// - /// This method does not recursively update child elements' properties or - /// their grandchildren. This is handled by [updateSemantics] method walking - /// all the update nodes. - void _updateChildrenInTraversalOrder() { - // Remove all children case. - if (_childrenInTraversalOrder == null || - _childrenInTraversalOrder!.isEmpty) { - if (_previousChildrenInTraversalOrder == null || - _previousChildrenInTraversalOrder!.isEmpty) { - // We must not have created a container element when child list is empty. - assert(_childContainerElement == null); - _previousChildrenInTraversalOrder = _childrenInTraversalOrder; - return; - } - - // We must have created a container element when child list is not empty. - assert(_childContainerElement != null); - - // Remove all children from this semantics object. - final int len = _previousChildrenInTraversalOrder!.length; - for (int i = 0; i < len; i++) { - owner._detachObject(_previousChildrenInTraversalOrder![i]); - } - _previousChildrenInTraversalOrder = null; - _childContainerElement!.remove(); - _childContainerElement = null; - _previousChildrenInTraversalOrder = _childrenInTraversalOrder; - return; - } - - final html.Element? containerElement = getOrCreateChildContainer(); - - // Empty case. - if (_previousChildrenInTraversalOrder == null || - _previousChildrenInTraversalOrder!.isEmpty) { - _previousChildrenInTraversalOrder = _childrenInTraversalOrder; - for (final int id in _previousChildrenInTraversalOrder!) { - final SemanticsObject child = owner.getOrCreateObject(id); - containerElement!.append(child.element); - owner._attachObject(parent: this, child: child); - } - _previousChildrenInTraversalOrder = _childrenInTraversalOrder; - return; - } - - // Both non-empty case. - - // Indices into the new child list pointing at children that also exist in - // the old child list. - final List intersectionIndicesNew = []; - - // Indices into the old child list pointing at children that also exist in - // the new child list. - final List intersectionIndicesOld = []; - - int newIndex = 0; - - // The smallest of the two child list lengths. - final int minLength = math.min( - _previousChildrenInTraversalOrder!.length, - _childrenInTraversalOrder!.length, - ); - - // Scan forward until first discrepancy. - while (newIndex < minLength && - _previousChildrenInTraversalOrder![newIndex] == - _childrenInTraversalOrder![newIndex]) { - intersectionIndicesNew.add(newIndex); - intersectionIndicesOld.add(newIndex); - newIndex += 1; - } - - // If child lists are identical, do nothing. - if (_previousChildrenInTraversalOrder!.length == - _childrenInTraversalOrder!.length && - newIndex == _childrenInTraversalOrder!.length) { - return; - } - - // If child lists are not identical, continue computing the intersection - // between the two lists. - while (newIndex < _childrenInTraversalOrder!.length) { - for (int oldIndex = 0; - oldIndex < _previousChildrenInTraversalOrder!.length; - oldIndex += 1) { - if (_previousChildrenInTraversalOrder![oldIndex] == - _childrenInTraversalOrder![newIndex]) { - intersectionIndicesNew.add(newIndex); - intersectionIndicesOld.add(oldIndex); - break; - } - } - newIndex += 1; - } - - // The longest sub-sequence in the old list maximizes the number of children - // that do not need to be moved. - final List longestSequence = - longestIncreasingSubsequence(intersectionIndicesOld); - final List stationaryIds = []; - for (int i = 0; i < longestSequence.length; i += 1) { - stationaryIds.add(_previousChildrenInTraversalOrder![ - intersectionIndicesOld[longestSequence[i]!]]); - } - - // Remove children that are no longer in the list. - for (int i = 0; i < _previousChildrenInTraversalOrder!.length; i++) { - if (!intersectionIndicesOld.contains(i)) { - // Child not in the intersection. Must be removed. - final int childId = _previousChildrenInTraversalOrder![i]; - owner._detachObject(childId); - } - } - - html.Element? refNode; - for (int i = _childrenInTraversalOrder!.length - 1; i >= 0; i -= 1) { - final int childId = _childrenInTraversalOrder![i]; - final SemanticsObject child = owner.getOrCreateObject(childId); - if (!stationaryIds.contains(childId)) { - if (refNode == null) { - containerElement!.append(child.element); - } else { - containerElement!.insertBefore(child.element, refNode); - } - owner._attachObject(parent: this, child: child); - } else { - assert(child._parent == this); - } - refNode = child.element; - } - - _previousChildrenInTraversalOrder = _childrenInTraversalOrder; - } - @override String toString() { if (assertionsEnabled) { @@ -1586,9 +1648,21 @@ class EngineSemanticsOwner { } final SemanticsUpdate update = uiUpdate as SemanticsUpdate; + + // First, update each object's information about itself. This information is + // later used to fix the parent-child and sibling relationships between + // objects. for (final SemanticsNodeUpdate nodeUpdate in update._nodeUpdates!) { final SemanticsObject object = getOrCreateObject(nodeUpdate.id); - object.updateWith(nodeUpdate); + object.updateSelf(nodeUpdate); + } + + // Second, fix the tree structure. This is moved out into its own loop, + // because we must make sure each object's own information is up-to-date. + for (final SemanticsNodeUpdate nodeUpdate in update._nodeUpdates!) { + final SemanticsObject object = _semanticsTree[nodeUpdate.id]!; + object.updateChildren(); + object._dirtyFields = 0; } if (_rootSemanticsElement == null) { @@ -1602,11 +1676,18 @@ class EngineSemanticsOwner { assert(_semanticsTree.containsKey(0)); // must contain root node assert(() { // Validate tree - _semanticsTree.forEach((int? id, SemanticsObject? object) { - assert(id == object!.id); + _semanticsTree.forEach((int? id, SemanticsObject object) { + assert(id == object.id); + + // Dirty fields should be cleared after the tree has been finalized. + assert(object._dirtyFields == 0); + + // Make sure we create a child container only when there are children. + assert(object._childContainerElement == null || object.hasChildren); + // Ensure child ID list is consistent with the parent-child // relationship of the semantics tree. - if (object!._childrenInTraversalOrder != null) { + if (object._childrenInTraversalOrder != null) { for (final int childId in object._childrenInTraversalOrder!) { final SemanticsObject? child = _semanticsTree[childId]; if (child == null) { diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index e6bdd4fff3f7f..5c2f254d1b505 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -71,6 +71,9 @@ Future testMain() async { group('live region', () { _testLiveRegion(); }); + group('platform view', () { + _testPlatformView(); + }); } void _testEngineSemanticsOwner() { @@ -422,13 +425,17 @@ void _testContainer() { updateNode( builder, id: 0, - actions: 0, - flags: 0, transform: Matrix4.identity().toFloat64(), rect: zeroOffsetRect, childrenInHitTestOrder: Int32List.fromList([1]), childrenInTraversalOrder: Int32List.fromList([1]), ); + updateNode( + builder, + id: 1, + transform: Matrix4.identity().toFloat64(), + rect: zeroOffsetRect, + ); semantics().updateSemantics(builder.build()); expectSemanticsTree(''' @@ -477,6 +484,12 @@ void _testContainer() { childrenInHitTestOrder: Int32List.fromList([1]), childrenInTraversalOrder: Int32List.fromList([1]), ); + updateNode( + builder, + id: 1, + transform: Matrix4.identity().toFloat64(), + rect: const ui.Rect.fromLTRB(10, 10, 20, 20), + ); semantics().updateSemantics(builder.build()); expectSemanticsTree(''' @@ -514,6 +527,12 @@ void _testContainer() { childrenInHitTestOrder: Int32List.fromList([1]), childrenInTraversalOrder: Int32List.fromList([1]), ); + updateNode( + builder, + id: 1, + transform: Matrix4.identity().toFloat64(), + rect: const ui.Rect.fromLTRB(10, 10, 20, 20), + ); semantics().updateSemantics(builder.build()); if (browserEngine == BrowserEngine.edge) { @@ -555,6 +574,134 @@ void _testContainer() { semantics().semanticsEnabled = false; }); + + test('renders in traversal order, hit-tests in reverse z-index order', () async { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + // State 1: render initial tree with middle elements swapped hit-test wise + { + final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); + updateNode( + builder, + childrenInTraversalOrder: Int32List.fromList([1, 2, 3, 4]), + childrenInHitTestOrder: Int32List.fromList([1, 3, 2, 4]), + ); + + for (int id = 1; id <= 4; id++) { + updateNode(builder, id: id); + } + + semantics().updateSemantics(builder.build()); + expectSemanticsTree(''' + + + + + + + +'''); + } + + // State 2: update z-index + { + final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); + updateNode( + builder, + childrenInTraversalOrder: Int32List.fromList([1, 2, 3, 4]), + childrenInHitTestOrder: Int32List.fromList([1, 2, 3, 4]), + ); + semantics().updateSemantics(builder.build()); + expectSemanticsTree(''' + + + + + + + +'''); + } + + // State 3: update traversal order + { + final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); + updateNode( + builder, + childrenInTraversalOrder: Int32List.fromList([4, 2, 3, 1]), + childrenInHitTestOrder: Int32List.fromList([1, 2, 3, 4]), + ); + semantics().updateSemantics(builder.build()); + expectSemanticsTree(''' + + + + + + + +'''); + } + + // State 3: update both orders + { + final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); + updateNode( + builder, + childrenInTraversalOrder: Int32List.fromList([1, 3, 2, 4]), + childrenInHitTestOrder: Int32List.fromList([3, 4, 1, 2]), + ); + semantics().updateSemantics(builder.build()); + expectSemanticsTree(''' + + + + + + + +'''); + } + + semantics().semanticsEnabled = false; + }); + + test('container nodes are transparent and leaf children are opaque hit-test wise', () async { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); + updateNode( + builder, + childrenInTraversalOrder: Int32List.fromList([1, 2]), + childrenInHitTestOrder: Int32List.fromList([1, 2]), + ); + updateNode(builder, id: 1); + updateNode(builder, id: 2); + + semantics().updateSemantics(builder.build()); + expectSemanticsTree(''' + + + + + +'''); + + final html.Element root = appHostNode.querySelector('#flt-semantic-node-0')!; + expect(root.style.pointerEvents, 'none'); + + final html.Element child1 = appHostNode.querySelector('#flt-semantic-node-1')!; + expect(child1.style.pointerEvents, 'all'); + + final html.Element child2 = appHostNode.querySelector('#flt-semantic-node-2')!; + expect(child2.style.pointerEvents, 'all'); + + semantics().semanticsEnabled = false; + }); } void _testVerticalScrolling() { @@ -597,6 +744,12 @@ void _testVerticalScrolling() { childrenInHitTestOrder: Int32List.fromList([1]), childrenInTraversalOrder: Int32List.fromList([1]), ); + updateNode( + builder, + id: 1, + transform: Matrix4.identity().toFloat64(), + rect: const ui.Rect.fromLTRB(10, 10, 20, 20), + ); semantics().updateSemantics(builder.build()); expectSemanticsTree(''' @@ -670,9 +823,9 @@ void _testVerticalScrolling() { expectSemanticsTree(''' - - - + + + '''); @@ -749,6 +902,12 @@ void _testHorizontalScrolling() { childrenInHitTestOrder: Int32List.fromList([1]), childrenInTraversalOrder: Int32List.fromList([1]), ); + updateNode( + builder, + id: 1, + transform: Matrix4.identity().toFloat64(), + rect: const ui.Rect.fromLTRB(10, 10, 20, 20), + ); semantics().updateSemantics(builder.build()); expectSemanticsTree(''' @@ -803,9 +962,9 @@ void _testHorizontalScrolling() { expectSemanticsTree(''' - - - + + + '''); @@ -1396,6 +1555,12 @@ void _testImage() { childrenInHitTestOrder: Int32List.fromList([1]), childrenInTraversalOrder: Int32List.fromList([1]), ); + updateNode( + builder, + id: 1, + transform: Matrix4.identity().toFloat64(), + rect: const ui.Rect.fromLTRB(10, 10, 20, 20), + ); semantics().updateSemantics(builder.build()); expectSemanticsTree(''' @@ -1452,6 +1617,12 @@ void _testImage() { childrenInHitTestOrder: Int32List.fromList([1]), childrenInTraversalOrder: Int32List.fromList([1]), ); + updateNode( + builder, + id: 1, + transform: Matrix4.identity().toFloat64(), + rect: const ui.Rect.fromLTRB(10, 10, 20, 20), + ); semantics().updateSemantics(builder.build()); expectSemanticsTree(''' @@ -1520,6 +1691,28 @@ void _testLiveRegion() { }); } +void _testPlatformView() { + test('is transparent w.r.t. hit testing', () async { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); + updateNode( + builder, + platformViewId: 5, + rect: const ui.Rect.fromLTRB(0, 0, 100, 50), + ); + semantics().updateSemantics(builder.build()); + + expectSemanticsTree(''); + final html.Element element = appHostNode.querySelector('flt-semantics')!; + expect(element.style.pointerEvents, 'none'); + + semantics().semanticsEnabled = false; + }); +} + /// A facade in front of [ui.SemanticsUpdateBuilder.updateNode] that /// supplies default values for semantics attributes. // TODO(yjbanov): move this to TestSemanticsBuilder @@ -1532,7 +1725,7 @@ void updateNode( int currentValueLength = 0, int textSelectionBase = 0, int textSelectionExtent = 0, - int platformViewId = 0, + int platformViewId = -1, // -1 means not a platform view int scrollChildren = 0, int scrollIndex = 0, double scrollPosition = 0.0, diff --git a/lib/web_ui/test/engine/semantics/semantics_tester.dart b/lib/web_ui/test/engine/semantics/semantics_tester.dart index 2c5f9d49c0be5..3181043de848c 100644 --- a/lib/web_ui/test/engine/semantics/semantics_tester.dart +++ b/lib/web_ui/test/engine/semantics/semantics_tester.dart @@ -360,8 +360,9 @@ class SemanticsTester { /// Verifies the HTML structure of the current semantics tree. void expectSemanticsTree(String semanticsHtml) { + const List ignoredAttributes = ['pointer-events']; expect( - canonicalizeHtml(appHostNode.querySelector('flt-semantics')!.outerHtml!), + canonicalizeHtml(appHostNode.querySelector('flt-semantics')!.outerHtml!, ignoredAttributes: ignoredAttributes), canonicalizeHtml(semanticsHtml), ); } diff --git a/lib/web_ui/test/matchers.dart b/lib/web_ui/test/matchers.dart index 0d3d2101dbfce..f47a30f78249c 100644 --- a/lib/web_ui/test/matchers.dart +++ b/lib/web_ui/test/matchers.dart @@ -225,9 +225,12 @@ enum HtmlComparisonMode { /// [throwOnUnusedAttributes] to `true` to check that expected HTML strings do /// not contain irrelevant attributes. It is ok for actual HTML to contain all /// kinds of attributes. They only need to be filtered out before testing. -String canonicalizeHtml(String htmlContent, - {HtmlComparisonMode mode = HtmlComparisonMode.nonLayoutOnly, - bool throwOnUnusedAttributes = false}) { +String canonicalizeHtml( + String htmlContent, { + HtmlComparisonMode mode = HtmlComparisonMode.nonLayoutOnly, + bool throwOnUnusedAttributes = false, + List? ignoredAttributes, +}) { if (htmlContent.trim().isEmpty) { return ''; } @@ -331,6 +334,11 @@ String canonicalizeHtml(String htmlContent, final List parts = attr.split(':'); if (parts.length == 2) { final String name = parts.first; + + if (ignoredAttributes != null && ignoredAttributes.contains(name)) { + return null; + } + // Whether the attribute is one that's set to the same value and // never changes. Such attributes are usually not interesting to // test. From a63bdd7e75b40e761a5f8cd7d917d776aace1af1 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Fri, 15 Apr 2022 15:45:53 -0700 Subject: [PATCH 2/5] remove "we" in a bunch of places --- lib/web_ui/lib/src/engine/embedder.dart | 28 ++++---- lib/web_ui/lib/src/engine/html/picture.dart | 4 +- .../lib/src/engine/semantics/semantics.dart | 70 +++++++++---------- 3 files changed, 51 insertions(+), 51 deletions(-) diff --git a/lib/web_ui/lib/src/engine/embedder.dart b/lib/web_ui/lib/src/engine/embedder.dart index c07a48e4a64de..c64976a3eb405 100644 --- a/lib/web_ui/lib/src/engine/embedder.dart +++ b/lib/web_ui/lib/src/engine/embedder.dart @@ -83,7 +83,7 @@ class FlutterViewEmbedder { /// This element is created and inserted in the HTML DOM once. It is never /// removed or moved. /// - /// We render semantics inside the glasspane for proper focus and event + /// Render semantics inside the glasspane for proper focus and event /// handling. If semantics is behind the glasspane, the phone will disable /// focusing by touch, only by tabbing around the UI. If semantics is in /// front of glasspane, then DOM event won't bubble up to the glasspane so @@ -99,7 +99,7 @@ class FlutterViewEmbedder { html.Element? _sceneElement; /// This is state persistent across hot restarts that indicates what - /// to clear. We delay removal of old visible state to make the + /// to clear. Delay removal of old visible state to make the /// transition appear smooth. static const String _staleHotRestartStore = '__flutter_state'; List? _staleHotRestartState; @@ -133,7 +133,7 @@ class FlutterViewEmbedder { } } - /// We don't want to unnecessarily move DOM nodes around. If a DOM node is + /// Don't unnecessarily move DOM nodes around. If a DOM node is /// already in the right place, skip DOM mutation. This is both faster and /// more correct, because moving DOM nodes loses internal state, such as /// text selection. @@ -203,7 +203,7 @@ class FlutterViewEmbedder { setElementStyle(bodyElement, 'padding', '0'); setElementStyle(bodyElement, 'margin', '0'); - // TODO(yjbanov): fix this when we support KVM I/O. Currently we scroll + // TODO(yjbanov): fix this when KVM I/O support is added. Currently scroll // using drag, and text selection interferes. setElementStyle(bodyElement, 'user-select', 'none'); setElementStyle(bodyElement, '-webkit-user-select', 'none'); @@ -211,7 +211,7 @@ class FlutterViewEmbedder { setElementStyle(bodyElement, '-moz-user-select', 'none'); // This is required to prevent the browser from doing any native touch - // handling. If we don't do this, the browser doesn't report 'pointermove' + // handling. If this is not done, the browser doesn't report 'pointermove' // events properly. setElementStyle(bodyElement, 'touch-action', 'none'); @@ -227,7 +227,7 @@ class FlutterViewEmbedder { for (final html.Element viewportMeta in html.document.head!.querySelectorAll('meta[name="viewport"]')) { if (assertionsEnabled) { - // Filter out the meta tag that we ourselves placed on the page. This is + // Filter out the meta tag that the engine placed on the page. This is // to avoid UI flicker during hot restart. Hot restart will clean up the // old meta tag synchronously with the first post-restart frame. if (!viewportMeta.hasAttribute('flt-viewport')) { @@ -265,7 +265,8 @@ class FlutterViewEmbedder { ..bottom = '0' ..left = '0'; - // This must be appended to the body, so we can create a host node properly. + // This must be appended to the body, so the engine can create a host node + // properly. bodyElement.append(glassPaneElement); // Create a [HostNode] under the glass pane element, and attach everything @@ -293,8 +294,8 @@ class FlutterViewEmbedder { _accessibilityPlaceholder, _sceneHostElement!, - // The semantic host goes last because hit-test order-wise we want it to - // be first. If semantics goes under the scene host, platform views will + // The semantic host goes last because hit-test order-wise it must be + // first. If semantics goes under the scene host, platform views will // obscure semantic elements. // // You may be wondering: wouldn't semantics obscure platform views and @@ -327,10 +328,11 @@ class FlutterViewEmbedder { // // VisualViewport API is not enabled in Firefox as well. On the other hand // Firefox returns correct values for innerHeight, innerWidth. - // Firefox also triggers html.window.onResize therefore we don't need this - // timer to be set up for Firefox. + // Firefox also triggers html.window.onResize therefore this timer does + // not need to be set up for Firefox. final int initialInnerWidth = html.window.innerWidth!; - // Counts how many times we checked screen size. We check up to 5 times. + // Counts how many times screen size was checked. It is checked up to 5 + // times. int checkCount = 0; Timer.periodic(const Duration(milliseconds: 100), (Timer t) { checkCount += 1; @@ -367,7 +369,7 @@ class FlutterViewEmbedder { } /// The framework specifies semantics in physical pixels, but CSS uses - /// logical pixels. To compensate, we inject an inverse scale at the root + /// logical pixels. To compensate, an inverse scale is injected at the root /// level. void updateSemanticsScreenProperties() { _semanticsHostElement!.style.transform = diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index 925e93025a1a7..0cecee819b768 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -130,8 +130,8 @@ class PersistedPicture extends PersistedLeafSurface { // users. UI semantics are exported as a separate DOM tree rendered parallel // to pictures. // - // Why do we not hide layer and scene elements from ARIA? Because those - // elements may contain platform views, and we do want platform views to be + // Why are layer and scene elements not hidden from ARIA? Because those + // elements may contain platform views, and platform views must be // accessible. element.setAttribute('aria-hidden', 'true'); diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index c71a655e0fac8..a7895ce23f2c8 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -278,11 +278,11 @@ class SemanticsObject { // The root node has some properties that other nodes do not. if (id == 0 && !configuration.debugShowSemanticsNodes) { - // Make all semantics transparent. We use `filter` instead of `opacity` + // Make all semantics transparent. Use `filter` instead of `opacity` // attribute because `filter` is stronger. `opacity` does not apply to // some elements, particularly on iOS, such as the slider thumb and track. // - // We use transparency instead of "visibility:hidden" or "display:none" + // Use transparency instead of "visibility:hidden" or "display:none" // so that a screen reader does not ignore these elements. element.style.filter = 'opacity(0%)'; @@ -292,7 +292,7 @@ class SemanticsObject { } // Make semantic elements visible for debugging by outlining them using a - // green border. We do not use `border` attribute because it affects layout + // green border. Do not use `border` attribute because it affects layout // (`outline` does not). if (configuration.debugShowSemanticsNodes) { element.style.outline = '1px solid green'; @@ -902,13 +902,13 @@ class SemanticsObject { _childrenInHitTestOrder!.isEmpty) { if (_currentChildrenInRenderOrder == null || _currentChildrenInRenderOrder!.isEmpty) { - // We must not have created a container element when child list is empty. + // A container element must not have been created when child list is empty. assert(_childContainerElement == null); _currentChildrenInRenderOrder = null; return; } - // We must have created a container element when child list is not empty. + // A container element must have been created when child list is not empty. assert(_childContainerElement != null); // Remove all children from this semantics object. @@ -922,7 +922,7 @@ class SemanticsObject { return; } - // At this point we're guaranteed to have at least one child. + // At this point it is guaranteed to have at least one child. final Int32List childrenInTraversalOrder = _childrenInTraversalOrder!; final Int32List childrenInHitTestOrder = _childrenInHitTestOrder!; final int childCount = childrenInHitTestOrder.length; @@ -930,7 +930,7 @@ class SemanticsObject { assert(childrenInTraversalOrder.length == childrenInHitTestOrder.length); - // We always render in traversal order, because the accessibility traversal + // Always render in traversal order, because the accessibility traversal // is determined by the DOM order of elements. final List childrenInRenderOrder = []; for (int i = 0; i < childCount; i++) { @@ -966,7 +966,7 @@ class SemanticsObject { return; } - // At this point we're guaranteed to have had a non-empty previous child list. + // At this point it is guaranteed to have had a non-empty previous child list. final List previousChildrenInRenderOrder = _currentChildrenInRenderOrder!; final int previousCount = previousChildrenInRenderOrder.length; @@ -1059,10 +1059,10 @@ class SemanticsObject { /// /// If [condition] is false, removes the HTML "role" attribute from [element] /// if the current role is set to [ariaRoleName]. Otherwise, leaves the value - /// unchanged. This is done so we gracefully handle multiple competing roles. + /// unchanged. This is done to gracefully handle multiple competing roles. /// For example, if the role changes from "button" to "img" and tappable role /// manager attempts to clean up after the image role manager applied the new - /// role, we do not want it to erase the new role. + /// role, semantics avoids erasing the new role. void setAriaRole(String ariaRoleName, bool condition) { if (condition) { element.setAttribute('role', ariaRoleName); @@ -1093,8 +1093,8 @@ class SemanticsObject { final 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. + // Text fields manage their own focus/tap interactions. Tappable role + // manager is not needed. It only confuses AT. !isTextField; _updateRole(Role.tappable, shouldUseTappableRole); @@ -1121,8 +1121,8 @@ class SemanticsObject { manager.dispose(); _roleManagers.remove(role); } - // Nothing to do in the "else case" as it means that we want to disable a - // role that we don't currently have in the first place. + // Nothing to do in the "else case". There's no existing role manager to + // disable. } /// Whether the object represents an UI element with "increase" or "decrease" @@ -1257,15 +1257,15 @@ class SemanticsObject { /// Controls how pointer events and browser-detected gestures are treated by /// the Web Engine. enum AccessibilityMode { - /// We are not told whether the assistive technology is enabled or not. + /// Flutter is not told whether the assistive technology is enabled or not. /// /// This is the default mode. /// - /// In this mode we use a gesture recognition system that deduplicates + /// In this mode a gesture recognition system is used that deduplicates /// gestures detected by Flutter with gestures detected by the browser. unknown, - /// We are told whether the assistive technology is enabled. + /// Flutter is told whether the assistive technology is enabled. known, } @@ -1430,7 +1430,7 @@ class EngineSemanticsOwner { _semanticsEnabled = value; if (!_semanticsEnabled) { - // We do not process browser events at all when semantics is explicitly + // Do not process browser events at all when semantics is explicitly // disabled. All gestures are handled by the framework-level gesture // recognizers from pointer events. if (_gestureMode != GestureMode.pointerEvents) { @@ -1488,8 +1488,7 @@ class EngineSemanticsOwner { return _gestureModeClock; } - /// Disables browser gestures temporarily because we have detected pointer - /// events. + /// Disables browser gestures temporarily because pointer events were detected. /// /// This is used to deduplicate gestures detected by Flutter and gestures /// detected by the browser. Flutter-detected gestures have higher precedence. @@ -1509,29 +1508,29 @@ class EngineSemanticsOwner { /// The browser sends us both raw pointer events and gestures from /// [SemanticsObject.element]s. There could be three possibilities: /// - /// 1. Assistive technology is enabled and we know that it is. - /// 2. Assistive technology is disabled and we know that it isn't. - /// 3. We do not know whether an assistive technology is enabled. + /// 1. Assistive technology is enabled and Flutter knows that it is. + /// 2. Assistive technology is disabled and Flutter knows that it isn't. + /// 3. Flutter does not know whether an assistive technology is enabled. /// /// If [autoEnableOnTap] was called, this will automatically enable semantics /// if the user requests it. /// - /// In the first case we can ignore raw pointer events and only interpret + /// In the first case ignore raw pointer events and only interpret /// high-level gestures, e.g. "click". /// - /// In the second case we can ignore high-level gestures and interpret the raw + /// In the second case ignore high-level gestures and interpret the raw /// pointer events directly. /// - /// Finally, in a mode when we do not know if an assistive technology is - /// enabled or not we do a best-effort estimate which to respond to, raw - /// pointer or high-level gestures. We avoid doing both because that will + /// Finally, in a mode when Flutter does not know if an assistive technology + /// is enabled or not do a best-effort estimate which to respond to, raw + /// pointer or high-level gestures. Avoid doing both because that will /// result in double-firing of event listeners, such as `onTap` on a button. - /// An approach we use is to measure the distance between the last pointer + /// The approach is to measure the distance between the last pointer /// event and a gesture event. If a gesture is receive "soon" after the last /// received pointer event (determined by a heuristic), it is debounced as it /// is likely that the gesture detected from the pointer even will do the - /// right thing. However, if we receive a standalone gesture we will map it - /// onto a [ui.SemanticsAction] to be processed by the framework. + /// right thing. However, if a standalone gesture is received, map it onto a + /// [ui.SemanticsAction] to be processed by the framework. bool receiveGlobalEvent(html.Event event) { // For pointer event reference see: // @@ -1598,7 +1597,7 @@ class EngineSemanticsOwner { /// [semanticsEnabled] is `false`. /// /// If [mode] is [AccessibilityMode.unknown] the gesture is accepted if it is - /// not accompanied by pointer events. In the presence of pointer events we + /// not accompanied by pointer events. In the presence of pointer events, /// delegate to Flutter's gesture detection system to produce gestures. bool shouldAcceptBrowserGesture(String eventType) { if (_mode == AccessibilityMode.known) { @@ -1658,7 +1657,7 @@ class EngineSemanticsOwner { } // Second, fix the tree structure. This is moved out into its own loop, - // because we must make sure each object's own information is up-to-date. + // because each object's own information must be updated first. for (final SemanticsNodeUpdate nodeUpdate in update._nodeUpdates!) { final SemanticsObject object = _semanticsTree[nodeUpdate.id]!; object.updateChildren(); @@ -1682,7 +1681,7 @@ class EngineSemanticsOwner { // Dirty fields should be cleared after the tree has been finalized. assert(object._dirtyFields == 0); - // Make sure we create a child container only when there are children. + // Make sure a child container is created only when there are children. assert(object._childContainerElement == null || object.hasChildren); // Ensure child ID list is consistent with the parent-child @@ -1754,8 +1753,7 @@ List longestIncreasingSubsequence(List list) { mins[expansionIndex] = i; } if (expansionIndex > longest) { - // If we found a subsequence longer than any we've - // found yet, update `longest` + // Record the longest subsequence found so far. longest = expansionIndex; } } From 20527731c40f736a457d34cc8dc2ca5d7ef105b0 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Fri, 15 Apr 2022 15:46:26 -0700 Subject: [PATCH 3/5] remove unused intersectionIndicesNew --- lib/web_ui/lib/src/engine/semantics/semantics.dart | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index a7895ce23f2c8..1715692b77953 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -972,10 +972,6 @@ class SemanticsObject { // Both non-empty case. - // Indices into the new child list pointing at children that also exist in - // the old child list. - final List intersectionIndicesNew = []; - // Indices into the old child list pointing at children that also exist in // the new child list. final List intersectionIndicesOld = []; @@ -989,7 +985,6 @@ class SemanticsObject { while (newIndex < minLength && previousChildrenInRenderOrder[newIndex] == childrenInRenderOrder[newIndex]) { - intersectionIndicesNew.add(newIndex); intersectionIndicesOld.add(newIndex); newIndex += 1; } @@ -1007,7 +1002,6 @@ class SemanticsObject { oldIndex += 1) { if (previousChildrenInRenderOrder[oldIndex] == childrenInRenderOrder[newIndex]) { - intersectionIndicesNew.add(newIndex); intersectionIndicesOld.add(oldIndex); break; } From 52076d3cb75818e181860c60b937b0d23554ab49 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Mon, 18 Apr 2022 11:01:41 -0700 Subject: [PATCH 4/5] add hit test test; fix platform view offset and scene host init --- .../src/engine/canvaskit/embedded_views.dart | 9 +- .../lib/src/engine/canvaskit/surface.dart | 12 ++ lib/web_ui/lib/src/engine/embedder.dart | 27 ++- lib/web_ui/lib/src/engine/initialization.dart | 6 - .../lib/src/engine/platform_dispatcher.dart | 2 +- .../lib/src/engine/semantics/semantics.dart | 18 +- .../test/canvaskit/embedded_views_test.dart | 28 +++ lib/web_ui/test/canvaskit/semantics_test.dart | 25 +++ .../test/engine/semantics/semantics_test.dart | 188 +++++++++++++++++- 9 files changed, 294 insertions(+), 21 deletions(-) create mode 100644 lib/web_ui/test/canvaskit/semantics_test.dart diff --git a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart index acbd5e8560333..aa307d4db8fb4 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/embedded_views.dart @@ -242,7 +242,7 @@ class HtmlViewEmbedder { } // Apply mutators to the slot - _applyMutators(params.mutators, slot, viewId); + _applyMutators(params, slot, viewId); } int _countClips(MutatorsStack mutators) { @@ -309,9 +309,12 @@ class HtmlViewEmbedder { } void _applyMutators( - MutatorsStack mutators, html.Element embeddedView, int viewId) { + EmbeddedViewParams params, html.Element embeddedView, int viewId) { + final MutatorsStack mutators = params.mutators; html.Element head = embeddedView; - Matrix4 headTransform = Matrix4.identity(); + Matrix4 headTransform = params.offset == ui.Offset.zero + ? Matrix4.identity() + : Matrix4.translationValues(params.offset.dx, params.offset.dy, 0); double embeddedOpacity = 1.0; _resetAnchor(head); _cleanUpClipDefs(viewId); diff --git a/lib/web_ui/lib/src/engine/canvaskit/surface.dart b/lib/web_ui/lib/src/engine/canvaskit/surface.dart index 0bfc5df7c10cc..17360017ee296 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/surface.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/surface.dart @@ -282,6 +282,18 @@ class Surface { height: _pixelHeight, ); this.htmlCanvas = htmlCanvas; + + // The DOM elements used to render pictures are used purely to put pixels on + // the screen. They have no semantic information. If an assistive technology + // attempts to scan picture content it will look like garbage and confuse + // users. UI semantics are exported as a separate DOM tree rendered parallel + // to pictures. + // + // Why are layer and scene elements not hidden from ARIA? Because those + // elements may contain platform views, and platform views must be + // accessible. + htmlCanvas.setAttribute('aria-hidden', 'true'); + htmlCanvas.style.position = 'absolute'; _updateLogicalHtmlCanvasSize(); diff --git a/lib/web_ui/lib/src/engine/embedder.dart b/lib/web_ui/lib/src/engine/embedder.dart index c64976a3eb405..5b228e6fca419 100644 --- a/lib/web_ui/lib/src/engine/embedder.dart +++ b/lib/web_ui/lib/src/engine/embedder.dart @@ -37,11 +37,15 @@ import 'window.dart'; /// - [semanticsHostElement], hosts the ARIA-annotated semantics tree. class FlutterViewEmbedder { FlutterViewEmbedder() { - reset(); assert(() { _setupHotRestart(); return true; }()); + reset(); + assert(() { + _registerHotRestartCleanUp(); + return true; + }()); } // The tag name for the root view of the flutter app (glass-pane) @@ -104,6 +108,10 @@ class FlutterViewEmbedder { static const String _staleHotRestartStore = '__flutter_state'; List? _staleHotRestartState; + /// Creates a container for DOM elements that need to be cleaned up between + /// hot restarts. + /// + /// If a contains already exists, reuses the existing one. void _setupHotRestart() { // This persists across hot restarts to clear stale DOM. _staleHotRestartState = getJsProperty?>(html.window, _staleHotRestartStore); @@ -112,7 +120,12 @@ class FlutterViewEmbedder { setJsProperty( html.window, _staleHotRestartStore, _staleHotRestartState); } + } + /// Registers DOM elements that need to be cleaned up before hot restarting. + /// + /// [_setupHotRestart] must have been called prior to calling this method. + void _registerHotRestartCleanUp() { registerHotRestartListener(() { _resizeSubscription?.cancel(); _localeSubscription?.cancel(); @@ -137,7 +150,7 @@ class FlutterViewEmbedder { /// already in the right place, skip DOM mutation. This is both faster and /// more correct, because moving DOM nodes loses internal state, such as /// text selection. - void renderScene(html.Element? sceneElement) { + void addSceneToSceneHost(html.Element? sceneElement) { if (sceneElement != _sceneElement) { _sceneElement?.remove(); _sceneElement = sceneElement; @@ -278,6 +291,14 @@ class FlutterViewEmbedder { _sceneHostElement = html.document.createElement('flt-scene-host') ..style.pointerEvents = 'none'; + /// CanvasKit uses a static scene element that never gets replaced, so it's + /// added eagerly during initialization here and never touched, unless the + /// system is reset due to hot restart or in a test. + if (useCanvasKit) { + skiaSceneHost = html.Element.tag('flt-scene'); + addSceneToSceneHost(skiaSceneHost); + } + final html.Element semanticsHostElement = html.document.createElement('flt-semantics-host'); semanticsHostElement.style @@ -308,7 +329,7 @@ class FlutterViewEmbedder { ]); // When debugging semantics, make the scene semi-transparent so that the - // semantics tree is visible. + // semantics tree is more prominent. if (configuration.debugShowSemanticsNodes) { _sceneHostElement!.style.opacity = '0.3'; } diff --git a/lib/web_ui/lib/src/engine/initialization.dart b/lib/web_ui/lib/src/engine/initialization.dart index 829ada7b47695..873dbc64cf37e 100644 --- a/lib/web_ui/lib/src/engine/initialization.dart +++ b/lib/web_ui/lib/src/engine/initialization.dart @@ -256,12 +256,6 @@ Future initializeEngineUi() async { Keyboard.initialize(onMacOs: operatingSystem == OperatingSystem.macOs); MouseCursor.initialize(); ensureFlutterViewEmbedderInitialized(); - - if (useCanvasKit) { - /// Add a Skia scene host. - skiaSceneHost = html.Element.tag('flt-scene'); - flutterViewEmbedder.renderScene(skiaSceneHost); - } _initializationState = DebugEngineInitializationState.initialized; } diff --git a/lib/web_ui/lib/src/engine/platform_dispatcher.dart b/lib/web_ui/lib/src/engine/platform_dispatcher.dart index 5ad7a4dd23a8f..76a6da3443877 100644 --- a/lib/web_ui/lib/src/engine/platform_dispatcher.dart +++ b/lib/web_ui/lib/src/engine/platform_dispatcher.dart @@ -607,7 +607,7 @@ class EnginePlatformDispatcher extends ui.PlatformDispatcher { rasterizer!.draw(layerScene.layerTree); } else { final SurfaceScene surfaceScene = scene as SurfaceScene; - flutterViewEmbedder.renderScene(surfaceScene.webOnlyRootElement); + flutterViewEmbedder.addSceneToSceneHost(surfaceScene.webOnlyRootElement); } frameTimingsOnRasterFinish(); } diff --git a/lib/web_ui/lib/src/engine/semantics/semantics.dart b/lib/web_ui/lib/src/engine/semantics/semantics.dart index 1715692b77953..01feefbe2307b 100644 --- a/lib/web_ui/lib/src/engine/semantics/semantics.dart +++ b/lib/web_ui/lib/src/engine/semantics/semantics.dart @@ -972,6 +972,20 @@ class SemanticsObject { // Both non-empty case. + // Problem: child nodes have been added, removed, and/or reordered. On the + // web, many assistive technologies cannot track DOM elements + // moving around, losing focus. The best approach is to try to keep + // child elements as stable as possible. + // Solution: find all common elements in both lists and record their indices + // in the old list (in the `intersectionIndicesOld` variable). The + // longest increases subsequence provides the longest chain of + // semantics nodes that didn't move relative to each other. Those + // nodes (represented by the `stationaryIds` variable) are kept + // stationary, while all others are moved/inserted/deleted around + // them. This gives the maximum node stability, and covers most + // use-cases, including scrolling in any direction, insertions, + // deletions, drag'n'drop, etc. + // Indices into the old child list pointing at children that also exist in // the new child list. final List intersectionIndicesOld = []; @@ -997,9 +1011,7 @@ class SemanticsObject { // If child lists are not identical, continue computing the intersection // between the two lists. while (newIndex < childCount) { - for (int oldIndex = 0; - oldIndex < previousCount; - oldIndex += 1) { + for (int oldIndex = 0; oldIndex < previousCount; oldIndex += 1) { if (previousChildrenInRenderOrder[oldIndex] == childrenInRenderOrder[newIndex]) { intersectionIndicesOld.add(oldIndex); diff --git a/lib/web_ui/test/canvaskit/embedded_views_test.dart b/lib/web_ui/test/canvaskit/embedded_views_test.dart index c619f5e5461fe..b6ada66460656 100644 --- a/lib/web_ui/test/canvaskit/embedded_views_test.dart +++ b/lib/web_ui/test/canvaskit/embedded_views_test.dart @@ -130,6 +130,34 @@ void testMain() { ); }); + test('correctly offsets platform views', () async { + ui.platformViewRegistry.registerViewFactory( + 'test-platform-view', + (int viewId) => html.DivElement()..id = 'view-0', + ); + await createPlatformView(0, 'test-platform-view'); + + final EnginePlatformDispatcher dispatcher = + ui.window.platformDispatcher as EnginePlatformDispatcher; + final LayerSceneBuilder sb = LayerSceneBuilder(); + sb.addPlatformView(0, offset: const ui.Offset(3, 4), width: 5, height: 6); + dispatcher.rasterizer!.draw(sb.build().layerTree); + + final html.Element slotHost = + flutterViewEmbedder.sceneElement!.querySelector('flt-platform-view-slot')!; + final html.CssStyleDeclaration style = slotHost.style; + + expect(style.transform, 'matrix(1, 0, 0, 1, 3, 4)'); + expect(style.width, '5px'); + expect(style.height, '6px'); + + final html.Rectangle slotRect = slotHost.getBoundingClientRect(); + expect(slotRect.left, 3); + expect(slotRect.top, 4); + expect(slotRect.right, 8); + expect(slotRect.bottom, 10); + }); + // Returns the list of CSS transforms applied to the ancestor chain of // elements starting from `viewHost`, up until and excluding . List getTransformChain(html.Element viewHost) { diff --git a/lib/web_ui/test/canvaskit/semantics_test.dart b/lib/web_ui/test/canvaskit/semantics_test.dart new file mode 100644 index 0000000000000..432bb53a105d7 --- /dev/null +++ b/lib/web_ui/test/canvaskit/semantics_test.dart @@ -0,0 +1,25 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +@TestOn('chrome || safari || firefox') + +import 'dart:async'; + +import 'package:test/bootstrap/browser.dart'; +import 'package:test/test.dart'; + +import '../engine/semantics/semantics_test.dart'; +import 'common.dart'; + +void main() { + internalBootstrapBrowserTest(() => testMain); +} + +// Run the same semantics tests in CanvasKit mode because as of today we do not +// yet share platform view logic with the HTML renderer, which affects +// semantics. +Future testMain() async { + setUpCanvasKitTest(); + runSemanticsTests(); +} diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index 5c2f254d1b505..0483a4ade3ffc 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -12,10 +12,7 @@ import 'package:quiver/testing/async.dart'; import 'package:test/bootstrap/browser.dart'; import 'package:test/test.dart'; -import 'package:ui/src/engine.dart' show flutterViewEmbedder; -import 'package:ui/src/engine/browser_detection.dart'; -import 'package:ui/src/engine/semantics.dart'; -import 'package:ui/src/engine/vector_math.dart'; +import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' as ui; import 'semantics_tester.dart'; @@ -25,12 +22,17 @@ DateTime _testTime = DateTime(2018, 12, 17); EngineSemanticsOwner semantics() => EngineSemanticsOwner.instance; void main() { - internalBootstrapBrowserTest(() => testMain); + internalBootstrapBrowserTest(() { + return testMain; + }); } Future testMain() async { await ui.webOnlyInitializePlatform(); + runSemanticsTests(); +} +void runSemanticsTests() { setUp(() { EngineSemanticsOwner.debugResetSemantics(); }); @@ -1711,6 +1713,152 @@ void _testPlatformView() { semantics().semanticsEnabled = false; }); + + // This test simulates the scenario of three child semantic nodes contained by + // a common parent. The first and the last nodes are plain leaf nodes. The + // middle node is a platform view node. Nodes overlap. The test hit tests + // various points and verifies that the correct DOM element receives the + // event. The test does this using `documentOrShadow.elementFromPoint`, which, + // if browsers are to be trusted, should do the same thing as if a pointer + // event landed at the given location. + // + // 0px ------------- + // | | + // | | <- plain semantic node + // | 1 | + // 15px | ------------- + // | | | + // 25px --| | + // | 2 | <- platform view + // | | + // 35px | ------------- + // | | | + // 45px --| | + // | 3 | <- plain semantic node + // | | + // | | + // 60px ------------- + test('is reachable via a hit test', () async { + semantics() + ..debugOverrideTimestampFunction(() => _testTime) + ..semanticsEnabled = true; + + ui.platformViewRegistry.registerViewFactory( + 'test-platform-view', + (int viewId) => html.DivElement() + ..id = 'view-0' + ..style.width = '100%' + ..style.height = '100%', + ); + await createPlatformView(0, 'test-platform-view'); + + final ui.SceneBuilder sceneBuilder = ui.SceneBuilder(); + sceneBuilder.addPlatformView( + 0, + offset: const ui.Offset(0, 15), + width: 20, + height: 30, + ); + ui.window.render(sceneBuilder.build()); + + final ui.SemanticsUpdateBuilder builder = ui.SemanticsUpdateBuilder(); + updateNode( + builder, + rect: const ui.Rect.fromLTRB(0, 0, 20, 60), + childrenInTraversalOrder: Int32List.fromList([1, 2, 3]), + childrenInHitTestOrder: Int32List.fromList([1, 2, 3]), + ); + updateNode( + builder, + id: 1, + rect: const ui.Rect.fromLTRB(0, 0, 20, 25), + ); + updateNode( + builder, + id: 2, + // This has to match the values passed to `addPlatformView` above. + rect: const ui.Rect.fromLTRB(0, 15, 20, 45), + platformViewId: 0, + ); + updateNode( + builder, + id: 3, + rect: const ui.Rect.fromLTRB(0, 35, 20, 60), + ); + + semantics().updateSemantics(builder.build()); + expectSemanticsTree(''' + + + + + + +'''); + + final html.Element root = appHostNode.querySelector('#flt-semantic-node-0')!; + expect(root.style.pointerEvents, 'none'); + + final html.Element child1 = appHostNode.querySelector('#flt-semantic-node-1')!; + expect(child1.style.pointerEvents, 'all'); + final html.Rectangle child1Rect = child1.getBoundingClientRect(); + expect(child1Rect.left, 0); + expect(child1Rect.top, 0); + expect(child1Rect.right, 20); + expect(child1Rect.bottom, 25); + + final html.Element child2 = appHostNode.querySelector('#flt-semantic-node-2')!; + expect(child2.style.pointerEvents, 'none'); + final html.Rectangle child2Rect = child2.getBoundingClientRect(); + expect(child2Rect.left, 0); + expect(child2Rect.top, 15); + expect(child2Rect.right, 20); + expect(child2Rect.bottom, 45); + + final html.Element child3 = appHostNode.querySelector('#flt-semantic-node-3')!; + expect(child3.style.pointerEvents, 'all'); + final html.Rectangle child3Rect = child3.getBoundingClientRect(); + expect(child3Rect.left, 0); + expect(child3Rect.top, 35); + expect(child3Rect.right, 20); + expect(child3Rect.bottom, 60); + + final html.Element platformViewElement = flutterViewEmbedder.glassPaneElement!.querySelector('#view-0')!; + final html.Rectangle platformViewRect = platformViewElement.getBoundingClientRect(); + expect(platformViewRect.left, 0); + expect(platformViewRect.top, 15); + expect(platformViewRect.right, 20); + expect(platformViewRect.bottom, 45); + + // This test is only relevant for shadow DOM because we only really support + // proper platform view embedding in browsers that support shadow DOM. + final html.ShadowRoot shadowRoot = appHostNode.node as html.ShadowRoot; + + // Hit test child 1 + expect(shadowRoot.elementFromPoint(10, 10)!, child1); + + // Hit test overlap between child 1 and 2 + // TODO(yjbanov): this is a known limitation, see https://github.com/flutter/flutter/issues/101439 + expect(shadowRoot.elementFromPoint(10, 20)!, child1); + + // Hit test child 2 + // Clicking at the location of the middle semantics node should allow the + // event to go through the semantic tree and hit the platform view. Since + // platform views are projected into the shadow DOM from outside the shadow + // root, it would be reachable both from the shadow root (by hitting the + // corresponding tag) and from the document (by hitting the platform + // view element itself). + expect(shadowRoot.elementFromPoint(10, 30)!, platformViewElement); + expect(html.document.elementFromPoint(10, 30)!, platformViewElement); + + // Hit test overlap between child 2 and 3 + expect(shadowRoot.elementFromPoint(10, 40)!, child3); + + // Hit test child 3 + expect(shadowRoot.elementFromPoint(10, 50)!, child3); + + semantics().semanticsEnabled = false; + }); } /// A facade in front of [ui.SemanticsUpdateBuilder.updateNode] that @@ -1790,3 +1938,33 @@ void updateNode( additionalActions: additionalActions, ); } + +const MethodCodec codec = StandardMethodCodec(); + +/// Sends a platform message to create a Platform View with the given id and viewType. +Future createPlatformView(int id, String viewType) { + final Completer completer = Completer(); + ui.window.sendPlatformMessage( + 'flutter/platform_views', + codec.encodeMethodCall(MethodCall( + 'create', + { + 'id': id, + 'viewType': viewType, + }, + )), + (dynamic _) => completer.complete(), + ); + return completer.future; +} + +/// Disposes of the platform view with the given [id]. +Future disposePlatformView(int id) { + final Completer completer = Completer(); + window.sendPlatformMessage( + 'flutter/platform_views', + codec.encodeMethodCall(MethodCall('dispose', id)), + (dynamic _) => completer.complete(), + ); + return completer.future; +} From 0ff84fcae71ce8826b4637cea3dd474c848eb8d5 Mon Sep 17 00:00:00 2001 From: Yegor Jbanov Date: Mon, 18 Apr 2022 13:53:47 -0700 Subject: [PATCH 5/5] browser-specific fixes --- lib/web_ui/test/canvaskit/semantics_test.dart | 8 +- .../test/engine/semantics/semantics_test.dart | 102 ++++++------------ 2 files changed, 39 insertions(+), 71 deletions(-) diff --git a/lib/web_ui/test/canvaskit/semantics_test.dart b/lib/web_ui/test/canvaskit/semantics_test.dart index 432bb53a105d7..36969fb886f5c 100644 --- a/lib/web_ui/test/canvaskit/semantics_test.dart +++ b/lib/web_ui/test/canvaskit/semantics_test.dart @@ -20,6 +20,10 @@ void main() { // yet share platform view logic with the HTML renderer, which affects // semantics. Future testMain() async { - setUpCanvasKitTest(); - runSemanticsTests(); + group('CanvasKit semantics', () { + setUpCanvasKitTest(); + + runSemanticsTests(); + // TODO(hterkelsen): https://github.com/flutter/flutter/issues/60040 + }, skip: isIosSafari); } diff --git a/lib/web_ui/test/engine/semantics/semantics_test.dart b/lib/web_ui/test/engine/semantics/semantics_test.dart index 0483a4ade3ffc..d7ec3fa45ae90 100644 --- a/lib/web_ui/test/engine/semantics/semantics_test.dart +++ b/lib/web_ui/test/engine/semantics/semantics_test.dart @@ -15,6 +15,7 @@ import 'package:test/test.dart'; import 'package:ui/src/engine.dart'; import 'package:ui/ui.dart' as ui; +import '../../common.dart'; import 'semantics_tester.dart'; DateTime _testTime = DateTime(2018, 12, 17); @@ -537,21 +538,12 @@ void _testContainer() { ); semantics().updateSemantics(builder.build()); - if (browserEngine == BrowserEngine.edge) { - expectSemanticsTree(''' - - - - -'''); - } else { - expectSemanticsTree(''' + expectSemanticsTree(''' '''); - } final html.Element parentElement = appHostNode.querySelector('flt-semantics')!; @@ -1094,9 +1086,7 @@ void _testIncrementables() { expect(await logger.actionLog.first, ui.SemanticsAction.decrease); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders a node that can both increment and decrement', () async { semantics() @@ -1125,9 +1115,7 @@ void _testIncrementables() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); } void _testTextField() { @@ -1154,9 +1142,7 @@ void _testTextField() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); // TODO(yjbanov): this test will need to be adjusted for Safari when we add // Safari testing. @@ -1223,9 +1209,7 @@ void _testCheckables() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders a switched on disabled switch element', () async { semantics() @@ -1251,9 +1235,7 @@ void _testCheckables() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders a switched off switch element', () async { semantics() @@ -1279,9 +1261,7 @@ void _testCheckables() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders a checked checkbox', () async { semantics() @@ -1308,9 +1288,7 @@ void _testCheckables() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders a checked disabled checkbox', () async { semantics() @@ -1336,9 +1314,7 @@ void _testCheckables() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders an unchecked checkbox', () async { semantics() @@ -1364,9 +1340,7 @@ void _testCheckables() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders a checked radio button', () async { semantics() @@ -1394,9 +1368,7 @@ void _testCheckables() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders a checked disabled radio button', () async { semantics() @@ -1423,9 +1395,7 @@ void _testCheckables() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders an unchecked checkbox', () async { semantics() @@ -1452,9 +1422,7 @@ void _testCheckables() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); } void _testTappable() { @@ -1481,9 +1449,7 @@ void _testTappable() { expect(tester.getSemanticsObject(0).element.tabIndex, 0); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders a disabled tappable widget', () async { semantics() @@ -1508,9 +1474,7 @@ void _testTappable() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); } void _testImage() { @@ -1536,9 +1500,7 @@ void _testImage() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders an image with a child node and with a label', () async { semantics() @@ -1575,9 +1537,7 @@ void _testImage() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders an image with no child nodes without a label', () async { semantics() @@ -1599,9 +1559,7 @@ void _testImage() { ''''''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('renders an image with a child node and without a label', () async { semantics() @@ -1637,9 +1595,7 @@ void _testImage() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); } void _testLiveRegion() { @@ -1665,9 +1621,7 @@ void _testLiveRegion() { '''); semantics().semanticsEnabled = false; - }, - // TODO(yjbanov): https://github.com/flutter/flutter/issues/50754 - skip: browserEngine == BrowserEngine.edge); + }); test('does not render a live region if there is no label', () async { semantics() @@ -1848,7 +1802,13 @@ void _testPlatformView() { // root, it would be reachable both from the shadow root (by hitting the // corresponding tag) and from the document (by hitting the platform // view element itself). - expect(shadowRoot.elementFromPoint(10, 30)!, platformViewElement); + + // Browsers disagree about which element should be returned when hit testing + // a shadow root. However, they do agree when hit testing `document`. + // + // See: + // * https://github.com/w3c/csswg-drafts/issues/556 + // * https://bugzilla.mozilla.org/show_bug.cgi?id=1502369 expect(html.document.elementFromPoint(10, 30)!, platformViewElement); // Hit test overlap between child 2 and 3 @@ -1858,7 +1818,11 @@ void _testPlatformView() { expect(shadowRoot.elementFromPoint(10, 50)!, child3); semantics().semanticsEnabled = false; - }); + // TODO(yjbanov): unable to debug this test on iOS Safari as hacking on a + // Linux machine. iOS Safari returns getBoundingClientRect + // values that are half of desktop browsers, possibly due to + // devicePixelRatio but need to confirm. + }, skip: isIosSafari); } /// A facade in front of [ui.SemanticsUpdateBuilder.updateNode] that