Skip to content

Fix stale browser pane content after drag splits #1215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
austinywang merged 5 commits into from
Mar 12, 2026
Merged

Fix stale browser pane content after drag splits #1215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5233874
ok
austinywang Mar 12, 2026
978f72b
Merge branch 'main' of https://github.com/manaflow-ai/cmux into issue…
austinywang Mar 12, 2026
81618bf
Fix stale browser pane content after drag splits
austinywang Mar 12, 2026
1dbd1e5
Merge branch 'main' of https://github.com/manaflow-ai/cmux into issue…
austinywang Mar 12, 2026
8aafb68
Fix browser pane drag/drop follow-ups
austinywang Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion Sources/BrowserWindowPortal.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2851,9 +2851,12 @@ final class WindowBrowserPortal: NSObject {
}
return
}
let previousTransientRecoveryReason = entry.transientRecoveryReason
func hideContainerView(reason: String) {
containerView.setPaneTopChromeHeight(0)
containerView.setSearchOverlay(nil)
containerView.setPaneDropContext(nil)
containerView.setPortalDragDropZone(nil)
containerView.setDropZoneOverlay(zone: nil)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if !containerView.isHidden, webView.superview === containerView {
webView.browserPortalNotifyHidden(reason: reason)
Expand Down Expand Up @@ -2884,6 +2887,8 @@ final class WindowBrowserPortal: NSObject {
"reason=\(reason) frame=\(browserPortalDebugFrame(containerView.frame))"
)
#endif
containerView.setPaneDropContext(nil)
containerView.setPortalDragDropZone(nil)
containerView.setDropZoneOverlay(zone: nil)
return true
}
Expand Down Expand Up @@ -3027,6 +3032,9 @@ final class WindowBrowserPortal: NSObject {
"reason=hostBoundsNotReady frame=\(browserPortalDebugFrame(containerView.frame))"
)
#endif
containerView.setPaneDropContext(nil)
containerView.setPortalDragDropZone(nil)
containerView.setDropZoneOverlay(zone: nil)
return
}
} else {
Expand Down Expand Up @@ -3089,6 +3097,10 @@ final class WindowBrowserPortal: NSObject {
shouldHide &&
entry.visibleInUI &&
!containerView.isHidden
let recoveredFromTransientGeometry =
previousTransientRecoveryReason != nil &&
transientRecoveryReason == nil &&
!shouldHide
#if DEBUG
let frameWasClamped = hasFiniteFrame && !Self.rectApproximatelyEqual(frameInHost, targetFrame)
if frameWasClamped {
Expand Down Expand Up @@ -3131,6 +3143,7 @@ final class WindowBrowserPortal: NSObject {
if hasExistingVisibleFrame {
containerView.setDropZoneOverlay(zone: nil)
containerView.setPaneDropContext(nil)
containerView.setPortalDragDropZone(nil)
return
}
}
Expand Down Expand Up @@ -3240,10 +3253,16 @@ final class WindowBrowserPortal: NSObject {
}
containerView.setPaneTopChromeHeight(shouldHide ? 0 : entry.paneTopChromeHeight)
containerView.setSearchOverlay(shouldHide ? nil : entry.searchOverlay)
containerView.setPaneDropContext(containerView.isHidden ? nil : entry.paneDropContext)
containerView.setDropZoneOverlay(zone: containerView.isHidden ? nil : entry.dropZone)
if revealedForDisplay {
refreshReasons.append("reveal")
}
if recoveredFromTransientGeometry {
// Drag/reparent churn can recover to the same visible frame we preserved.
// Force a redraw so WebKit doesn't keep stale tiles until a later resize/focus.
refreshReasons.append("transientRecovery")
}
if forcePresentationRefresh {
refreshReasons.append("anchor")
}
Expand All @@ -3254,7 +3273,7 @@ final class WindowBrowserPortal: NSObject {
containerOwnsWebView &&
hostView.reapplyHostedInspectorDividerIfNeeded(in: containerView, reason: "portal.sync")
if !shouldHide, containerOwnsWebView, !refreshReasons.isEmpty {
if hostedInspectorAdjustedDuringSync {
if hostedInspectorAdjustedDuringSync && !recoveredFromTransientGeometry {
#if DEBUG
dlog(
"browser.portal.refresh.skip web=\(browserPortalDebugToken(webView)) " +
Expand Down
154 changes: 142 additions & 12 deletions Sources/Panels/BrowserPanel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2005,6 +2005,12 @@ final class BrowserPanel: Panel, ObservableObject {
setupObservers(for: webView)
}

private func isCurrentWebView(_ candidate: WKWebView, instanceID: UUID? = nil) -> Bool {
guard candidate === webView else { return false }
guard let instanceID else { return true }
return instanceID == webViewInstanceID
}

init(workspaceId: UUID, initialURL: URL? = nil, bypassInsecureHTTPHostOnce: String? = nil) {
self.id = UUID()
self.workspaceId = workspaceId
Expand All @@ -2020,15 +2026,16 @@ final class BrowserPanel: Panel, ObservableObject {
navDelegate.didFinish = { webView in
BrowserHistoryStore.shared.recordVisit(url: webView.url, title: webView.title)
Task { @MainActor [weak self] in
self?.refreshFavicon(from: webView)
self?.applyBrowserThemeModeIfNeeded()
guard let self, self.isCurrentWebView(webView) else { return }
self.refreshFavicon(from: webView)
self.applyBrowserThemeModeIfNeeded()
// Keep find-in-page open through load completion and refresh matches for the new DOM.
self?.restoreFindStateAfterNavigation(replaySearch: true)
self.restoreFindStateAfterNavigation(replaySearch: true)
}
}
navDelegate.didFailNavigation = { [weak self] _, failedURL in
navDelegate.didFailNavigation = { [weak self] failedWebView, failedURL in
Task { @MainActor in
guard let self else { return }
guard let self, self.isCurrentWebView(failedWebView) else { return }
// Clear stale title/favicon from the previous page so the tab
// shows the failed URL instead of the old page's branding.
self.pageTitle = failedURL.isEmpty ? "" : failedURL
Expand Down Expand Up @@ -2162,39 +2169,44 @@ final class BrowserPanel: Panel, ObservableObject {
}

private func setupObservers(for webView: WKWebView) {
let observedWebViewInstanceID = webViewInstanceID

// URL changes
let urlObserver = webView.observe(\.url, options: [.new]) { [weak self] webView, _ in
Task { @MainActor in
self?.currentURL = webView.url
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
self.currentURL = webView.url
}
}
webViewObservers.append(urlObserver)

// Title changes
let titleObserver = webView.observe(\.title, options: [.new]) { [weak self] webView, _ in
Task { @MainActor in
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
// Keep showing the last non-empty title while the new navigation is loading.
// WebKit often clears title to nil/"" during reload/navigation, which causes
// a distracting tab-title flash (e.g. to host/URL). Only accept non-empty titles.
let trimmed = (webView.title ?? "").trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmed.isEmpty else { return }
self?.pageTitle = trimmed
self.pageTitle = trimmed
}
}
webViewObservers.append(titleObserver)

// Loading state
let loadingObserver = webView.observe(\.isLoading, options: [.new]) { [weak self] webView, _ in
Task { @MainActor in
self?.handleWebViewLoadingChanged(webView.isLoading)
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
self.handleWebViewLoadingChanged(webView.isLoading)
}
}
webViewObservers.append(loadingObserver)

// Can go back
let backObserver = webView.observe(\.canGoBack, options: [.new]) { [weak self] webView, _ in
Task { @MainActor in
guard let self else { return }
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
self.nativeCanGoBack = webView.canGoBack
self.refreshNavigationAvailability()
}
Expand All @@ -2204,7 +2216,7 @@ final class BrowserPanel: Panel, ObservableObject {
// Can go forward
let forwardObserver = webView.observe(\.canGoForward, options: [.new]) { [weak self] webView, _ in
Task { @MainActor in
guard let self else { return }
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
self.nativeCanGoForward = webView.canGoForward
self.refreshNavigationAvailability()
}
Expand All @@ -2214,7 +2226,8 @@ final class BrowserPanel: Panel, ObservableObject {
// Progress
let progressObserver = webView.observe(\.estimatedProgress, options: [.new]) { [weak self] webView, _ in
Task { @MainActor in
self?.estimatedProgress = webView.estimatedProgress
guard let self, self.isCurrentWebView(webView, instanceID: observedWebViewInstanceID) else { return }
self.estimatedProgress = webView.estimatedProgress
}
}
webViewObservers.append(progressObserver)
Expand Down Expand Up @@ -2250,6 +2263,9 @@ final class BrowserPanel: Panel, ObservableObject {

webViewObservers.removeAll()
webViewCancellables.removeAll()
faviconTask?.cancel()
faviconTask = nil
faviconRefreshGeneration &+= 1
BrowserWindowPortalRegistry.detach(webView: terminatedWebView)
terminatedWebView.stopLoading()
terminatedWebView.navigationDelegate = nil
Expand All @@ -2260,11 +2276,12 @@ final class BrowserPanel: Panel, ObservableObject {

let replacement = Self.makeWebView()
replacement.pageZoom = desiredZoom
webView = replacement
webViewInstanceID = UUID()
webView = replacement
shouldRenderWebView = wasRenderable

bindWebView(replacement)
applyBrowserThemeModeIfNeeded()

if !history.backHistoryURLStrings.isEmpty || !history.forwardHistoryURLStrings.isEmpty {
restoreSessionNavigationHistory(
Expand Down Expand Up @@ -2360,9 +2377,11 @@ final class BrowserPanel: Panel, ObservableObject {
guard let scheme = pageURL.scheme?.lowercased(), scheme == "http" || scheme == "https" else { return }
faviconRefreshGeneration &+= 1
let refreshGeneration = faviconRefreshGeneration
let refreshWebViewInstanceID = webViewInstanceID

faviconTask = Task { @MainActor [weak self, weak webView] in
guard let self, let webView else { return }
guard self.isCurrentWebView(webView, instanceID: refreshWebViewInstanceID) else { return }
guard self.isCurrentFaviconRefresh(generation: refreshGeneration) else { return }

// Try to discover the best icon URL from the document.
Expand Down Expand Up @@ -2397,6 +2416,7 @@ final class BrowserPanel: Panel, ObservableObject {
discoveredURL = u
}
}
guard self.isCurrentWebView(webView, instanceID: refreshWebViewInstanceID) else { return }
guard self.isCurrentFaviconRefresh(generation: refreshGeneration) else { return }

let fallbackURL = URL(string: "/favicon.ico", relativeTo: pageURL)
Expand All @@ -2422,6 +2442,7 @@ final class BrowserPanel: Panel, ObservableObject {
} catch {
return
}
guard self.isCurrentWebView(webView, instanceID: refreshWebViewInstanceID) else { return }
guard self.isCurrentFaviconRefresh(generation: refreshGeneration) else { return }

guard let http = response as? HTTPURLResponse,
Expand Down Expand Up @@ -2701,12 +2722,121 @@ final class BrowserPanel: Panel, ObservableObject {
if let detachedDeveloperToolsWindowCloseObserver {
NotificationCenter.default.removeObserver(detachedDeveloperToolsWindowCloseObserver)
}
webViewObservers.removeAll()
webViewCancellables.removeAll()
let webView = webView
Task { @MainActor in
BrowserWindowPortalRegistry.detach(webView: webView)
}
}
}

extension BrowserPanel {
private var needsWorkspaceContextReset: Bool {
shouldRenderWebView ||
currentURL != nil ||
!pageTitle.isEmpty ||
faviconPNGData != nil ||
searchState != nil ||
nativeCanGoBack ||
nativeCanGoForward ||
restoredHistoryCurrentURL != nil ||
!restoredBackHistoryStack.isEmpty ||
!restoredForwardHistoryStack.isEmpty ||
estimatedProgress > 0 ||
isLoading ||
isDownloading ||
activeDownloadCount != 0 ||
preferredDeveloperToolsVisible ||
webView.superview != nil
}

func resetForWorkspaceContextChange(reason: String) {
guard needsWorkspaceContextReset else {
#if DEBUG
dlog(
"browser.contextReset.skip panel=\(id.uuidString.prefix(5)) " +
"reason=\(reason) render=\(shouldRenderWebView ? 1 : 0)"
)
#endif
return
}

#if DEBUG
dlog(
"browser.contextReset.begin panel=\(id.uuidString.prefix(5)) " +
"reason=\(reason) render=\(shouldRenderWebView ? 1 : 0) " +
"url=\(preferredURLStringForOmnibar() ?? "nil")"
)
#endif

_ = hideDeveloperTools()
cancelDeveloperToolsRestoreRetry()
preferredDeveloperToolsVisible = false
preferredDeveloperToolsPresentation = .unknown
forceDeveloperToolsRefreshOnNextAttach = false
developerToolsDetachedOpenGraceDeadline = nil
developerToolsRestoreRetryAttempt = 0
preferredAttachedDeveloperToolsWidth = nil
preferredAttachedDeveloperToolsWidthFraction = nil

loadingEndWorkItem?.cancel()
loadingEndWorkItem = nil
faviconTask?.cancel()
faviconTask = nil
faviconRefreshGeneration &+= 1
loadingGeneration &+= 1
activeDownloadCount = 0
isDownloading = false
isLoading = false
estimatedProgress = 0
nativeCanGoBack = false
nativeCanGoForward = false
navigationDelegate?.lastAttemptedURL = nil
abandonRestoredSessionHistoryIfNeeded()

pendingAddressBarFocusRequestId = nil
preferredFocusIntent = .addressBar
suppressOmnibarAutofocusUntil = nil
suppressWebViewFocusUntil = nil
endSuppressWebViewFocusForAddressBar()
invalidateAddressBarPageFocusRestoreAttempts()
invalidateSearchFocusRequests(reason: "contextReset")
searchState = nil

pageTitle = ""
currentURL = nil
faviconPNGData = nil
lastFaviconURLString = nil
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
activePortalHostLease = nil
pendingDistinctPortalHostReplacementPaneId = nil
lockedPortalHost = nil

let oldWebView = webView
webViewObservers.removeAll()
webViewCancellables.removeAll()
BrowserWindowPortalRegistry.detach(webView: oldWebView)
oldWebView.stopLoading()
oldWebView.navigationDelegate = nil
oldWebView.uiDelegate = nil
if let oldCmuxWebView = oldWebView as? CmuxWebView {
oldCmuxWebView.onContextMenuDownloadStateChanged = nil
}

let replacement = Self.makeWebView()
webViewInstanceID = UUID()
webView = replacement
shouldRenderWebView = false
bindWebView(replacement)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
applyBrowserThemeModeIfNeeded()
refreshNavigationAvailability()
Comment on lines +2815 to +2832

@greptile-apps greptile-apps Bot Mar 12, 2026

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.

deinit clears observers after async detach; resetForWorkspaceContextChange does it before

deinit schedules BrowserWindowPortalRegistry.detach asynchronously and then clears webViewObservers/webViewCancellables, meaning observers are still live during the brief window until the async task runs. resetForWorkspaceContextChange does the correct thing — clears observers first, then calls detach synchronously — but the inconsistency means the portal registry's sync path and the dealloc path have different guarantees around observer lifetimes.

This won't cause an immediate crash (the Task hop is a single run-loop cycle), but if a KVO notification fires on the old webView between the Task dispatch and its execution in deinit, it will hit an observer whose owning BrowserPanel is in teardown. Consider reordering deinit to match the reset path:

deinit {
    developerToolsRestoreRetryWorkItem?.cancel()
    developerToolsRestoreRetryWorkItem = nil
    if let obs = detachedDeveloperToolsWindowCloseObserver {
        NotificationCenter.default.removeObserver(obs)
    }
    webViewObservers.removeAll()      // clear before any async work
    webViewCancellables.removeAll()
    let webView = webView
    Task { @MainActor in
        BrowserWindowPortalRegistry.detach(webView: webView)
    }
}

Comment on lines +2826 to +2832

@greptile-apps greptile-apps Bot Mar 12, 2026

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.

webViewInstanceID assigned after webView — binding window is open

Between webView = replacement (line 2803) and webViewInstanceID = UUID() (line 2804), the new webView is live but still carries the old webViewInstanceID. If bindWebView(replacement) (line 2806) or any synchronously-called initializer reads webViewInstanceID to identify the current web view (e.g., correlating completion callbacks to the right instance), it may use a stale ID for the interval between assignment and the UUID update.

Reorder to assign webViewInstanceID before assigning webView:

let replacement = Self.makeWebView()
webViewInstanceID = UUID()
webView = replacement
shouldRenderWebView = false
bindWebView(replacement)
refreshNavigationAvailability()

Comment thread
coderabbitai[bot] marked this conversation as resolved.

#if DEBUG
dlog(
"browser.contextReset.end panel=\(id.uuidString.prefix(5)) " +
"reason=\(reason) instance=\(webViewInstanceID.uuidString.prefix(6))"
)
#endif
}
}

Expand Down
9 changes: 4 additions & 5 deletions Sources/Panels/BrowserPanelView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5523,12 +5523,11 @@ struct WebViewRepresentable: NSViewRepresentable {
}

// SwiftUI can transiently dismantle/rebuild the browser host view during split
// rearrangement. Do not detach the portal-hosted WKWebView here; explicit detach
// still happens on real web view replacement and panel teardown.
// rearrangement. Do not detach the portal-hosted WKWebView or clear its pane-drop
// context here; explicit teardown still happens on real web view replacement and
// panel teardown, and preserving this state lets internal tab drags re-enter the
// browser pane while SwiftUI churns underneath.
BrowserWindowPortalRegistry.updateDropZoneOverlay(for: webView, zone: nil)
BrowserWindowPortalRegistry.updatePaneTopChromeHeight(for: webView, height: 0)
BrowserWindowPortalRegistry.updatePaneDropContext(for: webView, context: nil)
BrowserWindowPortalRegistry.updateSearchOverlay(for: webView, configuration: nil)
coordinator.lastPortalHostId = nil
coordinator.lastSynchronizedHostGeometryRevision = 0
}
Expand Down
11 changes: 1 addition & 10 deletions Sources/TerminalController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13813,16 +13813,7 @@ class TerminalController {
result = "ERROR: Tab not found"
return
}
tab.statusEntries.removeAll()
tab.logEntries.removeAll()
tab.progress = nil
tab.gitBranch = nil
tab.panelGitBranches.removeAll()
tab.pullRequest = nil
tab.panelPullRequests.removeAll()
tab.surfaceListeningPorts.removeAll()
tab.listeningPorts.removeAll()
tab.metadataBlocks.removeAll()
tab.resetSidebarContext(reason: "reset_sidebar")
}
return result
}
Expand Down
Loading
Loading