Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cp] Fix Tooltip crash when selected in a SelectableRegion (#130181) #131288

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 7 additions & 21 deletions packages/flutter/lib/src/material/selection_area.dart
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,7 @@ class SelectionArea extends StatefulWidget {
}

class _SelectionAreaState extends State<SelectionArea> {
FocusNode get _effectiveFocusNode {
if (widget.focusNode != null) {
return widget.focusNode!;
}
_internalNode ??= FocusNode();
return _internalNode!;
}
FocusNode get _effectiveFocusNode => widget.focusNode ?? (_internalNode ??= FocusNode());
FocusNode? _internalNode;

@override
Expand All @@ -121,20 +115,12 @@ class _SelectionAreaState extends State<SelectionArea> {
@override
Widget build(BuildContext context) {
assert(debugCheckHasMaterialLocalizations(context));
TextSelectionControls? controls = widget.selectionControls;
switch (Theme.of(context).platform) {
case TargetPlatform.android:
case TargetPlatform.fuchsia:
controls ??= materialTextSelectionHandleControls;
case TargetPlatform.iOS:
controls ??= cupertinoTextSelectionHandleControls;
case TargetPlatform.linux:
case TargetPlatform.windows:
controls ??= desktopTextSelectionHandleControls;
case TargetPlatform.macOS:
controls ??= cupertinoDesktopTextSelectionHandleControls;
}

final TextSelectionControls controls = widget.selectionControls ?? switch (Theme.of(context).platform) {
TargetPlatform.android || TargetPlatform.fuchsia => materialTextSelectionHandleControls,
TargetPlatform.linux || TargetPlatform.windows => desktopTextSelectionHandleControls,
TargetPlatform.iOS => cupertinoTextSelectionHandleControls,
TargetPlatform.macOS => cupertinoDesktopTextSelectionHandleControls,
};
return SelectableRegion(
selectionControls: controls,
focusNode: _effectiveFocusNode,
Expand Down
19 changes: 16 additions & 3 deletions packages/flutter/lib/src/material/tooltip.dart
Original file line number Diff line number Diff line change
Expand Up @@ -482,13 +482,16 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
void _scheduleDismissTooltip({ required Duration withDelay }) {
assert(mounted);
assert(
!(_timer?.isActive ?? false) || _controller.status != AnimationStatus.reverse,
!(_timer?.isActive ?? false) || _backingController?.status != AnimationStatus.reverse,
'timer must not be active when the tooltip is fading out',
);

_timer?.cancel();
_timer = null;
switch (_controller.status) {
// Use _backingController instead of _controller to prevent the lazy getter
// from instaniating an AnimationController unnecessarily.
switch (_backingController?.status) {
case null:
case AnimationStatus.reverse:
case AnimationStatus.dismissed:
break;
Expand Down Expand Up @@ -740,7 +743,7 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
};

final TooltipThemeData tooltipTheme = _tooltipTheme;
return _TooltipOverlay(
final _TooltipOverlay overlayChild = _TooltipOverlay(
richMessage: widget.richMessage ?? TextSpan(text: widget.message),
height: widget.height ?? tooltipTheme.height ?? _getDefaultTooltipHeight(),
padding: widget.padding ?? tooltipTheme.padding ?? _getDefaultPadding(),
Expand All @@ -755,13 +758,23 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
verticalOffset: widget.verticalOffset ?? tooltipTheme.verticalOffset ?? _defaultVerticalOffset,
preferBelow: widget.preferBelow ?? tooltipTheme.preferBelow ?? _defaultPreferBelow,
);

return SelectionContainer.maybeOf(context) == null
? overlayChild
: SelectionContainer.disabled(child: overlayChild);
}

@override
void dispose() {
GestureBinding.instance.pointerRouter.removeGlobalRoute(_handleGlobalPointerEvent);
Tooltip._openedTooltips.remove(this);
// _longPressRecognizer.dispose() and _tapRecognizer.dispose() may call
// their registered onCancel callbacks if there's a gesture in progress.
// Remove the onCancel callbacks to prevent the registered callbacks from
// triggering unnecessary side effects (such as animations).
_longPressRecognizer?.onLongPressCancel = null;
_longPressRecognizer?.dispose();
_tapRecognizer?.onTapCancel = null;
_tapRecognizer?.dispose();
_timer?.cancel();
_backingController?.dispose();
Expand Down
1 change: 1 addition & 0 deletions packages/flutter/lib/src/widgets/overlay.dart
Original file line number Diff line number Diff line change
Expand Up @@ -1569,6 +1569,7 @@ class _OverlayPortalState extends State<OverlayPortal> {

@override
void dispose() {
assert(widget.controller._attachTarget == this);
widget.controller._attachTarget = null;
_locationCache?._debugMarkLocationInvalid();
_locationCache = null;
Expand Down
91 changes: 91 additions & 0 deletions packages/flutter/test/material/tooltip_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2278,6 +2278,97 @@ void main() {
await tester.pump(const Duration(seconds: 1));
expect(element.dirty, isFalse);
});

testWidgets('Tooltip does not initialize animation controller in dispose process', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
home: Center(
child: Tooltip(
message: tooltipText,
waitDuration: Duration(seconds: 1),
triggerMode: TooltipTriggerMode.longPress,
child: SizedBox.square(dimension: 50),
),
),
),
);

await tester.startGesture(tester.getCenter(find.byType(Tooltip)));
await tester.pumpWidget(const SizedBox());
expect(tester.takeException(), isNull);
});

testWidgets('Tooltip does not crash when showing the tooltip but the OverlayPortal is unmounted, during dispose', (WidgetTester tester) async {
await tester.pumpWidget(
const MaterialApp(
home: SelectionArea(
child: Center(
child: Tooltip(
message: tooltipText,
waitDuration: Duration(seconds: 1),
triggerMode: TooltipTriggerMode.longPress,
child: SizedBox.square(dimension: 50),
),
),
),
),
);

final TooltipState tooltipState = tester.state(find.byType(Tooltip));
await tester.startGesture(tester.getCenter(find.byType(Tooltip)));
tooltipState.ensureTooltipVisible();
await tester.pumpWidget(const SizedBox());
expect(tester.takeException(), isNull);
});

testWidgets('Tooltip is not selectable', (WidgetTester tester) async {
const String tooltipText = 'AAAAAAAAAAAAAAAAAAAAAAA';
String? selectedText;
await tester.pumpWidget(
MaterialApp(
home: SelectionArea(
onSelectionChanged: (SelectedContent? content) { selectedText = content?.plainText; },
child: const Center(
child: Column(
children: <Widget>[
Text('Select Me'),
Tooltip(
message: tooltipText,
waitDuration: Duration(seconds: 1),
triggerMode: TooltipTriggerMode.longPress,
child: SizedBox.square(dimension: 50),
),
],
),
),
),
),
);

final TooltipState tooltipState = tester.state(find.byType(Tooltip));

final Rect textRect = tester.getRect(find.text('Select Me'));
final TestGesture gesture = await tester.startGesture(Alignment.centerLeft.alongSize(textRect.size) + textRect.topLeft);
// Drag from centerLeft to centerRight to select the text.
await tester.pump(const Duration(seconds: 1));
await gesture.moveTo(Alignment.centerRight.alongSize(textRect.size) + textRect.topLeft);
await tester.pump();

tooltipState.ensureTooltipVisible();
await tester.pump();
// Make sure the tooltip becomes visible.
expect(find.text(tooltipText), findsOneWidget);
assert(selectedText != null);

final Rect tooltipTextRect = tester.getRect(find.text(tooltipText));
// Now drag from centerLeft to centerRight to select the tooltip text.
await gesture.moveTo(Alignment.centerLeft.alongSize(tooltipTextRect.size) + tooltipTextRect.topLeft);
await tester.pump();
await gesture.moveTo(Alignment.centerRight.alongSize(tooltipTextRect.size) + tooltipTextRect.topLeft);
await tester.pump();

expect(selectedText, isNot(contains('A')));
});
}

Future<void> setWidgetForTooltipMode(
Expand Down