Fix number slider value getting stuck tied to mouse position (#4231)

[StefanoD]

Generated by Claude. Review this carefully!

Fixes #4231

Problem

In Increment mode, dragging a number field uses the asynchronous Pointer Lock API.
requestPointerLock() only takes effect once the "pointerlockchange" event fires, and the
drag's cleanup (removing the window pointermove/pointerup listeners) was driven exclusively
by that event via exitPointerLock().

When the interaction ends before the lock engages — e.g. a rapid click or a quick
drag-and-release, which is what the reporter triggered by double-clicking — pointerUp's
document.exitPointerLock() is a no-op, so "pointerlockchange" never fires and the listeners
are never removed. The leaked pointermove listener then keeps updating the value on every mouse
move. Because an unpressed move reports e.buttons === 0 with e.button === -1, it also slips
past the existing exit guard (kept for a Firefox/Wayland quirk), so the value stays permanently
tied to the cursor.

Fix

• Route every drag-exit path (pointerUp, right-click abort, buttons-released recovery) through a
  single endDrag() helper that cleans up directly when the pointer isn't actually locked
  (covers "lock hasn't engaged yet" and "pointer lock not in use"), instead of relying on
  "pointerlockchange".
• Handle the requestPointerLock() promise so a lock that engages after the drag already
  ended exits itself immediately (no stuck-locked cursor), and a rejected request (browser re-lock
  cooldown) doesn't surface as an unhandled rejection.

Single file changed: frontend/src/components/widgets/inputs/NumberInput.svelte.

How I tested

There's no frontend test harness in the repo (CI is svelte-check + eslint), and the bug is a
timing-dependent race that's hard to hit by hand, so I verified it by deterministically forcing
the race in the running editor.

Static checks

• svelte-check passes (type-correct).
• eslint on the changed file: 0 errors.

Deterministic before/after in the running dev build (Chrome/Chromium, usePointerLock === true)

I temporarily stubbed requestPointerLock to return a promise that resolves after release
(exactly the slow-lock condition behind the bug), dispatched pointerdown → pointermove → pointerup on a real Increment number field, then moved the mouse with no button pressed:

Code	Value before release	Value after no-button mouse move	Result
Before fix	4	0	❌ value tied to cursor
After fix	100%	100%	✅ value unchanged

A value change on a no-button move is only possible while the leaked pointermove listener is
still attached, so the "after" result (no change) confirms the listener is properly removed. The
editor also builds and runs normally with the change.

[gemini-code-assist]

Code Review

This pull request resolves a race condition in NumberInput.svelte where a pointer lock might engage after the user has already released the mouse button, potentially leaving the cursor stuck. It introduces a dragReleased flag and a consolidated endDrag cleanup function to handle asynchronous pointer lock requests. The review feedback points out that checking document.pointerLockElement globally is too broad and could release locks belonging to other elements; it is recommended to explicitly verify that document.pointerLockElement === target before calling document.exitPointerLock().

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

Checking document.pointerLockElement is too broad here. If another element or widget has requested and acquired pointer lock in the meantime, calling document.exitPointerLock() would incorrectly release their lock instead of ours. We should explicitly check if document.pointerLockElement === target to ensure we only exit the lock if it belongs to this specific input element.

			if (lockRequest instanceof Promise) {
				lockRequest.then(
					() => {
						// If the user already released before the lock engaged, exit it immediately so we aren't stuck locked.
						if (dragReleased && document.pointerLockElement === target) document.exitPointerLock();
					},

[gemini-code-assist]

Similarly, we should only call document.exitPointerLock() if the active pointer lock belongs to our target. If it belongs to another element, we should bypass exiting it and directly call pointerLockChange() to clean up our own listeners immediately.

		const endDrag = () => {
			dragReleased = true;

			if (usePointerLock && document.pointerLockElement === target) document.exitPointerLock();
			else pointerLockChange();
		};

[cubic-dev-ai]

2 issues found across 1 file

Confidence score: 3/5

• In frontend/src/components/widgets/inputs/NumberInput.svelte, the pointer-lock cleanup logic checks document.pointerLockElement too broadly and may call document.exitPointerLock() even when a different element owns the lock, which can interrupt unrelated pointer-locked interactions and cause confusing input behavior — only exit when document.pointerLockElement === target, otherwise continue with the normal pointer-lock request path before merging.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="frontend/src/components/widgets/inputs/NumberInput.svelte">

<violation number="1" location="frontend/src/components/widgets/inputs/NumberInput.svelte:439">
P2: Checking `document.pointerLockElement` without comparing to `target` is too broad. If another element has acquired pointer lock between the request and this resolution, calling `document.exitPointerLock()` here would incorrectly release their lock. Use `document.pointerLockElement === target` to ensure we only exit our own lock.</violation>

<violation number="2" location="frontend/src/components/widgets/inputs/NumberInput.svelte:530">
P2: Same issue here: `document.pointerLockElement` should be compared to `target` to avoid releasing another element's pointer lock. If the lock belongs to a different element, we should fall through to calling `pointerLockChange()` directly to clean up our own listeners.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Same issue here: document.pointerLockElement should be compared to target to avoid releasing another element's pointer lock. If the lock belongs to a different element, we should fall through to calling pointerLockChange() directly to clean up our own listeners.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/widgets/inputs/NumberInput.svelte, line 530:

<comment>Same issue here: `document.pointerLockElement` should be compared to `target` to avoid releasing another element's pointer lock. If the lock belongs to a different element, we should fall through to calling `pointerLockChange()` directly to clean up our own listeners.</comment>

<file context>
@@ -497,6 +519,17 @@
+		const endDrag = () => {
+			dragReleased = true;
+
+			if (usePointerLock && document.pointerLockElement) document.exitPointerLock();
+			else pointerLockChange();
+		};
</file context>

[cubic-dev-ai]

P2: Checking document.pointerLockElement without comparing to target is too broad. If another element has acquired pointer lock between the request and this resolution, calling document.exitPointerLock() here would incorrectly release their lock. Use document.pointerLockElement === target to ensure we only exit our own lock.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/components/widgets/inputs/NumberInput.svelte, line 439:

<comment>Checking `document.pointerLockElement` without comparing to `target` is too broad. If another element has acquired pointer lock between the request and this resolution, calling `document.exitPointerLock()` here would incorrectly release their lock. Use `document.pointerLockElement === target` to ensure we only exit our own lock.</comment>

<file context>
@@ -418,8 +418,33 @@
+				lockRequest.then(
+					() => {
+						// If the user already released before the lock engaged, exit it immediately so we aren't stuck locked.
+						if (dragReleased && document.pointerLockElement) document.exitPointerLock();
+					},
+					() => {
</file context>

Suggested change

	if (dragReleased && document.pointerLockElement) document.exitPointerLock();
	if (dragReleased && document.pointerLockElement === target) document.exitPointerLock();

[timon-schelling]

https://graphite.art/volunteer/guide/starting-a-task/ai-contribution-policy/