Skip to content

fix(scroll): guard against re-locking iOS Safari scroll while locked#642

Closed
blvdmitry wants to merge 1 commit into
canaryfrom
claude/fix-locksafari-nested-guard
Closed

fix(scroll): guard against re-locking iOS Safari scroll while locked#642
blvdmitry wants to merge 1 commit into
canaryfrom
claude/fix-locksafari-nested-guard

Conversation

@blvdmitry

Copy link
Copy Markdown
Contributor

Problem

On iOS the document scroll lock is applied to document.body (position: fixed), but the "already locked" guard in lockScroll checks document.documentElement.style.overflow. So on iOS a nested lock (e.g. a modal opening a dropdown) is not recognised as already-locked and re-enters lockSafariScroll. At that point:

  • window.scrollX/scrollY read 0 (the body is already fixed),
  • styleCache.set(document.body, …) overwrites the cached original top/left/position with the already-fixed values.

When the outer lock is released it restores the body to position: fixed, leaving the page permanently unscrollable until reload.

Fix

Bail out with a no-op unlock when the body is already fixed, so the second lock doesn't re-capture the zeroed scroll position or clobber the cache:

 const lockSafariScroll = () => {
+	// Already locked (e.g. a nested overlay): the body is fixed and window.scrollX/Y
+	// now read 0, so re-capturing here would overwrite the saved originals with the
+	// already-fixed values and lose the real scroll position. Return a no-op unlock.
+	if (document.body.style.position === "fixed") {
+		return () => {};
+	}
+
 	const viewport = window.visualViewport;

Verification

  • tsc --noEmit (utilities) → 0 errors. oxlint → clean.

How to test

  1. On iOS Safari, open a Modal (locks the document), then open a nested overlay from inside it, and close them.
  2. Before: the page is stuck (body remains position: fixed) and scroll position is lost. After: scroll is restored correctly to the original position.

Found during a full package bug review. Complementary to the lock ref-counting PR (which prevents the double-entry at the lockScroll level); this adds defense-in-depth inside lockSafariScroll itself.


Generated by Claude Code

@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Library / JS 48.49 KB (-0.05% 🔽) 970 ms (-0.05% 🔽) 1.5 s (+17.64% 🔺) 2.5 s
Library / CSS 22.18 KB (0%) 444 ms (0%) 0 ms (+100% 🔺) 444 ms
Theming / JS 7.81 KB (0%) 157 ms (0%) 409 ms (+140.17% 🔺) 565 ms
Theming with a default theme definition / JS 8.65 KB (0%) 174 ms (0%) 300 ms (+36.73% 🔺) 473 ms

On iOS the document lock is applied to document.body (position: fixed).
The 'already locked' check in lockScroll inspects documentElement, so a
nested lock re-entered lockSafariScroll, where window.scrollX/Y now read 0
because the body is already fixed. That overwrote the cached original
top/left/position with the fixed values, so releasing restored the body to
position: fixed permanently (page stuck). Bail out with a no-op unlock when
the body is already fixed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01We9NqptZjfbvXRL4hE1xri
@blvdmitry blvdmitry force-pushed the claude/fix-locksafari-nested-guard branch from 27c3491 to b3994ec Compare June 20, 2026 09:24

Copy link
Copy Markdown
Contributor Author

Closing this as redundant after the rest of the scroll-lock cluster.

The critical symptom this PR targeted — a nested iOS document lock overwriting the cached body originals with the already-fixed values, leaving the page permanently unscrollable — is now prevented by #641. StyleCache.set() is a no-op when the element is already cached, so the second lockSafariScroll call can no longer clobber the captured originals.

The remaining concern (a second document lock re-entering lockSafariScroll at all) is properly handled by #640's reference-counting: it ref-counts on the container (document.documentElement for the document lock), so on iOS the second lock just increments the count and never calls lockSafariScroll again. That makes the document.body.style.position === "fixed" guard here dead code.

That guard is also fragile — it would misfire if anything else set the body to position: fixed. Between #641 (idempotent set) and #640 (ref-counting), it's no longer needed.

Superseded by #640 + #641.


Generated by Claude Code

@blvdmitry blvdmitry closed this Jun 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants