erik/acdream
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

fix(ui): night-round review — F5/F6 structural single-tooltip invariant

Browse source 
F5: moved the unconditional RemovePopup() call into
RetailTooltipPresenter.TryBuildAndMountPopup itself so the single-
popup invariant (retail's own single m_pTooltipElement slot) is
enforced structurally rather than relying on every caller to have
already cleared a stale popup. Closes a real hole: UpdateWorldHoverTooltip's
own clear is gated on _worldTooltipShowing (only true when the WORLD
path itself mounted the current popup), and its "a UI popup cannot be
showing here" comment assumed the host's hover query is null whenever
that branch runs — an assumption that breaks the instant a modal opens
over a stationary cursor. UiRoot.Modal claims EXCLUSIVE hit-testing, so
Pick(MouseX, MouseY) can return null even though a UI-dwell tooltip is
still mounted underneath; UpdateWorldHoverTooltip would then mount a
second popup on top without ever clearing the first.

F6: fixed WorldHover_ThenUiDwellTooltip_ReplacesRatherThanStacks to
actually exercise the transition with a follow-up presenter.Tick()
(the old test only proved OnTooltipShow's own clear worked, never
checked the world-side bookkeeping after). Added
UiDwellTooltip_ThenModalStealsHitTesting_WorldHoverReplacesRatherThanStacks
for F5's own case, using UiRoot.Modal to reproduce the exclusive-hit-
testing hole precisely — empirically verified this new test fails
(2 popups instead of 1) with the structural RemovePopup() reverted,
confirming it is a real regression test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Erik 2026-08-17 04:45:49 +02:00
parent 4a24614fd1
commit ab84b54dfa
2 changed files with 77 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

25
src/AcDream.App/UI/Layout/RetailTooltipPresenter.cs
View file

@ -186,9 +186,34 @@ public sealed class RetailTooltipPresenter : IDisposable
/// family resolves to). Extracted unchanged from the pre-#411-follow-on
/// <c>OnTooltipShow</c> body — same F4/F5/F8 fixes, same failure
/// handling.
///
/// <para>
/// Night-round review F5: the single-popup invariant (retail's own
/// single <c>m_pTooltipElement</c> slot) is now enforced HERE,
/// structurally, rather than relying on every caller to have already
/// cleared a stale popup before reaching this method. Both existing
/// callers already clear on their own early-return paths too (a hover
/// change that resolves to no valid tooltip text must still tear down
/// the PREVIOUS popup, which never reaches this method at all), so
/// those calls stay — this is a belt-and-braces guarantee, not a
/// replacement for them. It closes a real hole: <see cref="UpdateWorldHoverTooltip"/>'s
/// own clear is gated on <c>_worldTooltipShowing</c> (only true when the
/// WORLD path itself mounted the current popup) and its "a UI popup
/// cannot be showing here" comment assumed <see cref="_host"/>'s hover
/// query is null whenever that branch runs — an assumption that does
/// not hold the instant a modal dialog opens over a stationary cursor:
/// the UI dwell popup from <see cref="OnTooltipShow"/> stays mounted
/// (<c>_owner</c>/<c>_popupRoot</c> set, <c>_worldTooltipShowing</c>
/// still false) while the world path could independently find an
/// object and call this method, mounting a second popup on top. Now it
/// cannot: this call clears whatever is mounted, UI-owned or
/// world-owned, before either ever gets a chance to layer.
/// </para>
/// </summary>
private bool TryBuildAndMountPopup(uint rootElementId, uint layoutDid, string tooltipText)
{
RemovePopup();
ImportedLayout? layout;
try
{