Fix tooltip crash when route has secondary animation #172678

victorsanni · 2025-07-24T00:06:35Z

Consolidates Fix the issue with Tooltip and Delay showing tooltip during page transition

Fix [Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay

justinmc

LGTM with nits 👍 .

justinmc · 2025-07-24T18:31:27Z

packages/flutter/lib/src/material/tooltip.dart

  void _handlePointerDown(PointerDownEvent event) {
    assert(mounted);
-    if (!(ModalRoute.isCurrentOf(context) ?? true)) {
+    if (!(_route?.isCurrent ?? true) || (_route?.secondaryAnimation?.isAnimating ?? false)) {


The secondaryAnimation is animating, meaning that the route is on its way out, so the tooltip shouldn't be interactive? Maybe consider leaving a comment explaining this if you think it's worth it. I guess you'd have to do that in all 3 places here.

I'm curious to understand this too.

I think pulling this logic out as a small private helper method would be helpful in any case; and that'd make a good home for the comment.

(In particular, I think even just writing down a name for that helper function will be helpful for clarifying what this logic is meant to do.)

justinmc · 2025-07-24T18:36:44Z

packages/flutter/test/material/tooltip_test.dart

+      final TestGesture gesture = await tester.createGesture(kind: PointerDeviceKind.mouse);
+      await gesture.addPointer();
+      await tester.tap(find.byType(BackButton));
+      await tester.pump(const Duration(milliseconds: 250));


This is supposed to be a fraction of the page transition right? Use PageTransitionObserver.transitionDuration. That way if the transition changes to be shorter than 250ms, this test will still work.

It's unlikely to be a problem but I was just dealing with this so I'm eager to use PageTransitionObserver :)

packages/flutter/test/material/tooltip_test.dart

justinmc · 2025-07-24T18:41:33Z

packages/flutter/test/material/tooltip_test.dart

+      await tester.tap(find.text('Go to Second Page'));
+      await tester.pumpAndSettle();
+      await tester.tap(find.text('Go to Third Page'));
+      await tester.pumpAndSettle();


Usually pumpAndSettle is not enough to pump through a page transition, though it appears to be working here. Maybe add expects for text that's present on each page so you verify which page(s) are visible at any time.

gnprice

Thanks for fixing this!

packages/flutter/lib/src/material/tooltip.dart

gnprice · 2025-07-24T19:44:42Z

packages/flutter/lib/src/material/tooltip.dart

+    // If the route's secondary animation is animating, the route is on its way
+    // out. So, the tooltip should be non-interactive.
+    final bool routeIsAnimating = _route?.secondaryAnimation?.isAnimating ?? false;


Very helpful, thanks!

gnprice · 2025-07-24T19:46:13Z

packages/flutter/lib/src/material/tooltip.dart

+  bool _isTooltipInteractive() {
+    final bool routeIsCurrent = _route?.isCurrent ?? false;
+    // If the route's secondary animation is animating, the route is on its way
+    // out. So, the tooltip should be non-interactive.
+    final bool routeIsAnimating = _route?.secondaryAnimation?.isAnimating ?? false;
+    return routeIsCurrent && !routeIsAnimating;


If _route is null, then both branches of this fall back, but they end up doing so in opposite directions.

I think it'll probably be easiest to think through the logic if there's an if (_route == null) condition at the top that returns the desired result for that case. Then the rest can say _route! and not think about the null case.

In particular it looks like this PR currently flips the answer that's used when the route is null. I'm not sure what the right answer for that is — but I'm also not sure if if that change was intended, because this logic is a bit hard to read for that case 🙂

(With your update, I believe it no longer flips the answer for when the route is null — instead that continues to be treated as meaning the tooltip should be interactive, as if all this logic weren't here. Which I think is the right answer.)

gnprice

Thanks! LGTM.

LongCatIsLooong · 2025-07-24T20:37:55Z

packages/flutter/lib/src/material/tooltip.dart

    }
  }

+  bool _isTooltipInteractive() {


Is this the right fix? Tooltip ideally should not know about route. This feels too specific and the problem may come up again in a different set up.

Good question! I agree Tooltip ideally shouldn't know about Route. There's probably a cleaner solution by using some other abstraction.

The same comment applies already to the previous fix #168546, which this PR fixes a bug in.

This is the core issue: #167359 (comment). Basically the tooltip shouldn't be displayed mid-transition.

Tooltip uses OverlayPortal behind the scenes:

flutter/packages/flutter/lib/src/material/tooltip.dart

Line 950 in 9f90740

return OverlayPortal(

So maybe the deeper solution is to have OverlayPortal not show its overlaychild in the middle of a route transition? But is that always desired behavior? Or can we hide it behind a flag?

I used the workaround in this PR previously in another PR for the CupertinoContextMenu overlay entry.

The root issue is that RenderObject.getTransformTo should not be called during build, it's unsafe even if in a LayoutBuilder.

Good point that Tooltip ideally shouldn't know about the route. Catching up here:

Is that the getTransformTo in OverlayPortal that you're referring to @LongCatIsLooong? Maybe this?

What is a better solution here?

Outgoing routes shouldn't be interactive at all? Maybe that's a bad user experience, not sure. Incoming routes definitely should be interactive. Does this bug happen on incoming routes too?

The getTransformTo in question should somehow wait until after layout is done?

This isn't something that OverlayPortal.overlayChildLayoutBuilder would help with is it?

Confirmed, OverlayPortal.overlayChildLayoutBuilder fixes the issue. Thanks @LongCatIsLooong.

I did a quick grep through the codebase, RawMenuAnchor also uses OverlayPortal and localToGlobal in its buildOverlay method, so this problem might occur there too. Maybe I can add a TODO to migrate to OverlayPortal.overlayChildLayoutBuilder.

Yes good call, can you create an issue too?

Filed #173440

justinmc

LGTM, this is a really simple fix now 👍

justinmc · 2025-08-07T21:54:58Z

packages/flutter/lib/src/material/tooltip.dart

-    assert(mounted);
-    if (!(ModalRoute.isCurrentOf(context) ?? true)) {
-      return;
-    }


Nice that we don't need these anymore!

justinmc · 2025-08-07T21:55:36Z

packages/flutter/lib/src/material/tooltip.dart

    }
  }

+  bool _isTooltipInteractive() {


Yes good call, can you create an issue too?

…er/flutter#172678)

flutter/flutter@3821790...1590543 2025-08-09 [email protected] Make device debuggable if useDwdsWebSocketConnection is true and added simple test case (flutter/flutter#171648) 2025-08-09 [email protected] Roll Dart SDK from 91cbf6d7563a to 6a7ae1ffd1c9 (1 revision) (flutter/flutter#173487) 2025-08-09 [email protected] add `--variance host_debug_unopt_arm64` for apple chip simulator (flutter/flutter#173475) 2025-08-08 [email protected] [WebParagraph] Fix a property name on newer Chrome versions (flutter/flutter#173477) 2025-08-08 [email protected] Make sure that a Checkbox doesn't crash in 0x0 environment (flutter/flutter#173178) 2025-08-08 [email protected] Make sure that a RawChip doesn't crash in 0x0 environment (flutter/flutter#173265) 2025-08-08 [email protected] Fix tooltip crash when route has secondary animation (flutter/flutter#172678) 2025-08-08 [email protected] Roll Skia from 86824ed582be to 44bb9d908ee4 (3 revisions) (flutter/flutter#173476) 2025-08-08 [email protected] Fix null value reference in `flutter logs` path (flutter/flutter#173437) 2025-08-08 [email protected] Remove jetifier usages from framework and engine (flutter/flutter#173459) 2025-08-08 [email protected] [a11y] Textfield has flag `isFocusable` set to true (flutter/flutter#173235) 2025-08-08 [email protected] Roll Dart SDK from 4b7b565eb468 to 91cbf6d7563a (1 revision) (flutter/flutter#173469) 2025-08-08 [email protected] Roll Packages from 6efb759 to 34948d1 (4 revisions) (flutter/flutter#173470) 2025-08-08 [email protected] Roll Skia from a6ccfeafbfba to 86824ed582be (20 revisions) (flutter/flutter#173468) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Makes the tooltip interactive if the route is the current route and the route is not animating out. Added `_route` to cache `ModalRoute.of(context)`. Consolidates [Fix the issue with Tooltip](flutter#168546) and [Delay showing tooltip during page transition](flutter#167614) Fix [[Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay](flutter#169741)

…er/flutter#172678)

Makes the tooltip interactive if the route is the current route and the route is not animating out. Added `_route` to cache `ModalRoute.of(context)`. Consolidates [Fix the issue with Tooltip](flutter#168546) and [Delay showing tooltip during page transition](flutter#167614) Fix [[Desktop] [Web] [Regression] [3.32] AppBar back - RenderBox was not laid out - TooltipState._buildTooltipOverlay](flutter#169741)

Fix tooltip crash

0bf182d

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jul 24, 2025

victorsanni added 3 commits July 23, 2025 17:07

Merge branch 'master' into modal-route-fix

f0406cc

Add test

5d820e3

Merge branch 'master' into modal-route-fix

326faec

victorsanni marked this pull request as ready for review July 24, 2025 18:19

victorsanni requested review from gnprice, justinmc and yiiim July 24, 2025 18:20

victorsanni changed the title ~~Fix tooltip crash~~ Fix tooltip crash when route has secondary animation Jul 24, 2025

More tweaks

464f0ad

justinmc approved these changes Jul 24, 2025

View reviewed changes

justinmc reviewed Jul 24, 2025

View reviewed changes

gnprice reviewed Jul 24, 2025

View reviewed changes

packages/flutter/lib/src/material/tooltip.dart Outdated Show resolved Hide resolved

Apply review suggestions

2288b42

victorsanni requested a review from gnprice July 24, 2025 19:47

gnprice reviewed Jul 24, 2025

View reviewed changes

Null check at the top of helper method

8c8b508

victorsanni requested a review from gnprice July 24, 2025 19:58

gnprice approved these changes Jul 24, 2025

View reviewed changes

LongCatIsLooong reviewed Jul 24, 2025

View reviewed changes

victorsanni added 3 commits August 7, 2025 14:35

Migrate to OverlayPortal.overlayChildLayoutBuilder

b092eb1

Merge branch 'master' into modal-route-fix

e1ebdba

Merge branch 'master' into modal-route-fix

fb624a7

victorsanni requested review from LongCatIsLooong and justinmc August 7, 2025 21:37

justinmc approved these changes Aug 7, 2025

View reviewed changes

victorsanni added 2 commits August 8, 2025 13:10

Stop early if child is not visible

f90a0c4

Merge branch 'master' into modal-route-fix

d0a0237

LongCatIsLooong approved these changes Aug 8, 2025

View reviewed changes

victorsanni added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 8, 2025

auto-submit bot added this pull request to the merge queue Aug 8, 2025

Merged via the queue into flutter:master with commit f1abbe2 Aug 8, 2025
75 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 8, 2025

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 9, 2025

f1abbe2bf Fix tooltip crash when route has secondary animation (flutt…

7ff6411

…er/flutter#172678)

engine-flutter-autoroll mentioned this pull request Aug 9, 2025

Roll Flutter from 38217906e95c to 1590543f6794 (14 revisions) flutter/packages#9779

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 9, 2025

f1abbe2bf Fix tooltip crash when route has secondary animation (flutt…

296b417

…er/flutter#172678)

engine-flutter-autoroll mentioned this pull request Aug 9, 2025

Roll Flutter from 38217906e95c to 1590543f6794 (14 revisions) flutter/packages#9780

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025

f1abbe2bf Fix tooltip crash when route has secondary animation (flutt…

703e433

…er/flutter#172678)

Fix tooltip crash when route has secondary animation #172678

Fix tooltip crash when route has secondary animation #172678

Uh oh!

Conversation

victorsanni commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

victorsanni commented Jul 24, 2025 •

edited

Loading