diff --git a/Chorus/Views/WebView/WebViewCoordinator.swift b/Chorus/Views/WebView/WebViewCoordinator.swift index de21d9b..c289f80 100644 --- a/Chorus/Views/WebView/WebViewCoordinator.swift +++ b/Chorus/Views/WebView/WebViewCoordinator.swift @@ -418,13 +418,26 @@ final class WebViewCoordinator: NSObject, WKNavigationDelegate, WKUIDelegate, WK windowFeatures: WKWindowFeatures ) -> WKWebView? { // If the new-window request is for the same service — e.g. Slack opening - // a workspace via window.open / target=_blank — load it in the existing - // web view instead of spawning a separate NSWindow. Only genuinely + // a workspace via a target=_blank link — load it in the existing web + // view instead of spawning a separate NSWindow. Only genuinely // cross-service popups (real OAuth sign-in windows to another domain) // fall through and get their own window below. - if let targetHost = navigationAction.request.url?.host, - let openerHost = webView.url?.host, - Self.belongsToService(targetHost, serviceHost: openerHost) { + // + // Restricted to real link clicks. A programmatic `window.open()` hands + // the caller a window handle, and sign-in flows test it: + // + // const w = window.open(url); if (!w) return; + // + // Returning nil there reads as "popup blocked", so the page abandons + // whatever it was starting with no window and no error to show for it. + // Same-service `window.open` therefore falls through to a real window, + // which shares the opener's data store so a session started in it lands + // in the right place. + if Self.shouldLoadNewWindowInPlace( + navigationType: navigationAction.navigationType, + targetHost: navigationAction.request.url?.host, + openerHost: webView.url?.host + ) { webView.load(navigationAction.request) return nil } @@ -956,6 +969,26 @@ final class WebViewCoordinator: NSObject, WKNavigationDelegate, WKUIDelegate, WK /// Slack can switch workspaces across *.slack.com in-app — except for /// shared-umbrella domains (see `sharedUmbrellaDomains`) where only the exact /// host matches. Used to decide in-app vs. browser for links and new windows. + /// Whether a new-window request should collapse into the opener's web view + /// instead of getting its own window. + /// + /// Only real link clicks collapse. A programmatic `window.open()` must come + /// back with a window handle: sign-in flows null-check the return value to + /// detect a popup blocker, and a nil answer makes them abandon the flow + /// silently — no window, no error, no request. Factored out so the rule is + /// unit-testable without a live `WKWebView`. + nonisolated static func shouldLoadNewWindowInPlace( + navigationType: WKNavigationType, + targetHost: String?, + openerHost: String? + ) -> Bool { + guard navigationType == .linkActivated, + let targetHost, + let openerHost + else { return false } + return belongsToService(targetHost, serviceHost: openerHost) + } + nonisolated static func belongsToService(_ targetHost: String, serviceHost: String) -> Bool { let target = normalizedHost(targetHost) let service = normalizedHost(serviceHost) diff --git a/ChorusTests/ChorusTests.swift b/ChorusTests/ChorusTests.swift index 213e152..5f99a31 100644 --- a/ChorusTests/ChorusTests.swift +++ b/ChorusTests/ChorusTests.swift @@ -1562,6 +1562,51 @@ final class ChorusTests: XCTestCase { XCTAssertFalse(ServiceInstance(label: "X", url: "https://x.test", forceDarkMode: false).isForceDarkModeEnabled) } + // MARK: - New-window requests (shouldLoadNewWindowInPlace) + + func testClickedSameServiceLinkLoadsInPlace() { + // A target=_blank click inside Slack should reuse the service's own web + // view rather than spawning a window. + XCTAssertTrue(WebViewCoordinator.shouldLoadNewWindowInPlace( + navigationType: .linkActivated, + targetHost: "myteam.slack.com", + openerHost: "app.slack.com" + )) + } + + func testProgrammaticSameServicePopupGetsItsOwnWindow() { + // A page calling window.open() against its own host must still get a + // handle back. Collapsing it in place returned nil, which a caller that + // null-checks the handle reads as a blocked popup — so it gives up + // silently, with no window and no error to show for it. + XCTAssertFalse(WebViewCoordinator.shouldLoadNewWindowInPlace( + navigationType: .other, + targetHost: "teams.cloud.microsoft", + openerHost: "teams.cloud.microsoft" + )) + } + + func testCrossServicePopupGetsItsOwnWindow() { + XCTAssertFalse(WebViewCoordinator.shouldLoadNewWindowInPlace( + navigationType: .linkActivated, + targetHost: "login.microsoftonline.com", + openerHost: "teams.cloud.microsoft" + )) + } + + func testNewWindowWithUnknownHostIsNotCollapsed() { + XCTAssertFalse(WebViewCoordinator.shouldLoadNewWindowInPlace( + navigationType: .linkActivated, + targetHost: nil, + openerHost: "app.slack.com" + )) + XCTAssertFalse(WebViewCoordinator.shouldLoadNewWindowInPlace( + navigationType: .linkActivated, + targetHost: "app.slack.com", + openerHost: nil + )) + } + // MARK: - Link routing (belongsToService) func testBelongsToServiceKeepsSlackWorkspacesInApp() {