From f15f2313b97d9c9945217c3d7210c895b550d5e8 Mon Sep 17 00:00:00 2001 From: Valentin Vignal <32538273+ValentinVignal@users.noreply.github.com> Date: Fri, 3 Nov 2023 01:59:16 +0800 Subject: [PATCH] Fixes `DragTarget` crash if `Draggable.data` is `null` (#133136) Makes the `data` parameter of `Draggable` non-nullable. Fixes https://github.com/flutter/flutter/issues/84816 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* --- .../flutter/lib/src/widgets/drag_target.dart | 14 ++- .../flutter/test/widgets/draggable_test.dart | 103 ++++++++++++++++++ 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/packages/flutter/lib/src/widgets/drag_target.dart b/packages/flutter/lib/src/widgets/drag_target.dart index bf323cf6de0..b6567fd01e5 100644 --- a/packages/flutter/lib/src/widgets/drag_target.dart +++ b/packages/flutter/lib/src/widgets/drag_target.dart @@ -652,11 +652,13 @@ class DragTarget extends StatefulWidget { final DragTargetWillAcceptWithDetails? onWillAcceptWithDetails; /// Called when an acceptable piece of data was dropped over this drag target. + /// It will not be called if `data` is `null`. /// /// Equivalent to [onAcceptWithDetails], but only includes the data. final DragTargetAccept? onAccept; /// Called when an acceptable piece of data was dropped over this drag target. + /// It will not be called if `data` is `null`. /// /// Equivalent to [onAccept], but with information, including the data, in a /// [DragTargetDetails]. @@ -666,7 +668,8 @@ class DragTarget extends StatefulWidget { /// the target. final DragTargetLeave? onLeave; - /// Called when a [Draggable] moves within this [DragTarget]. + /// Called when a [Draggable] moves within this [DragTarget]. It will not be + /// called if `data` is `null`. /// /// This includes entering and leaving the target. final DragTargetMove? onMove; @@ -707,6 +710,7 @@ class _DragTargetState extends State> { (widget.onWillAccept != null && widget.onWillAccept!(avatar.data as T?)) || (widget.onWillAcceptWithDetails != null && + avatar.data != null && widget.onWillAcceptWithDetails!(DragTargetDetails(data: avatar.data! as T, offset: avatar._lastOffset!))); if (resolvedWillAccept) { setState(() { @@ -741,12 +745,14 @@ class _DragTargetState extends State> { setState(() { _candidateAvatars.remove(avatar); }); - widget.onAccept?.call(avatar.data! as T); - widget.onAcceptWithDetails?.call(DragTargetDetails(data: avatar.data! as T, offset: avatar._lastOffset!)); + if (avatar.data != null) { + widget.onAccept?.call(avatar.data! as T); + widget.onAcceptWithDetails?.call(DragTargetDetails(data: avatar.data! as T, offset: avatar._lastOffset!)); + } } void didMove(_DragAvatar avatar) { - if (!mounted) { + if (!mounted || avatar.data == null) { return; } widget.onMove?.call(DragTargetDetails(data: avatar.data! as T, offset: avatar._lastOffset!)); diff --git a/packages/flutter/test/widgets/draggable_test.dart b/packages/flutter/test/widgets/draggable_test.dart index f4a4ab6b5e7..6ad69e21fae 100644 --- a/packages/flutter/test/widgets/draggable_test.dart +++ b/packages/flutter/test/widgets/draggable_test.dart @@ -395,6 +395,47 @@ void main() { expect(targetMoveCount['Target 2'], equals(1)); }); + testWidgetsWithLeakTracking('Drag and drop - onMove is not called if moved with null data', (WidgetTester tester) async { + bool onMoveCalled = false; + + await tester.pumpWidget(MaterialApp( + home: Column( + children: [ + const Draggable( + feedback: Text('Dragging'), + child: Text('Source'), + ), + DragTarget( + builder: (BuildContext context, List data, List rejects) { + return const SizedBox(height: 100.0, child: Text('Target')); + }, + onMove: (DragTargetDetails details) { + onMoveCalled = true; + }, + ), + ], + ), + )); + + expect(onMoveCalled, isFalse); + + final Offset firstLocation = tester.getCenter(find.text('Source')); + final TestGesture gesture = await tester.startGesture(firstLocation, pointer: 7); + await tester.pump(); + + expect(onMoveCalled, isFalse); + + final Offset secondLocation = tester.getCenter(find.text('Target')); + await gesture.moveTo(secondLocation); + await tester.pump(); + + expect(onMoveCalled, isFalse); + await gesture.up(); + await tester.pump(); + + expect(onMoveCalled, isFalse); + }); + testWidgetsWithLeakTracking('Drag and drop - dragging over button', (WidgetTester tester) async { final List events = []; Offset firstLocation, secondLocation; @@ -2392,6 +2433,68 @@ void main() { expect(find.text('Target'), findsOneWidget); }); + testWidgetsWithLeakTracking('Drag and drop - onAccept is not called if dropped with null data', (WidgetTester tester) async { + bool onAcceptCalled = false; + bool onAcceptWithDetailsCalled = false; + + await tester.pumpWidget(MaterialApp( + home: Column( + children: [ + const Draggable( + feedback: Text('Dragging'), + child: Text('Source'), + ), + DragTarget( + builder: (BuildContext context, List data, List rejects) { + return const SizedBox(height: 100.0, child: Text('Target')); + }, + onAccept: (int data) { + onAcceptCalled = true; + }, + onAcceptWithDetails: (DragTargetDetails details) { + onAcceptWithDetailsCalled =true; + }, + ), + ], + ), + )); + + expect(onAcceptCalled, isFalse); + expect(onAcceptWithDetailsCalled, isFalse); + expect(find.text('Source'), findsOneWidget); + expect(find.text('Dragging'), findsNothing); + expect(find.text('Target'), findsOneWidget); + + final Offset firstLocation = tester.getCenter(find.text('Source')); + final TestGesture gesture = await tester.startGesture(firstLocation, pointer: 7); + await tester.pump(); + + expect(onAcceptCalled, isFalse); + expect(onAcceptWithDetailsCalled, isFalse); + expect(find.text('Source'), findsOneWidget); + expect(find.text('Dragging'), findsOneWidget); + expect(find.text('Target'), findsOneWidget); + + final Offset secondLocation = tester.getCenter(find.text('Target')); + await gesture.moveTo(secondLocation); + await tester.pump(); + + expect(onAcceptCalled, isFalse); + expect(onAcceptWithDetailsCalled, isFalse); + expect(find.text('Source'), findsOneWidget); + expect(find.text('Dragging'), findsOneWidget); + expect(find.text('Target'), findsOneWidget); + + await gesture.up(); + await tester.pump(); + + expect(onAcceptCalled, isFalse, reason: 'onAccept should not be called when data is null'); + expect(onAcceptWithDetailsCalled, isFalse, reason: 'onAcceptWithDetails should not be called when data is null'); + expect(find.text('Source'), findsOneWidget); + expect(find.text('Dragging'), findsNothing); + expect(find.text('Target'), findsOneWidget); + }); + testWidgetsWithLeakTracking('Draggable disposes recognizer', (WidgetTester tester) async { late final OverlayEntry entry; addTearDown(() => entry..remove()..dispose());