Fix scrollable to clear inner semantics node if it does not use two p… (#120996)

* Fix scrollable to clear inner semantics node if it does not use two panel

* g

* add more check
This commit is contained in:
chunhtai 2023-02-17 14:33:15 -08:00 committed by GitHub
parent 6205c110d6
commit 1daa0be4f9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 62 additions and 0 deletions

View file

@ -1659,6 +1659,7 @@ class _RenderScrollSemantics extends RenderProxyBox {
@override
void assembleSemanticsNode(SemanticsNode node, SemanticsConfiguration config, Iterable<SemanticsNode> children) {
if (children.isEmpty || !children.first.isTagged(RenderViewport.useTwoPaneSemantics)) {
_innerNode = null;
super.assembleSemanticsNode(node, config, children);
return;
}

View file

@ -11,6 +11,8 @@ import 'package:flutter/scheduler.dart';
import 'package:flutter/services.dart';
import 'package:flutter_test/flutter_test.dart';
import 'semantics_tester.dart';
Future<void> pumpTest(
WidgetTester tester,
TargetPlatform? platform, {
@ -1643,6 +1645,51 @@ void main() {
await tester.pump(const Duration(milliseconds: 4800));
expect(getScrollOffset(tester), closeTo(333.2944, 0.0001));
});
testWidgets('Swapping viewports in a scrollable does not crash', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester);
final GlobalKey key = GlobalKey();
final GlobalKey key1 = GlobalKey();
Widget buildScrollable(bool withViewPort) {
return Scrollable(
key: key,
viewportBuilder: (BuildContext context, ViewportOffset position) {
if (withViewPort) {
return Viewport(
slivers: <Widget>[
SliverToBoxAdapter(child: Semantics(key: key1, container: true, child: const Text('text1')))
],
offset: ViewportOffset.zero(),
);
}
return Semantics(key: key1, container: true, child: const Text('text1'));
},
);
}
// This should cache the inner node in Scrollable with the children text1.
await tester.pumpWidget(
MaterialApp(
home: buildScrollable(true),
),
);
expect(semantics, includesNodeWith(tags: <SemanticsTag>{RenderViewport.useTwoPaneSemantics}));
// This does not use two panel, this should clear cached inner node.
await tester.pumpWidget(
MaterialApp(
home: buildScrollable(false),
),
);
expect(semantics, isNot(includesNodeWith(tags: <SemanticsTag>{RenderViewport.useTwoPaneSemantics})));
// If the inner node was cleared in the previous step, this should not crash.
await tester.pumpWidget(
MaterialApp(
home: buildScrollable(true),
),
);
expect(semantics, includesNodeWith(tags: <SemanticsTag>{RenderViewport.useTwoPaneSemantics}));
expect(tester.takeException(), isNull);
semantics.dispose();
});
}
// ignore: must_be_immutable

View file

@ -487,6 +487,7 @@ class SemanticsTester {
TextDirection? textDirection,
List<SemanticsAction>? actions,
List<SemanticsFlag>? flags,
Set<SemanticsTag>? tags,
double? scrollPosition,
double? scrollExtentMax,
double? scrollExtentMin,
@ -536,6 +537,12 @@ class SemanticsTester {
return false;
}
}
if (tags != null) {
final Set<SemanticsTag>? actualTags = node.getSemanticsData().tags;
if (!setEquals<SemanticsTag>(actualTags, tags)) {
return false;
}
}
if (scrollPosition != null && !nearEqual(node.scrollPosition, scrollPosition, 0.1)) {
return false;
}
@ -796,6 +803,7 @@ class _IncludesNodeWith extends Matcher {
this.textDirection,
this.actions,
this.flags,
this.tags,
this.scrollPosition,
this.scrollExtentMax,
this.scrollExtentMin,
@ -806,6 +814,7 @@ class _IncludesNodeWith extends Matcher {
value != null ||
actions != null ||
flags != null ||
tags != null ||
scrollPosition != null ||
scrollExtentMax != null ||
scrollExtentMin != null ||
@ -821,6 +830,7 @@ class _IncludesNodeWith extends Matcher {
final TextDirection? textDirection;
final List<SemanticsAction>? actions;
final List<SemanticsFlag>? flags;
final Set<SemanticsTag>? tags;
final double? scrollPosition;
final double? scrollExtentMax;
final double? scrollExtentMin;
@ -839,6 +849,7 @@ class _IncludesNodeWith extends Matcher {
textDirection: textDirection,
actions: actions,
flags: flags,
tags: tags,
scrollPosition: scrollPosition,
scrollExtentMax: scrollExtentMax,
scrollExtentMin: scrollExtentMin,
@ -865,6 +876,7 @@ class _IncludesNodeWith extends Matcher {
if (textDirection != null) ' (${textDirection!.name})',
if (actions != null) 'actions "${actions!.join(', ')}"',
if (flags != null) 'flags "${flags!.join(', ')}"',
if (tags != null) 'tags "${tags!.join(', ')}"',
if (scrollPosition != null) 'scrollPosition "$scrollPosition"',
if (scrollExtentMax != null) 'scrollExtentMax "$scrollExtentMax"',
if (scrollExtentMin != null) 'scrollExtentMin "$scrollExtentMin"',
@ -889,6 +901,7 @@ Matcher includesNodeWith({
TextDirection? textDirection,
List<SemanticsAction>? actions,
List<SemanticsFlag>? flags,
Set<SemanticsTag>? tags,
double? scrollPosition,
double? scrollExtentMax,
double? scrollExtentMin,
@ -905,6 +918,7 @@ Matcher includesNodeWith({
textDirection: textDirection,
actions: actions,
flags: flags,
tags: tags,
scrollPosition: scrollPosition,
scrollExtentMax: scrollExtentMax,
scrollExtentMin: scrollExtentMin,