fix(dashboard): terminate uncaptured drags when pointer capture acquisition fails - #9098
Open
javenciu wants to merge 1 commit into
Open
fix(dashboard): terminate uncaptured drags when pointer capture acquisition fails#9098javenciu wants to merge 1 commit into
javenciu wants to merge 1 commit into
Conversation
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem / Motivation
usePointerDrag(the shared drag hook behind the dashboard's pane resizers, column splitters, and panel handles) callssetPointerCaptureon pointer-down and swallows any failure:setPointerCapturecan genuinely throw at this call site:NotFoundErrorfor 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, nolostpointercapture(capture never existed, so it cannot be lost). The moment the pointer leaves the handle, the element hears nothing. Apointerupreleased anywhere outside the handle never reaches it, so the drag never ends:activestaystrueand every consumeronStartside effect — body-wideuser-selectsuppression, pinnedbody.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
lostpointercaptureas "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)
setPointerCapturecall threw can never terminate once the pointer leaves the handle;onEndnever fires andonStartside effects are stranded.lostpointercapture) is structurally unreachable when capture acquisition itself failed.captured = falsein the existingtry/catch) and, only in that case, arm window-levelpointerup/pointercancellisteners scoped to that exactpointerId, 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.s.activeguard inendalready collapses an element-then-window double delivery to oneonEnd; the fallback adds no second bookkeeping.pointerup's user-driven coordinates refresh the tracker; a platform-firedpointercancelwith sentinel coordinates ends from the last tracked position.useEffectcleanup), so they never leak across drags or outlive the component. A latest-ref (endRef, same pattern as the hook's existingoptsRef) keeps the window handler stable without re-subscribing.endnow acceptsReact.PointerEvent | PointerEvent(window events are native);releasePointerCaptureis guarded for the no-currentTargetcase — 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/pointercancelretargeted to the element, pluslostpointercapture), 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(newcapture-acquisition failureblock), all proven failing at the unfixed tree first (6 failed / 15 passed), all green after (21/21):pointerupoutside the handle ends the drag: capture throws, pointer moves off-element, window-levelpointerupfires → exactly oneonEndwith the correct delta.pointercancelwith0,0sentinel 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).pointerupfor a different pointerId does not end the drag.onEnd.setPointerCapturesucceeds, no window listeners are added.Full run: targeted suite 21/21; 126 consumer/neighbor test files (1336 tests) green;
tsc0;eslint0; 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
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)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.