Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion Sources/Panels/BrowserNavigationDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -576,7 +576,8 @@ import WebKit
let replacedNavigation = activeMainFrameNavigation
let reportReplacementWillStart = willReplaceNavigationForUserAgentPolicy
return webView.restartNavigationForBrowserUserAgentPolicyIfNeeded(
navigationAction,
for: navigationAction.request,
targetFrameIsMainFrame: navigationAction.targetFrame?.isMainFrame,
decisionHandler: decisionHandler,
willRestart: {
reportReplacementWillStart?(webView, replacedNavigation)
Expand Down
3 changes: 2 additions & 1 deletion Sources/Panels/BrowserPopupWindowController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,8 @@ private class PopupUIDelegate: BrowserPDFPreviewActionUIDelegate {
}

return webView.restartNavigationForBrowserUserAgentPolicyIfNeeded(
navigationAction,
for: navigationAction.request,
targetFrameIsMainFrame: navigationAction.targetFrame?.isMainFrame,
decisionHandler: decisionHandler,
startReplacement: { restartRequest in
controller.requestNavigation(restartRequest, in: webView)
Expand Down
43 changes: 39 additions & 4 deletions Sources/Panels/WKWebView+BrowserUserAgentPolicy.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import CmuxBrowser
import ObjectiveC
import WebKit

private var cmuxBrowserUserAgentPolicyRestartedURLKey: UInt8 = 0

extension WKWebView {
/// Applies the destination identity and reports whether an HTTP(S) navigation must restart.
@MainActor
Expand All @@ -17,7 +20,12 @@ extension WKWebView {
return false
}

guard customUserAgent != resolvedUserAgent else { return false }
// Some macOS versions report the native WebKit identity as "" instead
// of nil, so both must count as the webKitDefault resolution or a
// Sheets destination restarts on every policy pass forever
// (https://github.com/manaflow-ai/cmux/issues/9462).
let appliedUserAgent = customUserAgent.flatMap { $0.isEmpty ? nil : $0 }
guard appliedUserAgent != resolvedUserAgent else { return false }
customUserAgent = resolvedUserAgent
return true
}
Expand All @@ -39,20 +47,47 @@ extension WKWebView {
return browserUserAgentPolicyRestartRequest(for: request)
}

/// Destination of the most recent user-agent-policy restart. Each
/// destination may restart at most once in a row: when the replacement
/// load still reports a mismatched identity, the comparison cannot
/// converge on this system and the load must proceed instead of
/// cancel/reload looping on the main thread (issue #9462).
@MainActor
private var browserUserAgentPolicyRestartedURL: URL? {
get {
objc_getAssociatedObject(self, &cmuxBrowserUserAgentPolicyRestartedURLKey) as? URL
}
set {
objc_setAssociatedObject(
self,
&cmuxBrowserUserAgentPolicyRestartedURLKey,
newValue,
.OBJC_ASSOCIATION_RETAIN_NONATOMIC
)
}
}

@MainActor
func restartNavigationForBrowserUserAgentPolicyIfNeeded(
_ navigationAction: WKNavigationAction,
for request: URLRequest,
targetFrameIsMainFrame: Bool?,
decisionHandler: (WKNavigationActionPolicy) -> Void,
willRestart: () -> Void = {},
startReplacement: (URLRequest) -> Void
) -> Bool {
if targetFrameIsMainFrame == true, let url = request.url {
let alreadyRestartedForDestination = browserUserAgentPolicyRestartedURL == url
browserUserAgentPolicyRestartedURL = nil
if alreadyRestartedForDestination { return false }
}
guard let restartRequest = browserUserAgentPolicyRestartRequest(
for: navigationAction.request,
targetFrameIsMainFrame: navigationAction.targetFrame?.isMainFrame
for: request,
targetFrameIsMainFrame: targetFrameIsMainFrame
) else {
return false
}

browserUserAgentPolicyRestartedURL = request.url
willRestart()
decisionHandler(.cancel)
startReplacement(restartRequest)
Expand Down
57 changes: 57 additions & 0 deletions cmuxTests/BrowserUserAgentPolicyWebKitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,61 @@ struct BrowserUserAgentPolicyWebKitTests {
#expect(webView.browserUserAgentPolicyRestartRequest(for: request) == nil)
#expect(webView.customUserAgent == nil)
}

@Test func sheetsDestinationTreatsEmptyReportedIdentityAsWebKitDefault() {
let webView = WKWebView(frame: .zero, configuration: WKWebViewConfiguration())
// Some macOS versions report the native identity as "" instead of nil
// after a restart clears the custom identity; both must satisfy the
// webKitDefault resolution or Sheets restarts forever (issue #9462).
webView.customUserAgent = ""
let request = URLRequest(
url: URL(string: "https://docs.google.com/spreadsheets/d/example/edit")!
)

#expect(BrowserUserAgentPolicy.system.resolution(for: request.url) == .webKitDefault)
#expect(webView.browserUserAgentPolicyRestartRequest(for: request) == nil)
}

@Test func sheetsDestinationRestartsAtMostOnceWhenIdentityNeverConverges() throws {
let webView = WKWebView(frame: .zero, configuration: WKWebViewConfiguration())
let request = URLRequest(
url: URL(string: "https://docs.google.com/spreadsheets/d/example/edit")!
)
var decisions: [WKNavigationActionPolicy] = []
var replacementRequests: [URLRequest] = []

webView.customUserAgent = BrowserUserAgentPolicy.system.safariCompatibleUserAgent
#expect(webView.restartNavigationForBrowserUserAgentPolicyIfNeeded(
for: request,
targetFrameIsMainFrame: true,
decisionHandler: { decisions.append($0) },
startReplacement: { replacementRequests.append($0) }
))
#expect(decisions == [.cancel])
#expect((webView.customUserAgent ?? "").isEmpty)
let replacementRequest = try #require(replacementRequests.first)

// Simulate a WebKit whose reported identity never matches the resolved
// policy: the replacement pass must proceed instead of restarting again.
webView.customUserAgent = BrowserUserAgentPolicy.system.safariCompatibleUserAgent
#expect(!webView.restartNavigationForBrowserUserAgentPolicyIfNeeded(
for: replacementRequest,
targetFrameIsMainFrame: true,
decisionHandler: { decisions.append($0) },
startReplacement: { replacementRequests.append($0) }
))
#expect(decisions == [.cancel])
#expect(replacementRequests.count == 1)

// A later navigation to the same destination may restart once again.
webView.customUserAgent = BrowserUserAgentPolicy.system.safariCompatibleUserAgent
#expect(webView.restartNavigationForBrowserUserAgentPolicyIfNeeded(
for: request,
targetFrameIsMainFrame: true,
decisionHandler: { decisions.append($0) },
startReplacement: { replacementRequests.append($0) }
))
#expect(decisions == [.cancel, .cancel])
#expect(replacementRequests.count == 2)
}
}