Skip to content

Commit 8d44459

Browse files
committed
Never paint a browser snapshot at stale geometry (the modal-open flash)
Opening a modal locks scroll, which removes the window scrollbar and reflows the panel — so a capture taken before the lock describes a rect the panel no longer occupies. The handshake painted that frame anyway and only then retried, so the replacement landed visibly offset from the page it stands in for: the flash. A capture is now checked against the host's live rect before it is painted; a mismatched frame is skipped and re-captured at the settled layout instead (modal retries go 2 -> 3 to absorb the extra settle).
1 parent 1f3977b commit 8d44459

3 files changed

Lines changed: 81 additions & 3 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-panel-occlusion.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
createBrowserPanelGeometryOcclusionLease,
3131
hasNativeSurfaceOcclusion,
3232
NATIVE_SURFACE_OCCLUSION_SELECTOR,
33+
snapshotMatchesHost,
3334
useBrowserPanelOcclusion,
3435
} from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-panel-occlusion'
3536

@@ -582,3 +583,44 @@ describe('useBrowserPanelOcclusion modal lifecycle', () => {
582583
hook.unmount()
583584
})
584585
})
586+
587+
describe('snapshotMatchesHost', () => {
588+
const rect = (x: number, y: number, width: number, height: number) =>
589+
({ x, y, width, height }) as DOMRect
590+
591+
it('accepts a capture that still describes the host rect', () => {
592+
expect(
593+
snapshotMatchesHost(
594+
{ viewportBounds: { x: 10, y: 20, width: 800, height: 600 } },
595+
rect(10, 20, 800, 600)
596+
)
597+
).toBe(true)
598+
})
599+
600+
it('tolerates sub-pixel drift from rounding', () => {
601+
expect(
602+
snapshotMatchesHost(
603+
{ viewportBounds: { x: 10, y: 20, width: 800, height: 600 } },
604+
rect(10.4, 19.6, 800.5, 599.5)
605+
)
606+
).toBe(true)
607+
})
608+
609+
it('rejects a capture taken before a scroll lock reflowed the panel', () => {
610+
// Modal scroll lock removes the scrollbar: the host widens by 15px, so the
611+
// pre-lock capture would paint misaligned — the flash this guards.
612+
expect(
613+
snapshotMatchesHost(
614+
{ viewportBounds: { x: 10, y: 20, width: 800, height: 600 } },
615+
rect(10, 20, 815, 600)
616+
)
617+
).toBe(false)
618+
})
619+
620+
it('accepts captures with no viewport bounds (host-tracking fallback style)', () => {
621+
expect(snapshotMatchesHost({ viewportBounds: undefined }, rect(0, 0, 100, 100))).toBe(true)
622+
expect(
623+
snapshotMatchesHost({ viewportBounds: { x: 0, y: 0, width: 10, height: 10 } }, null)
624+
).toBe(true)
625+
})
626+
})

apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-panel-occlusion.ts

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,10 +184,34 @@ async function decodeSnapshot(dataUrl: string): Promise<boolean> {
184184
* changing layers never reveals or recaptures the native view, and the view is
185185
* revealed only after the final reason disappears.
186186
*/
187+
188+
/** Largest tolerated drift, in CSS px, between a capture and the live host rect. */
189+
const SNAPSHOT_GEOMETRY_TOLERANCE_PX = 1
190+
191+
/**
192+
* Whether a captured frame still describes the rectangle the panel occupies.
193+
* A capture with no viewport bounds is positioned by the fallback
194+
* `absolute inset-0` style, which tracks the host by construction.
195+
*/
196+
export function snapshotMatchesHost(
197+
frame: Pick<BrowserPanelSnapshot, 'viewportBounds'>,
198+
hostRect: DOMRect | null
199+
): boolean {
200+
const bounds = frame.viewportBounds
201+
if (!bounds || !hostRect) return true
202+
return (
203+
Math.abs(bounds.x - hostRect.x) <= SNAPSHOT_GEOMETRY_TOLERANCE_PX &&
204+
Math.abs(bounds.y - hostRect.y) <= SNAPSHOT_GEOMETRY_TOLERANCE_PX &&
205+
Math.abs(bounds.width - hostRect.width) <= SNAPSHOT_GEOMETRY_TOLERANCE_PX &&
206+
Math.abs(bounds.height - hostRect.height) <= SNAPSHOT_GEOMETRY_TOLERANCE_PX
207+
)
208+
}
209+
187210
export function useBrowserPanelOcclusion(
188211
scopeId: string,
189212
activeTabId: string | null,
190-
panelVisible = true
213+
panelVisible = true,
214+
getHostRect?: () => DOMRect | null
191215
): BrowserPanelOcclusion {
192216
const [snapshotRender, setSnapshotRender] = useState<SnapshotRender | null>(null)
193217
const [activeOverlay, setActiveOverlay] = useState<BrowserPanelOverlay | null>(null)
@@ -204,8 +228,10 @@ export function useBrowserPanelOcclusion(
204228
const paintFramesRef = useRef<number[]>([])
205229
const reconcileChainRef = useRef<Promise<boolean>>(Promise.resolve(true))
206230
const mountedRef = useRef(true)
231+
const getHostRectRef = useRef(getHostRect)
207232
activeTabIdRef.current = activeTabId
208233
panelVisibleRef.current = panelVisible
234+
getHostRectRef.current = getHostRect
209235

210236
const updateSnapshotRender = useCallback((render: SnapshotRender | null) => {
211237
snapshotRenderRef.current = render
@@ -303,7 +329,7 @@ export function useBrowserPanelOcclusion(
303329

304330
// Modal scroll locking can alter panel geometry between capture and the
305331
// final native hide. One fresh capture retries that now-settled layout.
306-
const maxAttempts = desired === 'modal' ? 2 : 1
332+
const maxAttempts = desired === 'modal' ? 3 : 1
307333
for (let attempt = 0; attempt < maxAttempts; attempt++) {
308334
const frame = await captureBrowserPanelSnapshot(scopeId).catch(() => null)
309335
if (!mountedRef.current || version !== transitionVersionRef.current) return false
@@ -318,6 +344,13 @@ export function useBrowserPanelOcclusion(
318344
desired = desiredLayer()
319345
if (!desired || !decoded) continue
320346

347+
// Painting a capture whose geometry no longer matches the host is the
348+
// flash: a modal's scroll lock changes the window's content width
349+
// between capture and paint, so the replacement lands offset from the
350+
// page it is standing in for. Skip that frame and re-capture at the
351+
// settled layout instead of showing a misaligned one.
352+
if (!snapshotMatchesHost(frame, getHostRectRef.current?.() ?? null)) continue
353+
321354
const paintId = ++paintIdRef.current
322355
const painted = new Promise<boolean>((resolve) => {
323356
pendingPaintRef.current = { paintId, resolve }

apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-session.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,9 @@ export function BrowserSession({
372372
const suspended = useBrowserSessionStore((state) => state.sessions[scopeId]?.suspended ?? false)
373373
const panelRef = useRef<HTMLDivElement>(null)
374374
const hostRef = useRef<HTMLDivElement>(null)
375+
// Lets the occlusion handshake reject a capture taken before a modal's
376+
// scroll lock reflowed the panel — painting that stale rect is the flash.
377+
const getHostRect = useCallback(() => hostRef.current?.getBoundingClientRect() ?? null, [])
375378
const urlInputRef = useRef<HTMLInputElement>(null)
376379
const findInputRef = useRef<HTMLInputElement>(null)
377380
const fillButtonRef = useRef<HTMLButtonElement>(null)
@@ -450,7 +453,7 @@ export function BrowserSession({
450453
requestOverlay,
451454
closeOverlay,
452455
onSnapshotError,
453-
} = useBrowserPanelOcclusion(scopeId, activeTabId, panelVisible)
456+
} = useBrowserPanelOcclusion(scopeId, activeTabId, panelVisible, getHostRect)
454457

455458
// The resource picker lives above this component in the panel tab bar. Give
456459
// that one external browser overlay access to the same capture/hide handshake

0 commit comments

Comments
 (0)