fix(ui): #409 tooltip review fix round — F1-F11
Opus review of a377b9bf returned architectural PASS-with-findings /
retail-fidelity FAIL with F1-F12 (F12 info-only). All eleven fixed,
each re-derived against docs/research/named-retail/acclient_2013_pseudo_c.txt:
- F1 PositionAtMouse: retail offsets BOTH axes +32px before the clamp
(StartTooltip @0x00459700, @0x00459739/@0x00459747) — was landing
flush at the cursor.
- F2 UiRoot: the dwell timer now anchors to mouse-IDLE like retail's
m_lastMouseMoveTime (MouseMoveHandler @0x0045E710), resetting on
every move within the same widget while !_tooltipFired, not just on
hover-enter.
- F3 register TS-85 rewritten: the "dynamic InqProperty(0x49) override"
framing was false — UIElement::InqProperty @0x004638D0's base impl
reads the same authored bags this port already reads. The real
second text source (m_TTText/SetTooltip, headed by the P0xD0
truncated-text auto-tooltip @0x00466F80) needs a per-line-position
truncation model UiText doesn't have — sized disproportionate for
this round and left honestly deferred rather than stubbed.
- F4 OnTooltipShow: null LayoutPolicy + Anchors=None on the popup root
and text child before resizing, mirroring RetailMessageDialogView's
sibling shape.
- F5 OnTooltipShow: return without mounting when the P0x4A text child
doesn't resolve to a UiText (retail's DynamicCast gate,
StartTooltip @0x0045DE90 @0x0045df65/@0x0045df6f) — was mounting an
empty 30x30 bevel artifact.
- F6 UiRoot.Tick: the dwell-arm branch now requires Captured is null
(CheckTooltip @0x0045B6E0 @0x0045b715) — a widget hovered before a
drag/resize/capture began must not pop mid-gesture.
- F7 UiRoot.ReleaseCapture: no longer resets _tooltipFired
(ReleaseMouseCapture @0x0045D2B0 touches only the idle timestamp) —
a mouse-up while a tooltip is shown no longer tears it down and
silently re-fires it 250ms later.
- F8 ApplyTooltipText: applies ResizeTo's own max/min width/height
clamps (P0x3C/0x3D/0x3E/0x3F, @0x00463C30) before assigning the
grown size; zeroes text.Padding to keep the measured size margin-
comparable. New ElementInfo/UiElement plumbing for the four
properties, same shape as the existing tooltip fields.
- F9 doc precision: sweep counts corrected 434->430 / 191->187 (live-
DAT re-measured), the "243 showable" claim now measured exactly
(not assumed) via a new Showable column in the sweep test, and the
MiscSettings citation split into its two real mechanisms
(RegisterPreference in Init vs. AttachPreference/SetPreferenceRange
elsewhere).
- F10 register AD-106: the topmost guarantee is versus dialogs/screens
only (the overlay popup layer and drag ghost still paint above
regardless), and the per-tick BringToFront ratchet has four rungs,
not three.
- F11 RetailUiRuntime.ResetSessionDialogs: now also calls the new
UiRoot.ResetTooltipTracking() so a post-reset hover re-shows
immediately instead of waiting out the stale fired-latch.
New pinning tests (RetailTooltipPresenterTests: F1/F2/F5/F6/F7/F8) each
verified to fail against the pre-fix behavior via a temporary revert-
and-rerun before being confirmed against the restored fix.
PortalProjectionTests.ProjectToClipLease_ReusesPooledWorkWithoutResultArrays
recurrence logged on issue #346 (already the tracking issue for this
load-sensitive flake) — hit twice under load this review, standalone
26/26, unrelated to #409.
Gates: Release build 0 errors; App suite (live-DAT env) 5416/5413
passed/3 skips (was 5410/5407/3, +6 new tests); Runtime 1735/0;
UI.Abstractions 926/0; full solution (no env, 69 skips expected)
14,623/14,554 passed/69 skipped/0 failed (was 14,617/14,548, +6).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
parent
a377b9bff7
commit
2719782dc0
11 changed files with 515 additions and 58 deletions
|
|
@ -523,7 +523,11 @@ public sealed class UiRoot : UiElement
|
|||
|
||||
// Tooltip timer: once mouse has hovered over the same widget for its
|
||||
// effective dwell delay, fire a Tooltip event on it exactly once.
|
||||
if (_hoverWidget is not null && !_tooltipFired
|
||||
// #409 F6: retail's arm branch is gated `m_pElementWithMouseCapture
|
||||
// == 0` (UIElementManager::CheckTooltip @0x0045B6E0, @0x0045b715) —
|
||||
// a widget hovered before a drag/resize/scrollbar-thumb capture
|
||||
// began must not pop a tooltip mid-gesture.
|
||||
if (_hoverWidget is not null && !_tooltipFired && Captured is null
|
||||
&& _nowMs - _hoverStartedMs >= EffectiveTooltipDelayMs(_hoverWidget))
|
||||
{
|
||||
var e = new UiEvent(_hoverWidget.EventId, _hoverWidget, UiEventType.Tooltip);
|
||||
|
|
@ -1112,9 +1116,14 @@ public sealed class UiRoot : UiElement
|
|||
{
|
||||
UiElement? previous = Captured;
|
||||
Captured = null;
|
||||
// Retail restarts the tooltip idle deadline when capture is released.
|
||||
// #409 F7: retail restarts the tooltip idle deadline when capture is
|
||||
// released (ReleaseMouseCapture @0x0045D2B0, @0x0045d2ce/@0x0045d2da
|
||||
// — m_lastMouseMoveTime only). It does NOT touch m_bHoverStarted
|
||||
// (our _tooltipFired): that field means "hover started" (a tooltip
|
||||
// is showing), not "capture is active", and a mouse-up while a
|
||||
// tooltip is already up must leave it up, not clear-then-re-fire it
|
||||
// 250ms later without ever going through TooltipHide.
|
||||
_hoverStartedMs = _nowMs;
|
||||
_tooltipFired = false;
|
||||
NotifyCaptureLost(previous);
|
||||
if (previous is not null)
|
||||
PointerCaptureChanged?.Invoke(previous, null);
|
||||
|
|
@ -1271,6 +1280,28 @@ public sealed class UiRoot : UiElement
|
|||
|
||||
// ── Hover / tooltip ─────────────────────────────────────────────────
|
||||
|
||||
/// <summary>
|
||||
/// #409 F11: forgets the in-progress hover-dwell/tooltip-shown latch
|
||||
/// without touching <see cref="_hoverWidget"/> itself or firing any
|
||||
/// hover-leave event. <see cref="RetailUiRuntime.ResetSessionDialogs"/>
|
||||
/// calls this alongside <see cref="RetailTooltipPresenter.HideCurrent"/>
|
||||
/// (which only tears down the presenter's own popup element) so a
|
||||
/// reconnect that hides an in-flight tooltip also re-arms the dwell
|
||||
/// timer — otherwise a mouse that never left the hovered widget across
|
||||
/// the reset would leave <c>_tooltipFired</c> latched true and the
|
||||
/// widget would not show a tooltip again until either the 10 s
|
||||
/// auto-hide timeout elapses or the hover target changes. No literal
|
||||
/// retail counterpart (retail's own session teardown is a full
|
||||
/// <c>UIElementManager</c> re-construction, not a partial reset), but
|
||||
/// the effect matches: a fresh dwell deadline, same as
|
||||
/// <see cref="ReleaseCapture"/>'s own idle-timestamp restart.
|
||||
/// </summary>
|
||||
public void ResetTooltipTracking()
|
||||
{
|
||||
_hoverStartedMs = _nowMs;
|
||||
_tooltipFired = false;
|
||||
}
|
||||
|
||||
private void UpdateHover(int x, int y)
|
||||
{
|
||||
// An open popup claims hover first (#374) — its item highlight must
|
||||
|
|
@ -1282,6 +1313,19 @@ public sealed class UiRoot : UiElement
|
|||
{
|
||||
if (w?.ReceivesHoverMouseMove == true)
|
||||
DispatchMouseMove(w, x, y);
|
||||
// #409 F2: retail's dwell timer anchors to mouse-IDLE, not
|
||||
// hover-enter — UIElementManager::MouseMoveHandler @0x0045E710
|
||||
// stamps m_lastMouseMoveTime on EVERY move (@0x0045e729/
|
||||
// @0x0045e736), unconditionally, before any hit-testing; the
|
||||
// arm check in CheckTooltip @0x0045B6E0 (@0x0045b747) compares
|
||||
// against that timestamp. Jiggling the mouse within the SAME
|
||||
// widget must keep re-arming the deadline, not just entering
|
||||
// it once. Guarded by !_tooltipFired — retail's m_bHoverStarted
|
||||
// (our _tooltipFired) keeps a SHOWN tooltip from being
|
||||
// re-armed by further moves; ReleaseCapture already ports the
|
||||
// same field's other writer (@0x0045D2B0).
|
||||
if (!_tooltipFired)
|
||||
_hoverStartedMs = _nowMs;
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue