Unverified Commit 7a3135b9 authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Make `SemanticsNode.isMergedIntoParent` Readonly (#137304)

Fixes https://github.com/flutter/flutter/issues/54665
parent 3ef84471
......@@ -4880,7 +4880,6 @@ class _SwitchableSemanticsFragment extends _InterestingSemanticsFragment {
}
final SemanticsNode node = (owner._semantics ??= SemanticsNode(showOnScreen: owner.showOnScreen))
..isMergedIntoParent = _mergeIntoParent
..tags = _tagsForChildren;
node.elevationAdjustment = elevationAdjustment;
......@@ -4942,9 +4941,7 @@ class _SwitchableSemanticsFragment extends _InterestingSemanticsFragment {
// They need to share the same transform if they are going to attach to the
// parent of the immediate explicit node.
assert(siblingNode.transform == null);
siblingNode
..transform = node.transform
..isMergedIntoParent = node.isMergedIntoParent;
siblingNode.transform = node.transform;
if (_tagsForChildren != null) {
siblingNode.tags ??= <SemanticsTag>{};
siblingNode.tags!.addAll(_tagsForChildren!);
......
......@@ -1230,7 +1230,7 @@ class RenderParagraph extends RenderBox with ContainerRenderObjectMixin<RenderBo
final Rect paintRect = node.parentPaintClipRect!.intersect(currentRect);
configuration.isHidden = paintRect.isEmpty && !currentRect.isEmpty;
}
late final SemanticsNode newChild;
final SemanticsNode newChild;
if (_cachedChildNodes?.isNotEmpty ?? false) {
newChild = _cachedChildNodes!.remove(_cachedChildNodes!.keys.first)!;
} else {
......
......@@ -1626,10 +1626,7 @@ class _RenderScrollSemantics extends RenderProxyBox {
return;
}
_innerNode ??= SemanticsNode(showOnScreen: showOnScreen);
_innerNode!
..isMergedIntoParent = node.isPartOfNodeMerging
..rect = node.rect;
(_innerNode ??= SemanticsNode(showOnScreen: showOnScreen)).rect = node.rect;
int? firstVisibleIndex;
final List<SemanticsNode> excluded = <SemanticsNode>[_innerNode!];
......
......@@ -58,7 +58,6 @@ void main() {
config: config,
childrenInInversePaintOrder: <SemanticsNode>[
SemanticsNode()
..isMergedIntoParent = true
..rect = const Rect.fromLTRB(5.0, 5.0, 10.0, 10.0)
..tags = tags,
],
......@@ -172,6 +171,135 @@ void main() {
);
});
test('provides the correct isMergedIntoParent value', () {
final SemanticsNode root = SemanticsNode()..rect = const Rect.fromLTRB(0.0, 0.0, 10.0, 10.0);
final SemanticsNode node1 = SemanticsNode()..rect = const Rect.fromLTRB(1.0, 0.0, 10.0, 10.0);
final SemanticsNode node11 = SemanticsNode()..rect = const Rect.fromLTRB(2.0, 0.0, 10.0, 10.0);
final SemanticsNode node12 = SemanticsNode()..rect = const Rect.fromLTRB(3.0, 0.0, 10.0, 10.0);
final SemanticsConfiguration noMergeConfig = SemanticsConfiguration()
..isSemanticBoundary = true
..isMergingSemanticsOfDescendants = false;
final SemanticsConfiguration mergeConfig = SemanticsConfiguration()
..isSemanticBoundary = true
..isMergingSemanticsOfDescendants = true;
node1.updateWith(config: noMergeConfig, childrenInInversePaintOrder: <SemanticsNode>[node11, node12]);
expect(node1.isMergedIntoParent, false);
expect(node1.mergeAllDescendantsIntoThisNode, false);
expect(node11.isMergedIntoParent, false);
expect(node12.isMergedIntoParent, false);
expect(root.isMergedIntoParent, false);
root.updateWith(config: mergeConfig, childrenInInversePaintOrder: <SemanticsNode>[node1]);
expect(node1.isMergedIntoParent, true);
expect(node1.mergeAllDescendantsIntoThisNode, false);
expect(node11.isMergedIntoParent, true);
expect(node12.isMergedIntoParent, true);
expect(root.isMergedIntoParent, false);
expect(root.mergeAllDescendantsIntoThisNode, true);
// Change config
node1.updateWith(config: mergeConfig, childrenInInversePaintOrder: <SemanticsNode>[node11, node12]);
expect(node1.isMergedIntoParent, true);
expect(node1.mergeAllDescendantsIntoThisNode, true);
expect(node11.isMergedIntoParent, true);
expect(node12.isMergedIntoParent, true);
expect(root.isMergedIntoParent, false);
expect(root.mergeAllDescendantsIntoThisNode, true);
root.updateWith(config: noMergeConfig, childrenInInversePaintOrder: <SemanticsNode>[node1]);
expect(node1.isMergedIntoParent, false);
expect(node1.mergeAllDescendantsIntoThisNode, true);
expect(node11.isMergedIntoParent, true);
expect(node12.isMergedIntoParent, true);
expect(root.isMergedIntoParent, false);
expect(root.mergeAllDescendantsIntoThisNode, false);
});
test('sendSemanticsUpdate verifies no invisible nodes', () {
const Rect invisibleRect = Rect.fromLTRB(0.0, 0.0, 0.0, 10.0);
const Rect visibleRect = Rect.fromLTRB(0.0, 0.0, 10.0, 10.0);
final SemanticsOwner owner = SemanticsOwner(
onSemanticsUpdate: (SemanticsUpdate update) {},
);
final SemanticsNode root = SemanticsNode.root(owner: owner)..rect = invisibleRect;
final SemanticsNode child = SemanticsNode();
// It's ok to have an invisible root.
expect(owner.sendSemanticsUpdate, returnsNormally);
// It's ok to have an invisible child if it's merged to an ancestor.
root
..rect = visibleRect
..updateWith(
config: SemanticsConfiguration()
..isSemanticBoundary = true
..isMergingSemanticsOfDescendants = true,
childrenInInversePaintOrder: <SemanticsNode>[child..rect = invisibleRect],
);
expect(owner.sendSemanticsUpdate, returnsNormally);
// It's ok if all nodes are visible.
root
..rect = visibleRect
..updateWith(
config: SemanticsConfiguration()
..isSemanticBoundary = true
..isMergingSemanticsOfDescendants = false,
childrenInInversePaintOrder: <SemanticsNode>[child..rect = visibleRect],
);
expect(owner.sendSemanticsUpdate, returnsNormally);
// Invisible root with children bad.
root
..rect = invisibleRect
..updateWith(
config: SemanticsConfiguration()
..isSemanticBoundary = true
..isMergingSemanticsOfDescendants = true,
childrenInInversePaintOrder: <SemanticsNode>[child..rect = invisibleRect],
);
expect(owner.sendSemanticsUpdate, throwsA(isA<FlutterError>().having(
(FlutterError error) => error.message, 'message', equals(
'Invisible SemanticsNodes should not be added to the tree.\n'
'The following invisible SemanticsNodes were added to the tree:\n'
'SemanticsNode#0(dirty, merge boundary ⛔️, Rect.fromLTRB(0.0, 0.0, 0.0, 10.0), invisible)\n'
'which was added as the root SemanticsNode\n'
'An invisible SemanticsNode is one whose rect is not on screen hence not reachable for users, and its semantic information is not merged into a visible parent.\n'
'An invisible SemantiscNode makes the accessibility experience confusing, as it does not provide any visual indication when the user selects it via accessibility technologies.\n'
'Consider removing the above invisible SemanticsNodes if they were added by your RenderObject.assembleSemanticsNode implementation, or filing a bug on GitHub:\n'
' https://github.com/flutter/flutter/issues/new?template=2_bug.yml'
),
)));
// Invisible children bad.
root
..rect = visibleRect
..updateWith(
config: SemanticsConfiguration()
..isSemanticBoundary = true
..isMergingSemanticsOfDescendants = false,
childrenInInversePaintOrder: <SemanticsNode>[child..rect = invisibleRect],
);
expect(owner.sendSemanticsUpdate, throwsA(isA<FlutterError>().having(
(FlutterError error) => error.message, 'message', equals(
'Invisible SemanticsNodes should not be added to the tree.\n'
'The following invisible SemanticsNodes were added to the tree:\n'
'SemanticsNode#1(dirty, Rect.fromLTRB(0.0, 0.0, 0.0, 10.0), invisible)\n'
'which was added as a child of:\n'
' SemanticsNode#0(dirty, Rect.fromLTRB(0.0, 0.0, 10.0, 10.0))\n'
'An invisible SemanticsNode is one whose rect is not on screen hence not reachable for users, and its semantic information is not merged into a visible parent.\n'
'An invisible SemantiscNode makes the accessibility experience confusing, as it does not provide any visual indication when the user selects it via accessibility technologies.\n'
'Consider removing the above invisible SemanticsNodes if they were added by your RenderObject.assembleSemanticsNode implementation, or filing a bug on GitHub:\n'
' https://github.com/flutter/flutter/issues/new?template=2_bug.yml'
),
)));
});
test('mutate existing semantic node list errors', () {
final SemanticsNode node = SemanticsNode()
..rect = const Rect.fromLTRB(0.0, 0.0, 10.0, 10.0);
......@@ -182,7 +310,6 @@ void main() {
final List<SemanticsNode> children = <SemanticsNode>[
SemanticsNode()
..isMergedIntoParent = true
..rect = const Rect.fromLTRB(5.0, 5.0, 10.0, 10.0),
];
......@@ -193,8 +320,7 @@ void main() {
children.add(
SemanticsNode()
..isMergedIntoParent = true
..rect = const Rect.fromLTRB(42.0, 42.0, 10.0, 10.0),
..rect = const Rect.fromLTRB(42.0, 42.0, 52.0, 52.0),
);
{
......@@ -223,10 +349,8 @@ void main() {
late FlutterError error;
final List<SemanticsNode> modifiedChildren = <SemanticsNode>[
SemanticsNode()
..isMergedIntoParent = true
..rect = const Rect.fromLTRB(5.0, 5.0, 10.0, 10.0),
SemanticsNode()
..isMergedIntoParent = true
..rect = const Rect.fromLTRB(10.0, 10.0, 20.0, 20.0),
];
node.updateWith(
......@@ -235,11 +359,9 @@ void main() {
);
try {
modifiedChildren[0] = SemanticsNode()
..isMergedIntoParent = true
..rect = const Rect.fromLTRB(0.0, 0.0, 20.0, 20.0);
modifiedChildren[1] = SemanticsNode()
..isMergedIntoParent = true
..rect = const Rect.fromLTRB(40.0, 14.0, 20.0, 20.0);
..rect = const Rect.fromLTRB(40.0, 14.0, 60.0, 60.0);
node.updateWith(
config: config,
childrenInInversePaintOrder: modifiedChildren,
......@@ -255,12 +377,12 @@ void main() {
' containing the desired `SemanticsNode`s.\n'
' Error details:\n'
' Child node at position 0 was replaced:\n'
' Previous child: SemanticsNode#6(STALE, owner: null, merged up ⬆️, Rect.fromLTRB(0.0, 0.0, 20.0, 20.0))\n'
' New child: SemanticsNode#4(STALE, owner: null, merged up ⬆️, Rect.fromLTRB(5.0, 5.0, 10.0, 10.0))\n'
' Previous child: SemanticsNode#4(STALE, owner: null, merged up ⬆️, Rect.fromLTRB(5.0, 5.0, 10.0, 10.0))\n'
' New child: SemanticsNode#6(STALE, owner: null, merged up ⬆️, Rect.fromLTRB(0.0, 0.0, 20.0, 20.0))\n'
'\n'
' Child node at position 1 was replaced:\n'
' Previous child: SemanticsNode#7(STALE, owner: null, merged up ⬆️, Rect.fromLTRB(40.0, 14.0, 20.0, 20.0))\n'
' New child: SemanticsNode#5(STALE, owner: null, merged up ⬆️, Rect.fromLTRB(10.0, 10.0, 20.0, 20.0))\n',
' Previous child: SemanticsNode#5(STALE, owner: null, merged up ⬆️, Rect.fromLTRB(10.0, 10.0, 20.0, 20.0))\n'
' New child: SemanticsNode#7(STALE, owner: null, merged up ⬆️, Rect.fromLTRB(40.0, 14.0, 60.0, 60.0))\n',
));
expect(
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment