Unverified Commit 2da353a5 authored by LongCatIsLooong's avatar LongCatIsLooong Committed by GitHub

Exclude `Tooltip`'s overlay child from SelectableRegion (#130181)

Fixes https://github.com/flutter/flutter/issues/129969 by making tooltip text unselectable (for now). 
Also fixes some other issues uncovered when I was writing the tests.

Currently `getTransformTo` only works on ancestors. I'll try to add a new method that computes the transform from 2 arbitrary render objects in the same render tree in a follow-up PR and make `Selectable` use that method instead.
parent dd0b6e35
...@@ -103,13 +103,7 @@ class SelectionArea extends StatefulWidget { ...@@ -103,13 +103,7 @@ class SelectionArea extends StatefulWidget {
} }
class _SelectionAreaState extends State<SelectionArea> { class _SelectionAreaState extends State<SelectionArea> {
FocusNode get _effectiveFocusNode { FocusNode get _effectiveFocusNode => widget.focusNode ?? (_internalNode ??= FocusNode());
if (widget.focusNode != null) {
return widget.focusNode!;
}
_internalNode ??= FocusNode();
return _internalNode!;
}
FocusNode? _internalNode; FocusNode? _internalNode;
@override @override
...@@ -121,20 +115,12 @@ class _SelectionAreaState extends State<SelectionArea> { ...@@ -121,20 +115,12 @@ class _SelectionAreaState extends State<SelectionArea> {
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
assert(debugCheckHasMaterialLocalizations(context)); assert(debugCheckHasMaterialLocalizations(context));
TextSelectionControls? controls = widget.selectionControls; final TextSelectionControls controls = widget.selectionControls ?? switch (Theme.of(context).platform) {
switch (Theme.of(context).platform) { TargetPlatform.android || TargetPlatform.fuchsia => materialTextSelectionHandleControls,
case TargetPlatform.android: TargetPlatform.linux || TargetPlatform.windows => desktopTextSelectionHandleControls,
case TargetPlatform.fuchsia: TargetPlatform.iOS => cupertinoTextSelectionHandleControls,
controls ??= materialTextSelectionHandleControls; TargetPlatform.macOS => cupertinoDesktopTextSelectionHandleControls,
case TargetPlatform.iOS: };
controls ??= cupertinoTextSelectionHandleControls;
case TargetPlatform.linux:
case TargetPlatform.windows:
controls ??= desktopTextSelectionHandleControls;
case TargetPlatform.macOS:
controls ??= cupertinoDesktopTextSelectionHandleControls;
}
return SelectableRegion( return SelectableRegion(
selectionControls: controls, selectionControls: controls,
focusNode: _effectiveFocusNode, focusNode: _effectiveFocusNode,
......
...@@ -482,13 +482,16 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin { ...@@ -482,13 +482,16 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
void _scheduleDismissTooltip({ required Duration withDelay }) { void _scheduleDismissTooltip({ required Duration withDelay }) {
assert(mounted); assert(mounted);
assert( 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 must not be active when the tooltip is fading out',
); );
_timer?.cancel(); _timer?.cancel();
_timer = null; _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.reverse:
case AnimationStatus.dismissed: case AnimationStatus.dismissed:
break; break;
...@@ -740,7 +743,7 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin { ...@@ -740,7 +743,7 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
}; };
final TooltipThemeData tooltipTheme = _tooltipTheme; final TooltipThemeData tooltipTheme = _tooltipTheme;
return _TooltipOverlay( final _TooltipOverlay overlayChild = _TooltipOverlay(
richMessage: widget.richMessage ?? TextSpan(text: widget.message), richMessage: widget.richMessage ?? TextSpan(text: widget.message),
height: widget.height ?? tooltipTheme.height ?? _getDefaultTooltipHeight(), height: widget.height ?? tooltipTheme.height ?? _getDefaultTooltipHeight(),
padding: widget.padding ?? tooltipTheme.padding ?? _getDefaultPadding(), padding: widget.padding ?? tooltipTheme.padding ?? _getDefaultPadding(),
...@@ -755,13 +758,23 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin { ...@@ -755,13 +758,23 @@ class TooltipState extends State<Tooltip> with SingleTickerProviderStateMixin {
verticalOffset: widget.verticalOffset ?? tooltipTheme.verticalOffset ?? _defaultVerticalOffset, verticalOffset: widget.verticalOffset ?? tooltipTheme.verticalOffset ?? _defaultVerticalOffset,
preferBelow: widget.preferBelow ?? tooltipTheme.preferBelow ?? _defaultPreferBelow, preferBelow: widget.preferBelow ?? tooltipTheme.preferBelow ?? _defaultPreferBelow,
); );
return SelectionContainer.maybeOf(context) == null
? overlayChild
: SelectionContainer.disabled(child: overlayChild);
} }
@override @override
void dispose() { void dispose() {
GestureBinding.instance.pointerRouter.removeGlobalRoute(_handleGlobalPointerEvent); GestureBinding.instance.pointerRouter.removeGlobalRoute(_handleGlobalPointerEvent);
Tooltip._openedTooltips.remove(this); 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(); _longPressRecognizer?.dispose();
_tapRecognizer?.onTapCancel = null;
_tapRecognizer?.dispose(); _tapRecognizer?.dispose();
_timer?.cancel(); _timer?.cancel();
_backingController?.dispose(); _backingController?.dispose();
......
...@@ -1569,6 +1569,7 @@ class _OverlayPortalState extends State<OverlayPortal> { ...@@ -1569,6 +1569,7 @@ class _OverlayPortalState extends State<OverlayPortal> {
@override @override
void dispose() { void dispose() {
assert(widget.controller._attachTarget == this);
widget.controller._attachTarget = null; widget.controller._attachTarget = null;
_locationCache?._debugMarkLocationInvalid(); _locationCache?._debugMarkLocationInvalid();
_locationCache = null; _locationCache = null;
......
...@@ -2278,6 +2278,97 @@ void main() { ...@@ -2278,6 +2278,97 @@ void main() {
await tester.pump(const Duration(seconds: 1)); await tester.pump(const Duration(seconds: 1));
expect(element.dirty, isFalse); 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( Future<void> setWidgetForTooltipMode(
......
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