Skip to content

fix(dashboard): terminate uncaptured drags when pointer capture acquisition fails - #9098

Open
javenciu wants to merge 1 commit into
kirodotdev:mainfrom
javenciu:fix/pointer-capture-failure-fallback
Open

fix(dashboard): terminate uncaptured drags when pointer capture acquisition fails#9098
javenciu wants to merge 1 commit into
kirodotdev:mainfrom
javenciu:fix/pointer-capture-failure-fallback

Conversation

@javenciu

@javenciu javenciu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

usePointerDrag (the shared drag hook behind the dashboard's pane resizers, column splitters, and panel handles) calls setPointerCapture on pointer-down and swallows any failure:

try { el.setPointerCapture(e.pointerId) } catch { /* capture is best-effort */ }

setPointerCapture can genuinely throw at this call site: NotFoundError for a pointerId that is no longer active, or the element being disconnected at call time. Swallowing is the right liveness call — the drag should still start — but the resulting drag is uncaptured: it gets no event retargeting and, critically, no lostpointercapture (capture never existed, so it cannot be lost). The moment the pointer leaves the handle, the element hears nothing. A pointerup released anywhere outside the handle never reaches it, so the drag never ends: active stays true and every consumer onStart side effect — body-wide user-select suppression, pinned body.cursor, "dragging" flags — is stranded while the component stays mounted, with no terminal event left that can heal it.

The hook's own safety-net comment names lostpointercapture as "the terminal event … when capture ends for ANY reason" — that net has a hole on the acquisition side: a capture that never existed cannot end.

Why it matters

Every resizer in the app funnels through this hook. A stranded drag leaves the whole dashboard with text selection disabled and a resize cursor pinned body-wide until the user happens to press-and-release on the same handle again. The failure is invisible in the code path (the catch is silent) and unrecoverable by any event the element will ever receive — it is exactly the class of stuck-interaction bug this hook's capture-loss handler (onLostPointerCapture) was built to prevent, on the twin path it does not cover.

What changed (motivation → approach → change)

  • Symptom: a drag whose setPointerCapture call threw can never terminate once the pointer leaves the handle; onEnd never fires and onStart side effects are stranded.
  • Root cause: the element is the only event target the hook listens on, and an uncaptured pointer stops delivering to that element outside its bounds. The terminal-event safety net (lostpointercapture) is structurally unreachable when capture acquisition itself failed.
  • Change: detect acquisition failure at the call site (captured = false in the existing try/catch) and, only in that case, arm window-level pointerup/pointercancel listeners scoped to that exact pointerId, routing into the same single-fire end path. The window is the one target guaranteed to still hear the terminal event for an uncaptured pointer.
    • Single-fire: the existing s.active guard in end already collapses an element-then-window double delivery to one onEnd; the fallback adds no second bookkeeping.
    • Coordinate policy composes: the end path's coordinate allow-list (introduced in fix(dashboard): stop trusting pointercancel end coordinates in drag hook #9018) governs the fallback identically — a window pointerup's user-driven coordinates refresh the tracker; a platform-fired pointercancel with sentinel coordinates ends from the last tracked position.
    • Lifecycle: the listeners are disarmed on any end (element or window path) and on unmount (useEffect cleanup), so they never leak across drags or outlive the component. A latest-ref (endRef, same pattern as the hook's existing optsRef) keeps the window handler stable without re-subscribing.
    • end now accepts React.PointerEvent | PointerEvent (window events are native); releasePointerCapture is guarded for the no-currentTarget case — on the fallback path there is no capture to release by definition.

Alternative considered: always arming window listeners on every pointer-down. Rejected — the captured path already has a complete terminal-event story (pointerup/pointercancel retargeted to the element, plus lostpointercapture), and unconditional window listeners would add churn to every drag to serve only the failure path. Arming exactly when capture failed keeps the fallback proportional to the defect.

Tests

Six new tests in usePointerDrag.test.tsx (new capture-acquisition failure block), all proven failing at the unfixed tree first (6 failed / 15 passed), all green after (21/21):

  • Attack — uncaptured pointerup outside the handle ends the drag: capture throws, pointer moves off-element, window-level pointerup fires → exactly one onEnd with the correct delta.
  • Attack — uncaptured platform pointercancel with 0,0 sentinel does not corrupt the delta: the end payload derives from the last tracked position (composition with fix(dashboard): stop trusting pointercancel end coordinates in drag hook #9018's allow-list, proven on the fallback path).
  • Control — pointerId scoping: a window pointerup for a different pointerId does not end the drag.
  • Control — single-fire: element-path end followed by a late window event yields exactly one onEnd.
  • Benign — captured path arms nothing: when setPointerCapture succeeds, no window listeners are added.
  • Benign — unmount mid-uncaptured-drag removes the window fallback listeners (no leak past component lifetime).

Full run: targeted suite 21/21; 126 consumer/neighbor test files (1336 tests) green; tsc 0; eslint 0; theme-colors / phantom-classes / i18n-keys custom lints all pass; full website suite green.

Manual verification

N/A — unit coverage sufficient: the defect and fix are entirely in event-listener wiring and are pinned by the attack/control/benign matrix above; there is no rendered output to inspect.

Why no screenshot: logic-only change inside a hook; zero rendered-pixel delta on any surface.

Related Issues

Follow-up to the drag-hook hardening line: #8904 (drag termination), #8958 (lostpointercapture tracker), #9018 (pointercancel sentinel coordinates — merged; its design review explicitly scoped the capture-acquisition window out as future work, which this PR closes). Fourth PR on this seam; each addressed a distinct terminal-event gap and this one closes the last known one (acquisition failure).

Pattern harvest

Rule candidate: review-prompt
Pattern: "best-effort acquisition of an event-delivery guarantee (pointer capture, focus, subscription) must pair the failure arm with an alternate delivery path for the terminal event — a swallowed acquisition failure silently converts a guaranteed-terminating interaction into an unterminatable one."

Checklist

  • At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
  • Existing tests pass and new tests added for new functionality
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable) — hook header doc updated in-diff
  • No secrets, credentials, or internal references in the diff

Contribution License Agreement

By submitting this pull request, I confirm that my contribution is made under the terms of the project's contribution license terms as designated by the repository owners, and I have the right to submit this work.

…sition fails

setPointerCapture can throw on pointer-down (NotFoundError for an
already-inactive pointerId, or the element disconnected at call time).
The drag hook swallows the failure -- the right liveness call, the drag
should still start -- but an uncaptured drag gets no event retargeting
and no lostpointercapture, because capture never existed. The moment the
pointer leaves the handle, the element hears nothing again: a pointerup
released anywhere outside never reaches it, the drag never ends, and
every consumer onStart side effect (body-wide user-select suppression,
pinned body.cursor, dragging flags) is stranded while the component
stays mounted. This is the acquisition-side twin of the capture-LOSS
class the onLostPointerCapture handler heals.

Fix: detect acquisition failure at the call site and arm window-level
pointerup/pointercancel listeners scoped to that pointerId, routing into
the same single-fire end path (the s.active guard keeps an
element-then-window double delivery to one onEnd). The listeners are
disarmed on any end and on unmount. The end path's coordinate allow-list
governs the fallback identically: a window pointerup's user-driven
coordinates refresh the tracker; a platform-fired pointercancel with
sentinel coordinates ends from the last tracked position.

Tests: two attack pins (uncaptured pointerup outside the handle must
end the drag; uncaptured platform cancel with 0,0 sentinel must not
corrupt the end delta), pointerId-scoping and single-fire controls, and
benign pins proving the captured path arms nothing, fallback listeners
do not leak across drags, and unmount disarms them.
@javenciu
javenciu requested a review from a team September 6, 2026 19:20
@javenciu
javenciu requested a review from a team as a code owner September 6, 2026 19:20
@javenciu
javenciu requested a review from smeyffret September 6, 2026 19:20
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention labels Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant