Skip to content

Fix browser pane reloads on tab switch - #1228

Merged
austinywang merged 1 commit into
mainfrom
issue-1225-browser-tab-switch-reload
Mar 12, 2026
Merged

austinywang merged 1 commit into
mainfrom
issue-1225-browser-tab-switch-reload

Conversation

@austinywang

@austinywang austinywang commented Mar 12, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • avoid calling the browser portal hide lifecycle for ordinary tab/workspace visibility hides
  • only re-run WebKit rendering state reattach when a prior hide actually invoked the detach selectors
  • keep the portal slot hidden on tab switches without firing WebKit visibilitychange reload triggers

Closes #1225.

Testing

  • ./scripts/reload.sh --tag fix-1225-tabs
  • Not run: automated tests (repo policy)

Summary by cubic

Stops unexpected page reloads when switching tabs/workspaces by skipping full WebKit hide/show lifecycles on visibility-only changes. Fixes #1225.

  • Bug Fixes
    • Only call the portal hide lifecycle when the pane actually leaves the window/render tree.
    • Add a browserPortalNeedsRenderingStateReattach flag to reattach rendering state only if it was previously detached.
    • Keep the portal slot hidden on tab switches without firing visibilitychange and reload triggers.

Written for commit c52bd24. Summary will update on new commits.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved handling of portal visibility and rendering state management.
    • Reduced unnecessary reloading operations when portals are hidden or removed from view.

@vercel

vercel Bot commented Mar 12, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Mar 12, 2026 5:53am

@coderabbitai

coderabbitai Bot commented Mar 12, 2026 •

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Introduces per-WKWebView rendering state reattach tracking via an associated object key and computed property. On hidden notification, marks the flag as needed. Adds a guard in rendering state reattachment to no-op unless the flag is set, and restricts hide notifications to trigger only when the portal is actually visible.

Changes

Cohort / File(s) Summary
WebView Reattach Tracking
Sources/BrowserWindowPortal.swift
Adds per-WebView rendering state reattach flag via associated object tracking. Marks flag on hidden notification and implements guard check in browserPortalReattachRenderingState() to skip reattachment unless previously marked as hidden. Restricts hide-notification path to only execute when portal is visible in UI and WebView remains in container, preventing unnecessary reloads when portal is effectively hidden.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 A tab that switches without a care,
No more reload in the morning air!
The flag now marks when hidden it goes,
And reattach waits—patience it knows. 🎬✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: fixing browser pane reloads when switching tabs, which matches the core objective of the pull request.
Description check ✅ Passed The description covers the main objectives and includes testing details, though the Testing section lacks information about what was manually verified beyond running the script.
Linked Issues check ✅ Passed The code changes directly address the requirements from issue #1225 by avoiding the detach/reattach cycle, adding rendering state reattach tracking, and restricting hide notifications to actual visibility changes.
Out of Scope Changes check ✅ Passed All changes are focused on the portal visibility tracking and reattach logic needed to fix the tab switch reload issue, with no out-of-scope modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch issue-1225-browser-tab-switch-reload

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 1 file

@greptile-apps

greptile-apps Bot commented Mar 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes browser pane reloads that occurred on tab/workspace switches by preventing unnecessary WebKit lifecycle notifications (_exitInWindow / _enterInWindow) from firing during ordinary UI visibility changes that don't actually alter the WebView's position in the render tree.

Key changes in Sources/BrowserWindowPortal.swift:

  • Adds a browserPortalNeedsRenderingStateReattach associated-object flag on WKWebView to track whether a prior hide actually called the WebKit detach selectors.
  • browserPortalNotifyHidden now sets this flag to true on entry, ensuring the matching reattach pass in browserPortalReattachRenderingState only runs when a real hide occurred.
  • browserPortalReattachRenderingState gains an early-exit guard on the flag, and resets it to false after the reattach selectors fire — preventing spurious _enterInWindow calls (and the visibilitychange event they trigger) on tab-switch-back.
  • hideContainerView adds the entry.visibleInUI pre-condition before calling browserPortalNotifyHidden, so tab/workspace visibility hides (which set visibleInUI = false) skip the WebKit lifecycle entirely and only set containerView.isHidden = true. Geometry-driven and window-detach-driven hides (where visibleInUI remains true) continue to fire the full lifecycle as before.

Confidence Score: 4/5

  • This PR is safe to merge; the logic correctly separates logical visibility hides from physical render-tree detaches with one minor imprecision in flag-set ordering.
  • The fix is well-reasoned and correctly implements the stated invariant: tab-switch hides (entry.visibleInUI=false) now skip the WebKit lifecycle while geometry/window-detach hides (entry.visibleInUI=true) retain full lifecycle behaviour. The new reattach flag state machine is internally consistent — flag is set on notify-hidden, cleared on reattach, and gated at the start of reattach. The only minor concern is that the flag is set unconditionally before checking whether the hide selectors actually responded, which could cause a spurious (but harmless) layout refresh pass in edge cases. No automated tests were run per repo policy.
  • No files require special attention beyond the one reviewed.

Important Files Changed

Filename Overview
Sources/BrowserWindowPortal.swift Adds a browserPortalNeedsRenderingStateReattach associated-object flag on WKWebView to gate the WebKit reattach selectors; conditions hideContainerView's browserPortalNotifyHidden call on entry.visibleInUI so tab/workspace visibility hides skip the WebKit lifecycle that triggers visibilitychange and page reloads.

Sequence Diagram

sequenceDiagram
    participant UI as Tab Logic
    participant Portal as WindowBrowserPortal
    participant Container as WindowBrowserSlotView
    participant WebView as WKWebView

    Note over UI,WebView: Tab switch away (visibleInUI becomes false)
    UI->>Portal: hideWebView sets entry.visibleInUI=false
    Portal->>Portal: synchronizeWebView - shouldHide=true
    Portal->>Portal: hideContainerView()
    Note over Portal: entry.visibleInUI==false - skip browserPortalNotifyHidden
    Portal->>Container: isHidden = true
    Note over WebView: flag stays false - no _exitInWindow fired - no visibilitychange

    Note over UI,WebView: Tab switch back (visibleInUI becomes true)
    UI->>Portal: bind/show sets entry.visibleInUI=true
    Portal->>Portal: synchronizeWebView - shouldHide=false
    Portal->>Container: isHidden = false
    Portal->>Portal: refreshHostedWebViewPresentation()
    Portal->>WebView: browserPortalReattachRenderingState()
    Note over WebView: flag==false - early return - no _enterInWindow - no reload

    Note over UI,WebView: Real detach (window close or anchor removed)
    UI->>Portal: detachWebView or hideContainerView with visibleInUI=true
    Portal->>WebView: browserPortalNotifyHidden - flag=true - _exitInWindow fires
    Portal->>Container: isHidden=true or removeFromSuperview

    Note over UI,WebView: Re-attach or reveal
    Portal->>Portal: refreshHostedWebViewPresentation()
    Portal->>WebView: browserPortalReattachRenderingState()
    Note over WebView: flag==true - proceed - flag=false - _enterInWindow fires
Loading

Last reviewed commit: c52bd24

Comment on lines 63 to 66
func browserPortalNotifyHidden(reason: String) {
browserPortalNeedsRenderingStateReattach = true
let firedSelectors = ["viewDidHide", "_exitInWindow"].filter {
browserPortalCallVoidIfAvailable($0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flag set unconditionally before selector check

browserPortalNeedsRenderingStateReattach is set to true before the selectors are actually attempted (line 64 precedes the filter on line 65). The PR description states the intent as "only re-run WebKit rendering state reattach when a prior hide actually invoked the detach selectors," but the current implementation marks the flag true even when neither viewDidHide nor _exitInWindow responds.

In practice this is harmless — if neither hide selector fires, the matching reattach selectors (viewDidUnhide, _enterInWindow) almost certainly won't fire either, and the only extra work is the needsLayout/needsDisplay calls in browserPortalReattachRenderingState. However, if precision matters for the stated goal, setting the flag only when firedSelectors is non-empty would more accurately track the invariant:

func browserPortalNotifyHidden(reason: String) {
    let firedSelectors = ["viewDidHide", "_exitInWindow"].filter {
        browserPortalCallVoidIfAvailable($0)
    }
    if !firedSelectors.isEmpty {
        browserPortalNeedsRenderingStateReattach = true
    }
    // ...
}

@austinywang
austinywang merged commit dc1f714 into main Mar 12, 2026
15 of 16 checks passed
bn-l pushed a commit to bn-l/cmux that referenced this pull request Apr 3, 2026
…er-tab-switch-reload

Fix browser pane reloads on tab switch

This branch was successfully deployed

1 active deployment
Preview — c52bd24e Deployed Mar 12, 2026 by vercel[bot]
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.

Browser pane reloads when switching tabs

1 participant