Skip to content

Commit 4e5a2db

Browse files
auto-submit[bot]auto-submit[bot]
andauthored
Reverts "[web][a11y]Delete _childContainerElement (#163662)" (#165416)
<!-- start_original_pr_link --> Reverts: flutter/flutter#163662 <!-- end_original_pr_link --> <!-- start_initiating_author --> Initiated by: hannah-hyj <!-- end_initiating_author --> <!-- start_revert_reason --> Reason for reverting: google 3 failure <!-- end_revert_reason --> <!-- start_original_pr_author --> Original PR Author: hannah-hyj <!-- end_original_pr_author --> <!-- start_reviewers --> Reviewed By: {yjbanov} <!-- end_reviewers --> <!-- start_revert_body --> This change reverts the following previous change: delete _childContainerElement , add the rect compensate and scrolling adjustment to the children ## Pre-launch Checklist - [ ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [ ] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [ ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [ ] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md <!-- end_revert_body --> Co-authored-by: auto-submit[bot] <flutter-engprod-team@google.com>
1 parent 155f6dc commit 4e5a2db

File tree

6 files changed

+261
-157
lines changed

6 files changed

+261
-157
lines changed

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/scrollable.dart

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ class SemanticScrollable extends SemanticRole {
7070
final bool doScrollForward = _domScrollPosition > _effectiveNeutralScrollPosition;
7171
_neutralizeDomScrollPosition();
7272
semanticsObject.recomputePositionAndSize();
73-
semanticsObject.updateChildrenPositionAndSize();
7473

7574
final int semanticsId = semanticsObject.id;
7675
if (doScrollForward) {
@@ -132,7 +131,6 @@ class SemanticScrollable extends SemanticRole {
132131
semanticsObject.owner.addOneTimePostUpdateCallback(() {
133132
_neutralizeDomScrollPosition();
134133
semanticsObject.recomputePositionAndSize();
135-
semanticsObject.updateChildrenPositionAndSize();
136134
});
137135

138136
if (_scrollListener == null) {
@@ -205,8 +203,8 @@ class SemanticScrollable extends SemanticRole {
205203
// Read back because the effective value depends on the amount of content.
206204
_effectiveNeutralScrollPosition = element.scrollTop.toInt();
207205
semanticsObject
208-
..verticalScrollAdjustment = _effectiveNeutralScrollPosition.toDouble()
209-
..horizontalScrollAdjustment = 0.0;
206+
..verticalContainerAdjustment = _effectiveNeutralScrollPosition.toDouble()
207+
..horizontalContainerAdjustment = 0.0;
210208
} else {
211209
// Place the _scrollOverflowElement at the end of the content and
212210
// make sure that when we neutralize the scrolling position,
@@ -221,8 +219,8 @@ class SemanticScrollable extends SemanticRole {
221219
// Read back because the effective value depends on the amount of content.
222220
_effectiveNeutralScrollPosition = element.scrollLeft.toInt();
223221
semanticsObject
224-
..verticalScrollAdjustment = 0.0
225-
..horizontalScrollAdjustment = _effectiveNeutralScrollPosition.toDouble();
222+
..verticalContainerAdjustment = 0.0
223+
..horizontalContainerAdjustment = _effectiveNeutralScrollPosition.toDouble();
226224
}
227225
}
228226

engine/src/flutter/lib/web_ui/lib/src/engine/semantics/semantics.dart

Lines changed: 76 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1358,6 +1358,33 @@ class SemanticsObject {
13581358
/// The dom element of this semantics object.
13591359
DomElement get element => semanticRole!.element;
13601360

1361+
/// Returns the HTML element that contains the HTML elements of direct
1362+
/// children of this object.
1363+
///
1364+
/// The element is created lazily. When the child list is empty this element
1365+
/// is not created. This is necessary for "aria-label" to function correctly.
1366+
/// The browser will ignore the [label] of HTML element that contain child
1367+
/// elements.
1368+
DomElement? getOrCreateChildContainer() {
1369+
if (_childContainerElement == null) {
1370+
_childContainerElement = createDomElement('flt-semantics-container');
1371+
_childContainerElement!.style
1372+
..position = 'absolute'
1373+
// Ignore pointer events on child container so that platform views
1374+
// behind it can be reached.
1375+
..pointerEvents = 'none';
1376+
element.append(_childContainerElement!);
1377+
}
1378+
return _childContainerElement;
1379+
}
1380+
1381+
/// The element that contains the elements belonging to the child semantics
1382+
/// nodes.
1383+
///
1384+
/// This element is used to correct for [_rect] offsets. It is only non-`null`
1385+
/// when there are non-zero children (i.e. when [hasChildren] is `true`).
1386+
DomElement? _childContainerElement;
1387+
13611388
/// The parent of this semantics object.
13621389
///
13631390
/// This value is not final until the tree is finalized. It is not safe to
@@ -1655,15 +1682,22 @@ class SemanticsObject {
16551682
// Trivial case: remove all children.
16561683
if (_childrenInHitTestOrder == null || _childrenInHitTestOrder!.isEmpty) {
16571684
if (_currentChildrenInRenderOrder == null || _currentChildrenInRenderOrder!.isEmpty) {
1685+
// A container element must not have been created when child list is empty.
1686+
assert(_childContainerElement == null);
16581687
_currentChildrenInRenderOrder = null;
16591688
return;
16601689
}
16611690

1691+
// A container element must have been created when child list is not empty.
1692+
assert(_childContainerElement != null);
1693+
16621694
// Remove all children from this semantics object.
16631695
final int len = _currentChildrenInRenderOrder!.length;
16641696
for (int i = 0; i < len; i++) {
16651697
owner._detachObject(_currentChildrenInRenderOrder![i].id);
16661698
}
1699+
_childContainerElement!.remove();
1700+
_childContainerElement = null;
16671701
_currentChildrenInRenderOrder = null;
16681702
return;
16691703
}
@@ -1672,6 +1706,7 @@ class SemanticsObject {
16721706
final Int32List childrenInTraversalOrder = _childrenInTraversalOrder!;
16731707
final Int32List childrenInHitTestOrder = _childrenInHitTestOrder!;
16741708
final int childCount = childrenInHitTestOrder.length;
1709+
final DomElement? containerElement = getOrCreateChildContainer();
16751710

16761711
assert(childrenInTraversalOrder.length == childrenInHitTestOrder.length);
16771712

@@ -1703,7 +1738,7 @@ class SemanticsObject {
17031738
// Trivial case: previous list was empty => just populate the container.
17041739
if (_currentChildrenInRenderOrder == null || _currentChildrenInRenderOrder!.isEmpty) {
17051740
for (final SemanticsObject child in childrenInRenderOrder) {
1706-
element.append(child.element);
1741+
containerElement!.append(child.element);
17071742
owner._attachObject(parent: this, child: child);
17081743
}
17091744
_currentChildrenInRenderOrder = childrenInRenderOrder;
@@ -1787,9 +1822,9 @@ class SemanticsObject {
17871822
final SemanticsObject child = childrenInRenderOrder[i];
17881823
if (!stationaryIds.contains(child.id)) {
17891824
if (refNode == null) {
1790-
element.append(child.element);
1825+
containerElement!.append(child.element);
17911826
} else {
1792-
element.insertBefore(child.element, refNode);
1827+
containerElement!.insertBefore(child.element, refNode);
17931828
}
17941829
owner._attachObject(parent: this, child: child);
17951830
} else {
@@ -1955,6 +1990,10 @@ class SemanticsObject {
19551990

19561991
// Reparent element.
19571992
if (previousElement != element) {
1993+
final DomElement? container = _childContainerElement;
1994+
if (container != null) {
1995+
element.append(container);
1996+
}
19581997
final DomElement? parent = previousElement?.parent;
19591998
if (parent != null) {
19601999
parent.insertBefore(element, previousElement);
@@ -2040,74 +2079,60 @@ class SemanticsObject {
20402079
/// Indicates whether the node is currently expanded.
20412080
bool get isExpanded => hasFlag(ui.SemanticsFlag.isExpanded);
20422081

2043-
/// Role-specific adjustment of the vertical position of the children.
2082+
/// Role-specific adjustment of the vertical position of the child container.
20442083
///
20452084
/// This is used, for example, by the [SemanticScrollable] to compensate for the
20462085
/// `scrollTop` offset in the DOM.
20472086
///
20482087
/// This field must not be null.
2049-
double verticalScrollAdjustment = 0.0;
2088+
double verticalContainerAdjustment = 0.0;
20502089

2051-
/// Role-specific adjustment of the horizontal position of children.
2090+
/// Role-specific adjustment of the horizontal position of the child
2091+
/// container.
20522092
///
20532093
/// This is used, for example, by the [SemanticScrollable] to compensate for the
20542094
/// `scrollLeft` offset in the DOM.
20552095
///
20562096
/// This field must not be null.
2057-
double horizontalScrollAdjustment = 0.0;
2058-
2059-
double verticalAdjustmentFromParent = 0.0;
2060-
double horizontalAdjustmentFromParent = 0.0;
2097+
double horizontalContainerAdjustment = 0.0;
20612098

20622099
/// Computes the size and position of [element] and, if this element
2063-
/// [hasChildren], computes the parent adjustment for each child.
2100+
/// [hasChildren], of [getOrCreateChildContainer].
20642101
void recomputePositionAndSize() {
20652102
element.style
20662103
..width = '${_rect!.width}px'
20672104
..height = '${_rect!.height}px';
20682105

2106+
final DomElement? containerElement = hasChildren ? getOrCreateChildContainer() : null;
2107+
20692108
final bool hasZeroRectOffset = _rect!.top == 0.0 && _rect!.left == 0.0;
20702109
final Float32List? transform = _transform;
20712110
final bool hasIdentityTransform =
20722111
transform == null || isIdentityFloat32ListTransform(transform);
20732112

2074-
// If this node has children, we need to compensate for the parent's rect and
2075-
// pass down the scroll adjustments.
2076-
if (hasChildren) {
2077-
final double translateX = -_rect!.left + horizontalScrollAdjustment;
2078-
final double translateY = -_rect!.top + verticalScrollAdjustment;
2079-
2080-
for (final childIndex in _childrenInTraversalOrder!) {
2081-
final child = owner._semanticsTree[childIndex];
2082-
if (child == null) {
2083-
continue;
2084-
}
2085-
child.horizontalAdjustmentFromParent = translateX;
2086-
child.verticalAdjustmentFromParent = translateY;
2087-
}
2088-
}
2089-
20902113
if (hasZeroRectOffset &&
20912114
hasIdentityTransform &&
2092-
verticalAdjustmentFromParent == 0.0 &&
2093-
horizontalAdjustmentFromParent == 0.0) {
2115+
verticalContainerAdjustment == 0.0 &&
2116+
horizontalContainerAdjustment == 0.0) {
20942117
_clearSemanticElementTransform(element);
2118+
if (containerElement != null) {
2119+
_clearSemanticElementTransform(containerElement);
2120+
}
20952121
return;
20962122
}
20972123

20982124
late Matrix4 effectiveTransform;
20992125
bool effectiveTransformIsIdentity = true;
2100-
2101-
final double left = _rect!.left + horizontalAdjustmentFromParent;
2102-
final double top = _rect!.top + verticalAdjustmentFromParent;
2103-
2104-
if (left != 0.0 || top != 0.0) {
2126+
if (!hasZeroRectOffset) {
21052127
if (transform == null) {
2128+
final double left = _rect!.left;
2129+
final double top = _rect!.top;
21062130
effectiveTransform = Matrix4.translationValues(left, top, 0.0);
2107-
effectiveTransformIsIdentity = false;
2131+
effectiveTransformIsIdentity = left == 0.0 && top == 0.0;
21082132
} else {
21092133
// Clone to avoid mutating _transform.
2110-
effectiveTransform = Matrix4.fromFloat32List(transform).clone()..translate(left, top);
2134+
effectiveTransform =
2135+
Matrix4.fromFloat32List(transform).clone()..translate(_rect!.left, _rect!.top);
21112136
effectiveTransformIsIdentity = effectiveTransform.isIdentity();
21122137
}
21132138
} else if (!hasIdentityTransform) {
@@ -2122,16 +2147,19 @@ class SemanticsObject {
21222147
} else {
21232148
_clearSemanticElementTransform(element);
21242149
}
2125-
}
21262150

2127-
/// Computes the size and position of children.
2128-
void updateChildrenPositionAndSize() {
2129-
for (final childIndex in _childrenInTraversalOrder!) {
2130-
final child = owner._semanticsTree[childIndex];
2131-
if (child == null) {
2132-
continue;
2151+
if (containerElement != null) {
2152+
if (!hasZeroRectOffset ||
2153+
verticalContainerAdjustment != 0.0 ||
2154+
horizontalContainerAdjustment != 0.0) {
2155+
final double translateX = -_rect!.left + horizontalContainerAdjustment;
2156+
final double translateY = -_rect!.top + verticalContainerAdjustment;
2157+
containerElement.style
2158+
..top = '${translateY}px'
2159+
..left = '${translateX}px';
2160+
} else {
2161+
_clearSemanticElementTransform(containerElement);
21332162
}
2134-
child.recomputePositionAndSize();
21352163
}
21362164
}
21372165

@@ -2691,7 +2719,7 @@ class EngineSemanticsOwner {
26912719
removals.add(node);
26922720
} else {
26932721
assert(node._parent == parent);
2694-
assert(node.element.parentNode == parent.element);
2722+
assert(node.element.parentNode == parent._childContainerElement);
26952723
}
26962724
return true;
26972725
});
@@ -2800,9 +2828,6 @@ class EngineSemanticsOwner {
28002828
final SemanticsObject object = _semanticsTree[nodeUpdate.id]!;
28012829
object.updateChildren();
28022830
object._dirtyFields = 0;
2803-
2804-
object.recomputePositionAndSize();
2805-
object.updateChildrenPositionAndSize();
28062831
}
28072832

28082833
final SemanticsObject root = _semanticsTree[0]!;
@@ -2838,6 +2863,9 @@ AFTER: $description
28382863
// Dirty fields should be cleared after the tree has been finalized.
28392864
assert(object._dirtyFields == 0);
28402865

2866+
// Make sure a child container is created only when there are children.
2867+
assert(object._childContainerElement == null || object.hasChildren);
2868+
28412869
// Ensure child ID list is consistent with the parent-child
28422870
// relationship of the semantics tree.
28432871
if (object._childrenInTraversalOrder != null) {

engine/src/flutter/lib/web_ui/test/common/matchers.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ class HtmlPatternMatcher extends Matcher {
274274
static bool _areTagsEqual(html.Element a, html.Element b) {
275275
const Map<String, String> synonyms = <String, String>{
276276
'sem': 'flt-semantics',
277+
'sem-c': 'flt-semantics-container',
277278
'sem-img': 'flt-semantics-img',
278279
'sem-tf': 'flt-semantics-text-field',
279280
};

engine/src/flutter/lib/web_ui/test/engine/semantics/semantics_multi_view_test.dart

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,15 @@ Future<void> testMain() async {
9999
// Test that each view renders its own semantics tree.
100100
expectSemanticsTree(view1.semantics, '''
101101
<sem style="filter: opacity(0%); color: rgba(0, 0, 0, 0)">
102+
<sem-c>
102103
<sem flt-tappable="" role="button"></sem>
104+
</sem-c>
103105
</sem>''');
104106
expectSemanticsTree(view2.semantics, '''
105107
<sem style="filter: opacity(0%); color: rgba(0, 0, 0, 0)">
108+
<sem-c>
106109
<sem aria-label="d"><input aria-valuemax="1" aria-valuemin="1" aria-valuenow="1" aria-valuetext="" role="slider"></sem>
110+
</sem-c>
107111
</sem>
108112
''');
109113

0 commit comments

Comments
 (0)