Sitelet https://github.com/flutter/flutter/pull/193192
Skip to content

Reland "fix(widgets): dismiss open RawTooltip on Escape key (WCAG 1.4.13)" - #193192

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
kevmoo:reland-raw-tooltip-escape
Sep 23, 2026
Merged

auto-submit[bot] merged 3 commits into
flutter:masterfrom
kevmoo:reland-raw-tooltip-escape

Conversation

@kevmoo

@kevmoo kevmoo commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Relands #192966 (reverted in #193183) with OverlayEntry disposal in raw_tooltip_test.dart and FocusManager early key-event interception in raw_tooltip.dart.

What changed since #192966 (f8c85519a060)

  1. Test OverlayEntry Leak Fix (raw_tooltip_test.dart):
    • In raw_tooltip_test.dart (Escape key dismisses hovered RawTooltip even when primaryFocus is on a sibling consuming Escape), #192966 passed an inline OverlayEntry(...) to Overlay(initialEntries: <OverlayEntry>[...]) without calling entry.remove() and entry.dispose() in addTearDown. Because Overlay does not dispose initialEntries on unmount, the OverlayEntry's internal ValueNotifier<_OverlayEntryWidgetState?> was flagged as notDisposed by leak_tracker in CI (Linux framework_tests_widgets_leak_tracking).
    • This reland captures OverlayEntry? entry and registers addTearDown(() { entry?.remove(); entry?.dispose(); }). Verified locally with flutter test --dart-define=LEAK_TRACKING=true test/widgets/raw_tooltip_test.dart.
  2. Stop Escape Propagation Before Focus-Tree Shortcuts (raw_tooltip.dart):
    • In #192966, RawTooltipState registered HardwareKeyboard.instance.addHandler(_handleKeyEvent) alone. Because KeyEventManager runs HardwareKeyboard handlers before FocusManager.handleKeyMessage without stopping the focus-tree walk when returning true, emptying RawTooltip._openedTooltips in HardwareKeyboard caused Escape to still reach focused siblings (siblingReceivedEscape) or WidgetsApp's Shortcuts (DismissIntent on enclosing ModalRoutes such as DatePickerDialog).
    • This reland registers a single static pair (FocusManager.instance.addEarlyKeyEventHandler(_handleEarlyKeyEvent) plus HardwareKeyboard.instance.addHandler(_handleHardwareKeyEvent) fallback when primaryFocus == null) when RawTooltip._openedTooltips transitions from empty to non-empty, and removes them when the last open tooltip closes or disposes. Returning KeyEventResult.handled from _handleEarlyKeyEvent stops FocusManager before walking the focus tree, and a second test (Escape key dismisses open RawTooltip inside WidgetsApp without invoking ancestor DismissIntent) verifies both cases.

Original description

Fixes http://b/463905826

WCAG 1.4.13 (Content on Hover or Focus) requires a mechanism to dismiss additional content triggered by pointer hover or keyboard focus (Escape key) without moving pointer hover or keyboard focus, and without unintended side effects such as dismissing an enclosing modal dialog (DatePickerDialog) when a tooltip is open.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Sep 22, 2026
@github-actions github-actions Bot added the framework flutter/packages/flutter repository. See also f: labels. label Sep 22, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request registers early key event handlers on FocusManager and HardwareKeyboard when a RawTooltip is shown, ensuring the Escape key dismisses the tooltip without triggering other focused elements or ancestor DismissIntent handlers. The feedback suggests refactoring these handlers to be static and managing their registration globally to avoid redundant allocations. Additionally, it recommends replacing a late final variable in the test's teardown block with a nullable variable to prevent potential LateInitializationErrors during setup failures.

Comment thread packages/flutter/lib/src/widgets/raw_tooltip.dart
Comment thread packages/flutter/lib/src/widgets/raw_tooltip.dart Outdated
Comment thread packages/flutter/lib/src/widgets/raw_tooltip.dart Outdated
Comment thread packages/flutter/test/widgets/raw_tooltip_test.dart Outdated
navaronbracke
navaronbracke previously approved these changes Sep 23, 2026

@navaronbracke navaronbracke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nit

Thanks for the extra test, too!

Comment thread packages/flutter/test/widgets/raw_tooltip_test.dart Outdated
@kevmoo

kevmoo commented Sep 23, 2026

Copy link
Copy Markdown
Contributor Author

@navaronbracke - need another nod from you, please

@navaronbracke navaronbracke added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Sep 23, 2026
Merged via the queue into flutter:master with commit 5048996 Sep 23, 2026
23 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2026
@kevmoo
kevmoo deleted the reland-raw-tooltip-escape branch September 23, 2026 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants