Skip to content

Commit 8d01cd9

Browse files
authored
[two_dimensional_scrollables] Fix TreeView null dereference during paint (#9103)
Collapsing a node when there were other nodes offscreen was causing an unexpected null dereference during painting. This PR fixes the bug and adds a test. The bug was caused by erroneous computation of the max vertical scroll extent. Previously, the code computed this considering only scroll extent in the trailing (down) direction; it may also be the case that there is a larger scroll extent in the leading (up) direction. The miscalculation resulted in subsequent error computing the first visible row as a row that is actually offscreen, and thus does not have a render box. The row render box is asserted to be non-null during painting. Fixes flutter/flutter#149182 and flutter/flutter#164981 ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
1 parent 1f069a7 commit 8d01cd9

File tree

4 files changed

+97
-3
lines changed

4 files changed

+97
-3
lines changed

packages/two_dimensional_scrollables/CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
## NEXT
1+
## 0.3.4
22

33
* Updates minimum supported SDK version to Flutter 3.22/Dart 3.4.
4+
* Fixes a bug where collapsing a node in a TreeView with other offscreen nodes would dereference a null value.
45

56
## 0.3.3
67

packages/two_dimensional_scrollables/lib/src/tree_view/render_tree.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,9 +318,12 @@ class RenderTreeViewport extends RenderTwoDimensionalViewport {
318318
);
319319
_horizontalOverflows = maxHorizontalExtent > 0.0;
320320

321+
final double verticalLeadingExtent = verticalOffset.pixels;
322+
final double verticalTrailingExtent =
323+
_rowMetrics[_lastRow!]!.trailingOffset - viewportDimension.height;
321324
final double maxVerticalExtent = math.max(
322325
0.0,
323-
_rowMetrics[_lastRow!]!.trailingOffset - viewportDimension.height,
326+
math.max(verticalLeadingExtent, verticalTrailingExtent),
324327
);
325328
_verticalOverflows = maxVerticalExtent > 0.0;
326329

packages/two_dimensional_scrollables/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
name: two_dimensional_scrollables
22
description: Widgets that scroll using the two dimensional scrolling foundation.
3-
version: 0.3.3
3+
version: 0.3.4
44
repository: https://github.com/flutter/packages/tree/main/packages/two_dimensional_scrollables
55
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+two_dimensional_scrollables%22+
66

packages/two_dimensional_scrollables/test/tree_view/tree_test.dart

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -886,6 +886,96 @@ void main() {
886886
expect(find.text('tres'), findsNothing);
887887
});
888888
});
889+
890+
testWidgets('Expand then collapse with offscreen nodes (top)',
891+
(WidgetTester tester) async {
892+
final ScrollController verticalController = ScrollController();
893+
final TreeViewController controller = TreeViewController();
894+
addTearDown(verticalController.dispose);
895+
896+
final List<TreeViewNode<String>> tree = <TreeViewNode<String>>[
897+
TreeViewNode<String>(
898+
'alpha',
899+
children: <TreeViewNode<String>>[
900+
TreeViewNode<String>('a'),
901+
TreeViewNode<String>('b'),
902+
TreeViewNode<String>('c'),
903+
],
904+
),
905+
TreeViewNode<String>(
906+
'beta',
907+
children: <TreeViewNode<String>>[
908+
TreeViewNode<String>('d'),
909+
TreeViewNode<String>('e'),
910+
TreeViewNode<String>('f'),
911+
],
912+
),
913+
TreeViewNode<String>(
914+
'gamma',
915+
children: <TreeViewNode<String>>[
916+
TreeViewNode<String>('g'),
917+
TreeViewNode<String>('h'),
918+
TreeViewNode<String>('i'),
919+
],
920+
),
921+
TreeViewNode<String>(
922+
'delta',
923+
children: <TreeViewNode<String>>[
924+
TreeViewNode<String>('j'),
925+
TreeViewNode<String>('k'),
926+
TreeViewNode<String>('l'),
927+
],
928+
),
929+
];
930+
931+
await tester.pumpWidget(MaterialApp(
932+
home: TreeView<String>(
933+
tree: tree,
934+
controller: controller,
935+
toggleAnimationStyle: AnimationStyle.noAnimation,
936+
verticalDetails: ScrollableDetails.vertical(
937+
controller: verticalController,
938+
),
939+
treeNodeBuilder: (
940+
BuildContext context,
941+
TreeViewNode<Object?> node,
942+
AnimationStyle animationStyle,
943+
) =>
944+
GestureDetector(
945+
onTap: () => controller.toggleNode(node),
946+
child: TreeView.defaultTreeNodeBuilder(
947+
context,
948+
node,
949+
animationStyle,
950+
),
951+
),
952+
),
953+
));
954+
955+
// Expand a few nodes
956+
await tester.tap(find.text('alpha'));
957+
await tester.tap(find.text('beta'));
958+
await tester.tap(find.text('gamma'));
959+
await tester.tap(find.text('delta'));
960+
await tester.pumpAndSettle();
961+
962+
// Scroll down to hide some of them
963+
expect(verticalController.position.pixels, 0.0);
964+
const double defaultRowExtent = 40;
965+
verticalController.jumpTo(3 * defaultRowExtent + 10);
966+
await tester.pump();
967+
expect(find.text('alpha'), findsNothing);
968+
969+
// Collapse the bottommost node
970+
expect(find.text('j'), findsOneWidget);
971+
expect(find.text('k'), findsOneWidget);
972+
expect(find.text('l'), findsOneWidget);
973+
await tester.tap(find.text('delta'));
974+
await tester.pumpAndSettle();
975+
expect(find.text('j'), findsNothing);
976+
expect(find.text('k'), findsNothing);
977+
expect(find.text('l'), findsNothing);
978+
});
889979
});
890980

891981
group('TreeViewport', () {

0 commit comments

Comments
 (0)