Skip to content

fix(webview): keep native pages visible under overlays - #954

Draft
beruro wants to merge 12 commits into
developfrom
junyu/fix-native-webview-occlusion
Draft

fix(webview): keep native pages visible under overlays#954
beruro wants to merge 12 commits into
developfrom
junyu/fix-native-webview-occlusion

Conversation

@beruro

@beruro beruro commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator

Problem

Inline Browser WebViews are native child WKWebViews. They do not participate in DOM stacking contexts, so CSS z-index, portal roots, and overflow: hidden cannot place React UI above or clip them. The previous mitigation was to send the surface to back or hide the whole WebView whenever an overlay opened, which blanked the live page for any modal, dropdown, tooltip, or tour step raised over a browser pane.

The visible symptoms this fixes:

  • 页面教程 modal over inline GitHub: a white strip beside the panel, because the padded modal wrapper — not the opaque panel — was registered as the occluding surface.
  • 布局教程 step 1 over the browser: bright page content bleeding through the tour highlight, because the highlight cut a native hole that exposed the undimmed page.
  • Tab bar + menu over the browser: a white block at (0,0) or an oversized cutout, because the dropdown registered a native hole before it had been positioned.

Solution

Keep the WebView painted and occlude only where React UI actually covers it, on the macOS compositor:

  • Split mask holes from dim holes. Mask holes cut the live WebView where opaque/glass React UI sits; dim holes mark regions that stay undimmed. Separating the two lets modals, spotlight tours, and dropdowns behave like ordinary React components over a live page.
  • Modals register uniform native dim plus a mask hole aligned to .liquid-modal-content, so the wrapper's padding slack no longer reads as opaque and no white strip appears. A named black CALayer above the native page supplies the scrim a DOM element cannot alpha-composite over a sibling WKWebView.
  • useSpotlightTourNativeOcclusion (new) drives layout/code-editor tours and guide highlights: uniform browser dim, popover mask-only, no native highlight holes that would reveal bright page content.
  • Dropdowns gate their native hole until they are positioned and use zero slop, eliminating the (0,0) white block and oversized cutouts. The gate applies only to the portal branch: in-flow panels are placed by Tailwind position classes and never populate dropdownPosition, so gating them on it would have skipped their registration entirely.
  • isActive (page lifecycle) stays separate from isVisible (native surface): opening an overlay never destroys, reloads, or navigates the page, so cookies, login state, history, scroll, and in-page memory survive. Rects are coalesced and capped at 64 on both sides, and surface commands are serialized per WebView so the last requested visibility wins.

Architecture notes live in docs/workstation/native-webview-scale-system--0622.md under Native surface visibility and overlays.

Platform support

The compositor path is macOS-only. set_inline_webview_occlusions is a no-op under #[cfg(not(target_os = "macos"))], and useInlineWebviewOcclusions gates every publish on isMacOS().

Windows and Linux keep the pre-existing offscreen fallback: webviewOverlayBlockedAtom moves the whole surface offscreen while any overlay is registered, so the page blanks rather than staying painted. It is not destroyed — the isActive / isVisible split means cookies, login state, history, scroll, and in-page memory still survive an overlay there. This PR does not regress those platforms; it does not improve them either. Because both paths read the same useOverlayLayer registry, a future WebView2 region-clip or WebKitGTK implementation only has to supply the native layer.

Merge note: develop was merged in twice. The only conflict was src/modules/WorkStation/shared/StatusBar/EditorStatusBar.tsx: develop removed the in-editor LSP dropdown entirely, so this branch's overlay-layer registration for that dropdown had no remaining target and was dropped in favor of develop's version of the file.

Potential risks

  • macOS-only path. The mask/dim layers are implemented against CALayer/CAShapeLayer. Windows and Linux keep the offscreen fallback and are unaffected — but also ungained, so the reported symptoms are only fixed on macOS. The four overlay atoms that previously forced the fallback on every platform (componentIssueModalOpenAtom, quitConfirmationModalOpenAtom, toolbarDropdownOpenAtom, spotlightOpenAtom) now do so only off macOS; a mask-registration gap on macOS would surface as a page painting over that UI rather than as a blank surface.
  • Correctness depends on published geometry. Any overlay that fails to publish its rect, publishes a stale one, or unmounts without unregistering will either leave a hole over live content or leave a region masked after the overlay is gone. Resize/scroll/scale callbacks re-check desired visibility before writing a frame, but overlays added later must opt in.
  • 64-rectangle cap. With many simultaneous overlays the even-odd path is truncated; excess regions will not be cut.
  • Native input handoff. Interactive overlays hand pointer input back to React while open. The fallback is written to fail closed inside the inline WebView; a regression here would either swallow clicks over the page or let them escape the window.
  • IPC surface change. New occlusion commands are registered in src-tauri/src/commands/handler_list.inc; frontend and Rust ship in the same bundle, so there is no version-skew path, but a partial revert of one side would break the other.
  • No data or persistence risk. Nothing here writes to disk or to the database, so rollback is a plain revert of the branch.

Verification

Automated, run on the post-merge tree at 8cd6bd36f:

  • pnpm typecheck (tsc --noEmit --pretty false, whole project) — exit 0, 0 errors
  • pnpm exec vitest run (full suite) — 1236 files, 9796 tests passed, 0 failed
  • cargo check -p browser — clean, no warnings
  • cargo test -p browser — 47 passed, 0 failed
  • pnpm exec eslint --max-warnings 0 on every file this branch touches — clean

The full vitest run caught src/components/Dropdown/index.test.ts > registers a visible controlled menu as a webview-blocking overlay failing against the positioned-gate change. That is fixed in 8cd6bd36f; the suite is green.

Manual checks on macOS, still outstanding:

  • Open inline GitHub, trigger 页面教程 modal — uniform dim, no white strip beside panel, × closes
  • Run 布局教程 step 1 over browser — browser dimmed uniformly, popover readable, no GitHub card bleed-through
  • Open tab bar + menu over browser — no white blank region; menu clickable
  • Regression: dropdown/tooltip over browser still receives clicks; page stays mounted after close
  • Regression: an in-flow dropdown (no getPopupContainer) over the browser is masked, not painted over

Not run: any Windows or Linux check. Those platforms take the unchanged offscreen fallback path.

No screenshots or recordings are attached yet. These are compositor-level visual changes, so the manual checks above are the meaningful evidence and are not substitutable by unit tests.

beruro added 7 commits August 25, 2026 18:02
Pre-commit hook ran. Total eslint: 0, total circular: 0
Pre-commit hook ran. Total eslint: 0, total circular: 0
Pre-commit hook ran. Total eslint: 0, total circular: 0
Pre-commit hook ran. Total eslint: 0, total circular: 0
Pre-commit hook ran. Total eslint: 0, total circular: 0
Pre-commit hook ran. Total eslint: 0, total circular: 0
Pre-commit hook ran. Total eslint: 0, total circular: 0
@beruro
beruro marked this pull request as draft August 25, 2026 10:28
@beruro

beruro commented Aug 25, 2026

Copy link
Copy Markdown
Collaborator Author

Updated the modal solution after visual feedback: full-screen wrappers are no longer treated as opaque holes. The live native page now receives a theme-matched black CALayer scrim, while only the opaque dialog panel is masked out. Marked the PR draft until the revised native composition has screenshot-level manual verification.

@Harry19081 Harry19081 added bug Something isn't working workstation Workstation, editor, source control, LSP, or status bar frontend-ui Frontend UI, design system, accessibility, layout, or theming labels Aug 25, 2026
Separate WebView mask cuts from dim-layer holes so modals, spotlight tours,
and dropdowns behave like normal React components over inline browsers without
bright bleed-through, white padding artifacts, or premature hole registration.
@Harry19081
Harry19081 marked this pull request as ready for review August 25, 2026 16:52
Resolves the EditorStatusBar conflict: develop removed the in-editor LSP
dropdown (archived with the rest of the LSP UI), so the overlay-layer
registration this branch added for that dropdown no longer has a target.
Took develop's version of the file.
`sanitize_occlusion_rects` is called through its `super::` path inside the
macOS module, so the unqualified import was dead and tripped the unused-import
warning.
The occlusion gate added for dropdowns required `dropdownPosition`, but that
state is only ever populated on the portal branch — in-flow panels are placed
by their Tailwind position classes and leave it null for their whole lifetime.
Every in-flow dropdown therefore skipped overlay registration: on macOS it got
no compositor mask, and on Windows and Linux it never tripped the offscreen
fallback, so the native page painted over the menu on both.

Gate on measured coordinates only where they exist.
@Harry19081

Copy link
Copy Markdown
Member

Review notes — parking this for now

First, credit where it is due: this is an excellent piece of work. Correctly identifying that native child WKWebViews do not participate in the DOM stacking context — and that no amount of z-index, portal roots, or overflow: hidden will ever fix it — is the kind of root-cause diagnosis that most people never reach. They reach for another CSS workaround instead. Going from that diagnosis to a compositor-level even-odd CAShapeLayer mask is a real jump in approach, and the mask hole vs. dim hole split is the genuinely clever part: recognizing that "cut the live surface" and "leave this region undimmed" are two different operations, which is exactly why the spotlight tour highlights were bleeding bright page content. Nice find, and nicely reasoned.

That said, I want to park this for a while rather than land it now. This is not a rejection — the approach is right and I want it eventually. It is about the risk profile.

The risk

1. Correctness moves from one boolean to N registration sites. The old offscreen fallback was blunt but structurally safe: activeOverlayCountAtom > 0 and the whole surface hides. Coarse, but it cannot miss. The new model is precise, and precision means every overlay component must register its geometry correctly, forever. Miss one and the native page paints over your UI.

This is not hypothetical. 52f0177b9 gated dropdown registration on visible && Boolean(dropdownPosition), but dropdownPosition is only ever populated on the portal branch — updatePosition() returns early at if (!getPopupContainer) return;. Every in-flow dropdown silently stopped registering: no compositor mask on macOS, and no offscreen fallback on Windows/Linux either, since it never incremented activeOverlayCountAtom. Fixed in 8cd6bd36f, but note it took a full vitest run to surface — a targeted subset missed it.

2. hitTest: swizzling is the sharpest edge here. It is a class-level method swizzle with the original Imp stashed in a static map, deliberately not per-instance because AppKit KVO-observes WKWebView frames. clear_webview_occlusions exists specifically to defend against pointer-address reuse. That is all correct as written, but it is the part most likely to produce a bug that is hard to reproduce and harder to attribute — clicks landing in the wrong webview, or escaping to another application entirely.

3. Silent truncation. sanitize_occlusion_rects uses .take(MAX_OCCLUSION_RECTS). Past 64 rects, holes are dropped with no error and no warning.

4. A real power regression. The old path shrank the surface to 1×1, which effectively stopped the page painting. The new path keeps it fully composited underneath. A modal left open over a page running video or animation now costs materially more GPU/CPU than before. Worth measuring before we commit to it.

5. Platform divergence. This is macOS-only. Windows and Linux keep the offscreen fallback, so every symptom in the description remains on those platforms. It also changes cross-platform behavior asymmetrically: the four overlay atoms (componentIssueModalOpenAtom, quitConfirmationModalOpenAtom, toolbarDropdownOpenAtom, spotlightOpenAtom) previously forced the fallback everywhere and now do so only off macOS. We should decide deliberately whether this is "macOS-only, documented" or the first step toward parity — that decision belongs before the merge, not after.

6. The one bug class that escapes has no test. Existing coverage tests rect math and specific components. Nothing asserts that a given overlay is registered at all — which is precisely how #1 slipped through.

Why park rather than push through

The blast radius spans input routing, the macOS compositor, and cross-platform behavior, and the verification that would actually de-risk it is manual macOS QA that we do not have bandwidth to do properly right now. The remaining manual checks are still unticked and there is no visual evidence attached, which is exactly the evidence that matters for compositor-level changes. Nothing here is urgent enough to justify absorbing that risk on a partial verification pass.

What is already done on the branch

  • develop merged twice, up to 4505e1a1c. The one conflict was EditorStatusBar.tsx, where develop removed the in-editor LSP dropdown entirely — this branch's overlay registration for it had no target left, so develop's version was taken.
  • Dead sanitize_occlusion_rects import removed; cargo check -p browser is warning-free.
  • In-flow dropdown registration gate fixed.
  • Full suite green: tsc clean, vitest 1236 files / 9796 tests, cargo test -p browser 47/47.
  • Description brought in line with PR_RULES.md.

So the branch is in good shape and merges cleanly. It is parked on risk appetite and verification bandwidth, not on code quality.

To pick this back up

  1. A registration-level regression test — assert that every overlay component which can float over the browser actually lands in overlayLayerRegistryAtom.
  2. A decision on Windows/Linux: parity work, or documented as macOS-only.
  3. The manual macOS passes with screenshots, including an in-flow dropdown (no getPopupContainer) over the browser.
  4. A power measurement for a modal held open over an animating page.

Moving to Draft to reflect that. Really good work — this is worth finishing properly rather than rushing.

@Harry19081
Harry19081 marked this pull request as draft August 25, 2026 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working frontend-ui Frontend UI, design system, accessibility, layout, or theming workstation Workstation, editor, source control, LSP, or status bar

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants