From 57988871d696f2e7aa5af4f11a4c3d22fdaaa985 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Thu, 3 Sep 2026 01:13:12 -0700 Subject: [PATCH 1/6] browser: apply external-open rules to sidebar links The sidebar's pull-request and port links (SwiftUI and AppKit rows, and the open-all-pull-requests action) opened in the embedded browser whenever that preference was on, without consulting the URL rules that route a site to the system browser. Sites listed there cannot work in the embedded web view at all, so a rule now wins over the embedded preference on those paths, the same way it does for a click inside a page. --- Sources/ContentView.swift | 19 ++++++++++++++++--- .../BrowserExternalNavigationHandler.swift | 9 +++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index b3f5eecab323..1143e930daca 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -10741,10 +10741,14 @@ struct ContentView: View { var openedCount = 0 if BrowserLinkOpenSettings.openSidebarPullRequestLinksInCmuxBrowser() { + let externalNavigationHandler = BrowserExternalNavigationHandler() for pullRequest in pullRequests { - if tabManager.openBrowser(url: pullRequest.url, insertAtEnd: true) != nil { - openedCount += 1 - } else if NSWorkspace.shared.open(pullRequest.url) { + // The external-open rules outrank the embedded-browser + // preference: rule-listed sites cannot work in the embedded + // web view at all. + let openedEmbedded = !externalNavigationHandler.linkEscapesToSystemBrowser(pullRequest.url) + && tabManager.openBrowser(url: pullRequest.url, insertAtEnd: true) != nil + if openedEmbedded || NSWorkspace.shared.open(pullRequest.url) { openedCount += 1 } } @@ -12660,7 +12664,11 @@ struct VerticalTabsSidebar: View, Equatable { snapshotProvider: { [snapshot = input.workspace] in snapshot } ) let openInBrowser: @MainActor (URL, Bool) -> Void = { [weak tabManager, workspaceId = tab.id] url, preferBrowser in + // The external-open rules outrank the embedded-browser preference + // here just like on the SwiftUI sidebar path: rule-listed sites + // cannot work in the embedded web view at all. if preferBrowser, + !BrowserExternalNavigationHandler().linkEscapesToSystemBrowser(url), let tabManager, tabManager.openBrowser( inWorkspace: workspaceId, @@ -14878,7 +14886,12 @@ struct VerticalTabsSidebar: View, Equatable { opensInCmuxBrowser: Bool ) { selectWorkspaceRow(workspace, index: index, modifiers: NSEvent.modifierFlags) + // The external-open rules outrank the embedded-browser preference: + // a matching link goes to the system browser even when the setting + // prefers embedded, because rule-listed sites cannot work in the + // embedded web view at all. if opensInCmuxBrowser, + !BrowserExternalNavigationHandler().linkEscapesToSystemBrowser(url), tabManager.openBrowser( inWorkspace: workspace.id, url: url, diff --git a/Sources/Panels/BrowserExternalNavigationHandler.swift b/Sources/Panels/BrowserExternalNavigationHandler.swift index 97d9fc81df49..c347a8f666e4 100644 --- a/Sources/Panels/BrowserExternalNavigationHandler.swift +++ b/Sources/Panels/BrowserExternalNavigationHandler.swift @@ -56,6 +56,15 @@ struct BrowserExternalNavigationHandler { return policyCache.currentPolicy().matches(target) } + /// True when a link the user chose outside a web view (a sidebar + /// pull-request or port link) should bypass the embedded browser and go + /// to the system browser. Restricted to web schemes; other schemes have + /// their own external-open routing. + func linkEscapesToSystemBrowser(_ url: URL) -> Bool { + guard Self.isWebNavigationURL(url) else { return false } + return shouldOpenExternally(url) + } + /// Returns whether a user-activated main-frame navigation should be external. func shouldOpenExternally( _ url: URL, From def664081fc8ba324f721088040a1ab9352d5896 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Thu, 3 Sep 2026 01:15:12 -0700 Subject: [PATCH 2/6] browser: require a user event before a link escapes to the system browser WebKit reports a script calling click() on an anchor as .linkActivated, the same as a real click, so the external-open rules on their own let a page hand itself a system-browser open at a moment of its choosing. The navigation-typed escape now also requires an AppKit event in flight (a key, left-mouse, or middle-mouse event) and never intercepts a download, on every path that consults the rules: the main navigation delegate, the target=_blank UI delegate, and both popup delegates. The context menu's Open Link in New Tab is a gesture by construction and says so. The event check is a bound rather than a proof: NSApp.currentEvent says an event is being dispatched, not that this navigation is the thing the user asked for. Middle-clicks arrive as otherMouse events and count. --- .../BrowserExternalNavigationHandler.swift | 24 +++- .../Panels/BrowserNavigationDelegate.swift | 1 + .../Panels/BrowserNavigationPopupPolicy.swift | 5 +- Sources/Panels/BrowserPanel.swift | 8 +- .../Panels/BrowserPopupWindowController.swift | 4 +- cmuxTests/BrowserConfigTests.swift | 111 ++++++++++++++++-- 6 files changed, 139 insertions(+), 14 deletions(-) diff --git a/Sources/Panels/BrowserExternalNavigationHandler.swift b/Sources/Panels/BrowserExternalNavigationHandler.swift index c347a8f666e4..da679b063cbe 100644 --- a/Sources/Panels/BrowserExternalNavigationHandler.swift +++ b/Sources/Panels/BrowserExternalNavigationHandler.swift @@ -66,13 +66,25 @@ struct BrowserExternalNavigationHandler { } /// Returns whether a user-activated main-frame navigation should be external. + /// + /// Downloads keep the download flow. WebKit reports a script calling + /// `click()` on an anchor as `.linkActivated`, the same as a real click, + /// so the rules on their own would let a page hand itself a system-browser + /// open at a moment of its choosing; requiring an AppKit event in flight + /// makes the page ride a click the user actually made. It is a bound + /// rather than a proof — `NSApp.currentEvent` says an event is being + /// dispatched, not that this navigation is the thing the user asked for. func shouldOpenExternally( _ url: URL, navigationType: WKNavigationType, - targetFrameIsMain: Bool? + targetFrameIsMain: Bool?, + shouldPerformDownload: Bool = false, + hasUserActivation: Bool = browserNavigationHasSimpleUserActivation() ) -> Bool { guard navigationType == .linkActivated, targetFrameIsMain != false, + !shouldPerformDownload, + hasUserActivation, Self.isWebNavigationURL(url), !Self.isAppOwnedInternalURL(url) else { return false @@ -148,12 +160,16 @@ struct BrowserExternalNavigationHandler { _ url: URL, navigationType: WKNavigationType, targetFrameIsMain: Bool?, + shouldPerformDownload: Bool = false, + hasUserActivation: Bool = browserNavigationHasSimpleUserActivation(), onOpened: @escaping @MainActor () -> Void = {} ) -> Bool { if case .opened = openConfiguredExternallyResult( url, navigationType: navigationType, targetFrameIsMain: targetFrameIsMain, + shouldPerformDownload: shouldPerformDownload, + hasUserActivation: hasUserActivation, onOpened: onOpened ) { return true @@ -167,13 +183,17 @@ struct BrowserExternalNavigationHandler { _ url: URL, navigationType: WKNavigationType, targetFrameIsMain: Bool?, + shouldPerformDownload: Bool = false, + hasUserActivation: Bool = browserNavigationHasSimpleUserActivation(), onOpened: @escaping @MainActor () -> Void = {} ) -> OpenResult { let externalURL = canonicalURL(for: url) guard shouldOpenExternally( externalURL, navigationType: navigationType, - targetFrameIsMain: targetFrameIsMain + targetFrameIsMain: targetFrameIsMain, + shouldPerformDownload: shouldPerformDownload, + hasUserActivation: hasUserActivation ) else { return .notConfigured } diff --git a/Sources/Panels/BrowserNavigationDelegate.swift b/Sources/Panels/BrowserNavigationDelegate.swift index 69af82714ac4..aeb32f5568f3 100644 --- a/Sources/Panels/BrowserNavigationDelegate.swift +++ b/Sources/Panels/BrowserNavigationDelegate.swift @@ -376,6 +376,7 @@ import WebKit url, navigationType: navigationAction.navigationType, targetFrameIsMain: navigationAction.targetFrame?.isMainFrame, + shouldPerformDownload: navigationAction.shouldPerformDownload, onOpened: { [self] in clearAttemptedRequest(discardPendingBypasses: true) let reportTerminalCancellation = terminalPolicyCancellationReporter?( diff --git a/Sources/Panels/BrowserNavigationPopupPolicy.swift b/Sources/Panels/BrowserNavigationPopupPolicy.swift index e5d491a475c0..8a58360e3d28 100644 --- a/Sources/Panels/BrowserNavigationPopupPolicy.swift +++ b/Sources/Panels/BrowserNavigationPopupPolicy.swift @@ -67,7 +67,10 @@ func browserNavigationHasSimpleUserActivation( currentEventType: NSEvent.EventType? = NSApp.currentEvent?.type ) -> Bool { switch currentEventType { - case .keyDown, .keyUp, .leftMouseDown, .leftMouseUp: + case .keyDown, .keyUp, .leftMouseDown, .leftMouseUp, + .otherMouseDown, .otherMouseUp: + // Middle-clicks arrive as otherMouse events and are user input the + // same as a left click, so a matched link still escapes on them. return true default: return false diff --git a/Sources/Panels/BrowserPanel.swift b/Sources/Panels/BrowserPanel.swift index e00e96fb0044..715de347c383 100644 --- a/Sources/Panels/BrowserPanel.swift +++ b/Sources/Panels/BrowserPanel.swift @@ -6367,11 +6367,14 @@ extension BrowserPanel { } /// Routes the context-menu tab action through configured external rules. + /// Choosing a menu item is the user's own gesture, so the user-event + /// guard is satisfied by construction here. func openContextMenuLinkInNewTab(url: URL) { switch externalNavigationHandler.openConfiguredExternallyResult( url, navigationType: .linkActivated, - targetFrameIsMain: true + targetFrameIsMain: true, + hasUserActivation: true ) { case .opened: return @@ -8666,7 +8669,8 @@ final class BrowserUIDelegate: BrowserPDFPreviewActionUIDelegate { switch externalNavigationHandler.openConfiguredExternallyResult( url, navigationType: navigationAction.navigationType, - targetFrameIsMain: navigationAction.targetFrame?.isMainFrame + targetFrameIsMain: navigationAction.targetFrame?.isMainFrame, + shouldPerformDownload: navigationAction.shouldPerformDownload ) { case .opened: return nil diff --git a/Sources/Panels/BrowserPopupWindowController.swift b/Sources/Panels/BrowserPopupWindowController.swift index aeba931e78a8..2cadf7590bcb 100644 --- a/Sources/Panels/BrowserPopupWindowController.swift +++ b/Sources/Panels/BrowserPopupWindowController.swift @@ -500,7 +500,8 @@ private final class PopupUIDelegate: BrowserPDFPreviewActionUIDelegate { switch externalNavigationHandler.openConfiguredExternallyResult( url, navigationType: navigationAction.navigationType, - targetFrameIsMain: navigationAction.targetFrame?.isMainFrame + targetFrameIsMain: navigationAction.targetFrame?.isMainFrame, + shouldPerformDownload: navigationAction.shouldPerformDownload ) { case .opened: return nil @@ -797,6 +798,7 @@ private final class PopupUIDelegate: BrowserPDFPreviewActionUIDelegate { url, navigationType: navigationAction.navigationType, targetFrameIsMain: navigationAction.targetFrame?.isMainFrame, + shouldPerformDownload: navigationAction.shouldPerformDownload, onOpened: { [self] in clearAttemptedRequest(discardPendingBypasses: true) } diff --git a/cmuxTests/BrowserConfigTests.swift b/cmuxTests/BrowserConfigTests.swift index 2c52ffad5397..38ab6232b196 100644 --- a/cmuxTests/BrowserConfigTests.swift +++ b/cmuxTests/BrowserConfigTests.swift @@ -5666,7 +5666,8 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { handler.shouldOpenExternally( aliasedURL, navigationType: .linkActivated, - targetFrameIsMain: true + targetFrameIsMain: true, + hasUserActivation: true ) ) XCTAssertEqual(handler.openConfiguredExternallyResult(aliasedURL), .opened) @@ -5722,21 +5723,24 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( url, navigationType: .linkActivated, - targetFrameIsMain: true + targetFrameIsMain: true, + hasUserActivation: true ) ) XCTAssertFalse( BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( url, navigationType: .other, - targetFrameIsMain: true + targetFrameIsMain: true, + hasUserActivation: true ) ) XCTAssertFalse( BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( url, navigationType: .linkActivated, - targetFrameIsMain: false + targetFrameIsMain: false, + hasUserActivation: true ) ) let callbackURL = try XCTUnwrap( @@ -5747,7 +5751,8 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( callbackURL, navigationType: .linkActivated, - targetFrameIsMain: true + targetFrameIsMain: true, + hasUserActivation: true ) ) let siblingCallbackURL = try XCTUnwrap( @@ -5757,7 +5762,8 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( siblingCallbackURL, navigationType: .linkActivated, - targetFrameIsMain: true + targetFrameIsMain: true, + hasUserActivation: true ) ) XCTAssertFalse( @@ -5776,7 +5782,8 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( diffViewerURL, navigationType: .linkActivated, - targetFrameIsMain: true + targetFrameIsMain: true, + hasUserActivation: true ) ) let customAppURL = try XCTUnwrap(URL(string: "slack://open?token=secret")) @@ -5784,7 +5791,8 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( customAppURL, navigationType: .linkActivated, - targetFrameIsMain: true + targetFrameIsMain: true, + hasUserActivation: true ), "Configured browser rules must not bypass the existing custom-scheme confirmation prompt." ) @@ -5809,6 +5817,7 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { url, navigationType: .linkActivated, targetFrameIsMain: true, + hasUserActivation: true, onOpened: { didRunAfterOpen = true } @@ -5832,6 +5841,92 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { XCTAssertEqual(result, .failed) } + + func testExternalOpenDomainPatternCoversSubdomainURLs() throws { + defaults.set("corp.example", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) + let handler = BrowserExternalNavigationHandler(defaults: defaults) + XCTAssertTrue(handler.shouldOpenExternally(try XCTUnwrap(URL(string: "https://corp.example/")))) + XCTAssertTrue(handler.shouldOpenExternally(try XCTUnwrap(URL(string: "https://sso.corp.example/login")))) + XCTAssertTrue(handler.shouldOpenExternally(try XCTUnwrap(URL(string: "https://CORP.EXAMPLE/tools")))) + XCTAssertFalse(handler.shouldOpenExternally(try XCTUnwrap(URL(string: "https://unrelated.example/")))) + } + + func testNavigationEscapeRequiresUserActivation() throws { + defaults.set("corp.example", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) + let url = try XCTUnwrap(URL(string: "https://sso.corp.example/")) + let handler = BrowserExternalNavigationHandler(defaults: defaults) + // A page can call click() on an anchor and WebKit still reports + // .linkActivated, so the rules alone are not enough to let it escape. + XCTAssertFalse( + handler.shouldOpenExternally( + url, + navigationType: .linkActivated, + targetFrameIsMain: true, + shouldPerformDownload: false, + hasUserActivation: false + ) + ) + XCTAssertTrue( + handler.shouldOpenExternally( + url, + navigationType: .linkActivated, + targetFrameIsMain: true, + shouldPerformDownload: false, + hasUserActivation: true + ) + ) + XCTAssertEqual( + handler.openConfiguredExternallyResult( + url, + navigationType: .linkActivated, + targetFrameIsMain: true, + hasUserActivation: false + ), + .notConfigured + ) + } + + func testNavigationEscapeStillRejectsDownloadsAndNonLinkNavigations() throws { + defaults.set("corp.example", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) + let url = try XCTUnwrap(URL(string: "https://sso.corp.example/")) + let handler = BrowserExternalNavigationHandler(defaults: defaults) + XCTAssertFalse( + handler.shouldOpenExternally( + url, + navigationType: .linkActivated, + targetFrameIsMain: true, + shouldPerformDownload: true, + hasUserActivation: true + ) + ) + XCTAssertFalse( + handler.shouldOpenExternally( + url, + navigationType: .other, + targetFrameIsMain: true, + shouldPerformDownload: false, + hasUserActivation: true + ) + ) + } + + func testSimpleUserActivationTracksTheEventInFlight() { + XCTAssertTrue(browserNavigationHasSimpleUserActivation(currentEventType: .leftMouseUp)) + XCTAssertTrue(browserNavigationHasSimpleUserActivation(currentEventType: .keyDown)) + XCTAssertTrue(browserNavigationHasSimpleUserActivation(currentEventType: .otherMouseDown)) + XCTAssertTrue(browserNavigationHasSimpleUserActivation(currentEventType: .otherMouseUp)) + XCTAssertFalse(browserNavigationHasSimpleUserActivation(currentEventType: nil)) + XCTAssertFalse(browserNavigationHasSimpleUserActivation(currentEventType: .mouseMoved)) + } + + func testSidebarLinkEscapesOnlyForWebSchemes() throws { + defaults.set("corp.example", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) + let handler = BrowserExternalNavigationHandler(defaults: defaults) + XCTAssertTrue(handler.linkEscapesToSystemBrowser(try XCTUnwrap(URL(string: "https://sso.corp.example/")))) + XCTAssertFalse(handler.linkEscapesToSystemBrowser(try XCTUnwrap(URL(string: "file:///tmp/corp.example.html")))) + defaults.removeObject(forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) + XCTAssertFalse(handler.linkEscapesToSystemBrowser(try XCTUnwrap(URL(string: "https://corp.example/")))) + } } From 258e3f76ad5f9e72c823fa0d5b09affc9abbab54 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Thu, 3 Sep 2026 01:16:57 -0700 Subject: [PATCH 3/6] browser: e2e coverage for external-open link routing BrowserExternalOpenRoutingUITests drives real WebKit link activations through the socket browser.click against a local fixture and asserts routing at the delegate layer, where popup-vs-link-activation behavior actually diverges and unit tests cannot reach. Escapes are captured to a file through the existing DEBUG-only UI-test sink (CMUX_UI_TEST_CAPTURE_EXTERNAL_OPEN_PATH) from the external-navigation handler's default opener, so CI never opens Safari. Four cases: a matched link click escapes while the embedded page stays put; an unmatched click navigates embedded; a scripted window.open to a matched host never escapes; a target=_blank form POST to a matched host stays embedded. The shared BrowserFixtureSocketTestCase gains subclass hooks for launch arguments and environment, falls back from the in-process socket client to nc -U and then the bundled cmux CLI, disables hidden-webview discarding for the backgrounded UI-test host, and polls browser.wait through the cold-start content-process transient. --- .../BrowserExternalNavigationHandler.swift | 19 +- cmux.xcodeproj/project.pbxproj | 4 + .../BrowserExternalOpenRoutingUITests.swift | 131 ++++++++++ .../BrowserFixtureInteractionUITests.swift | 229 +++++++++++++++++- .../external-open-routing.html | 14 ++ .../BrowserFixtures/external-open-target.html | 7 + 6 files changed, 398 insertions(+), 6 deletions(-) create mode 100644 cmuxUITests/BrowserExternalOpenRoutingUITests.swift create mode 100644 cmuxUITests/BrowserFixtures/external-open-routing.html create mode 100644 cmuxUITests/BrowserFixtures/external-open-target.html diff --git a/Sources/Panels/BrowserExternalNavigationHandler.swift b/Sources/Panels/BrowserExternalNavigationHandler.swift index da679b063cbe..2a62fe1ca1fc 100644 --- a/Sources/Panels/BrowserExternalNavigationHandler.swift +++ b/Sources/Panels/BrowserExternalNavigationHandler.swift @@ -3,6 +3,7 @@ import AppKit import CmuxBrowser import CmuxCore import CmuxSettings +import CmuxTestSupport import Foundation import WebKit @@ -23,13 +24,29 @@ struct BrowserExternalNavigationHandler { init( defaults: UserDefaults = .standard, - openURL: @escaping @MainActor @Sendable (URL) -> Bool = { NSWorkspace.shared.open($0) } + openURL: @escaping @MainActor @Sendable (URL) -> Bool = Self.openInSystemBrowser ) { self.defaults = defaults self.openURL = openURL self.policyCache = BrowserExternalURLPolicyCache(defaults: defaults) } + /// The default opener. UI tests observe external-open routing through the + /// capture sink; a configured sink intercepts the open so CI never + /// launches a real browser. + @MainActor + private static func openInSystemBrowser(_ url: URL) -> Bool { +#if DEBUG + if UITestCaptureSink().appendLineIfConfigured( + envKey: "CMUX_UI_TEST_CAPTURE_EXTERNAL_OPEN_PATH", + line: "externalOpen \(url.absoluteString)" + ) { + return true + } +#endif + return NSWorkspace.shared.open(url) + } + /// Returns whether a URL matches a configured external rule. func shouldOpenExternally(_ url: URL) -> Bool { let externalURL = canonicalURL(for: url) diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index bd7b6fc362cd..e90dd2717067 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -420,6 +420,7 @@ B5A5E55B0000000000000001 /* BrowserEphemeralRestorationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B5A5E55B0000000000000002 /* BrowserEphemeralRestorationTests.swift */; }; C2035A010000000000000001 /* BrowserErrorPage.swift in Sources */ = {isa = PBXBuildFile; fileRef = C2035A010000000000000002 /* BrowserErrorPage.swift */; }; BEEA00010000000000000001 /* BrowserExternalNavigationHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEEA00010000000000000002 /* BrowserExternalNavigationHandler.swift */; }; + B90000E2A1B2C3D4E5F60719 /* BrowserExternalOpenRoutingUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B90000E1A1B2C3D4E5F60719 /* BrowserExternalOpenRoutingUITests.swift */; }; 129980000000000000000001 /* BrowserFailedNavigationReloadTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 129980000000000000000002 /* BrowserFailedNavigationReloadTests.swift */; }; D7632A11A1B2C3D4E5F60718 /* BrowserFileDropNavigationGuard.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7632A12A1B2C3D4E5F60718 /* BrowserFileDropNavigationGuard.swift */; }; A5008381 /* BrowserFindJavaScriptTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5008380 /* BrowserFindJavaScriptTests.swift */; }; @@ -4533,6 +4534,7 @@ B5A5E55B0000000000000002 /* BrowserEphemeralRestorationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserEphemeralRestorationTests.swift; sourceTree = ""; }; C2035A010000000000000002 /* BrowserErrorPage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserErrorPage.swift; sourceTree = ""; }; BEEA00010000000000000002 /* BrowserExternalNavigationHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserExternalNavigationHandler.swift; sourceTree = ""; }; + B90000E1A1B2C3D4E5F60719 /* BrowserExternalOpenRoutingUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserExternalOpenRoutingUITests.swift; sourceTree = ""; }; 129980000000000000000002 /* BrowserFailedNavigationReloadTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserFailedNavigationReloadTests.swift; sourceTree = ""; }; D7632A12A1B2C3D4E5F60718 /* BrowserFileDropNavigationGuard.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserFileDropNavigationGuard.swift; sourceTree = ""; }; A5008380 /* BrowserFindJavaScriptTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserFindJavaScriptTests.swift; sourceTree = ""; }; @@ -8325,6 +8327,7 @@ D0E0F0B3A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift */, D93410000000000000000002 /* BrowserDownloadsPopoverContrastUITests.swift */, FB100001A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift */, + B90000E1A1B2C3D4E5F60719 /* BrowserExternalOpenRoutingUITests.swift */, 7B5F1A2E9C0D4B6A8E217301 /* BrowserFixtureInteractionUITests.swift */, 81464EB0D515022903F26DD8 /* WorkspaceWorkingDirectorySpawnUITests.swift */, 8478B0000000000000000001 /* BrowserFixtureSocketTestCase+PendingRequest.swift */, @@ -15513,6 +15516,7 @@ B9000012A1B2C3D4E5F60719 /* AutomationSocketUITests.swift in Sources */, AA1B2C3D4E5F60718 /* BonsplitTabDragUITests.swift in Sources */, D93410000000000000000001 /* BrowserDownloadsPopoverContrastUITests.swift in Sources */, + B90000E2A1B2C3D4E5F60719 /* BrowserExternalOpenRoutingUITests.swift in Sources */, 7B5F1A2E9C0D4B6A8E217302 /* BrowserFixtureInteractionUITests.swift in Sources */, 8478B0000000000000000002 /* BrowserFixtureSocketTestCase+PendingRequest.swift in Sources */, FB100000A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift in Sources */, diff --git a/cmuxUITests/BrowserExternalOpenRoutingUITests.swift b/cmuxUITests/BrowserExternalOpenRoutingUITests.swift new file mode 100644 index 000000000000..a91d06527db4 --- /dev/null +++ b/cmuxUITests/BrowserExternalOpenRoutingUITests.swift @@ -0,0 +1,131 @@ +import XCTest +import Foundation + +/// End-to-end coverage for external-open rule routing in the embedded +/// browser. The app launches with a rule matching `127.0.0.1:8397` and with +/// `CMUX_UI_TEST_CAPTURE_EXTERNAL_OPEN_PATH` configured, so system-browser +/// escapes are written to a capture file instead of launching a real +/// browser. Clicks are delivered through the socket `browser.click` (real +/// WebKit link activations), which is exactly the layer where routing bugs +/// live — matcher unit tests cannot see delegate wiring. The capture line is +/// written by the shared external-navigation handler's default opener, so +/// every escape path (navigation delegate, target=_blank, popups) lands here. +final class BrowserExternalOpenRoutingUITests: BrowserFixtureSocketTestCase { + private var capturePath = "" + + override var extraLaunchArguments: [String] { + ["-browserExternalOpenPatterns", "127.0.0.1:8397"] + } + + override var extraLaunchEnvironment: [String: String] { + ["CMUX_UI_TEST_CAPTURE_EXTERNAL_OPEN_PATH": capturePath] + } + + override func setUp() { + super.setUp() + capturePath = "/tmp/cmux-ui-test-external-open-\(UUID().uuidString).log" + try? FileManager.default.removeItem(atPath: capturePath) + } + + override func tearDown() { + try? FileManager.default.removeItem(atPath: capturePath) + super.tearDown() + } + + private func captureLines() -> [String] { + (try? String(contentsOfFile: capturePath, encoding: .utf8))? + .split(separator: "\n").map(String.init) ?? [] + } + + private func waitForCaptureLine( + containing needle: String, + timeout: TimeInterval = 10.0 + ) -> Bool { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + if captureLines().contains(where: { $0.contains(needle) }) { return true } + RunLoop.current.run(until: Date().addingTimeInterval(0.2)) + } + return false + } + + func testMatchedLinkClickEscapesToSystemBrowser() throws { + try launchApp() + let sid = try openFixture("external-open-routing") + try waitForSelector("#matched-link", surfaceID: sid) + try socketResult(method: "browser.click", params: ["surface_id": sid, "selector": "#matched-link"]) + XCTAssertTrue( + waitForCaptureLine(containing: "http://127.0.0.1:8397/matched"), + "Expected matched link click to escape to the system browser. capture=\(captureLines())" + ) + XCTAssertTrue( + captureLines().contains { $0.hasPrefix("externalOpen ") }, + "Expected the escape to route through the external-navigation handler. capture=\(captureLines())" + ) + // The embedded page must not have navigated to the matched URL. + let href = try evalString("location.href", surfaceID: sid) + XCTAssertTrue( + href.hasSuffix("external-open-routing.html"), + "Embedded page navigated away after an escape: \(href)" + ) + } + + func testUnmatchedLinkClickStaysEmbedded() throws { + try launchApp() + let sid = try openFixture("external-open-routing") + try waitForSelector("#unmatched-link", surfaceID: sid) + try socketResult(method: "browser.click", params: ["surface_id": sid, "selector": "#unmatched-link"]) + try socketResult( + method: "browser.wait", + params: ["surface_id": sid, "load_state": "complete", "timeout_ms": 10_000], + responseTimeout: 16.0 + ) + let href = try evalString("location.href", surfaceID: sid) + XCTAssertTrue( + href.hasSuffix("external-open-target.html"), + "Expected unmatched link to navigate embedded, got: \(href)" + ) + XCTAssertEqual(captureLines(), [], "Unmatched navigation must not touch the system browser") + } + + func testScriptedWindowOpenToMatchedURLNeverEscapes() throws { + try launchApp() + let sid = try openFixture("external-open-routing") + try waitForSelector("#popup-button", surfaceID: sid) + try socketResult(method: "browser.click", params: ["surface_id": sid, "selector": "#popup-button"]) + // Affirmative: window.open must actually open an embedded tab (proves it + // fired and stayed embedded, not that it silently no-op'd), and it must + // not reach the system browser. + let popupOpenedEmbedded = try waitForEmbeddedTab(urlContaining: "127.0.0.1:8397") + let tabsAfterPopup = try openTabURLs() + XCTAssertTrue( + popupOpenedEmbedded, + "Scripted window.open should open an embedded tab. tabs=\(tabsAfterPopup)" + ) + XCTAssertFalse( + captureLines().contains { $0.contains("/popup") }, + "Scripted window.open must never reach the system browser. capture=\(captureLines())" + ) + } + + func testFormPostTargetBlankToMatchedHostStaysEmbedded() throws { + try launchApp() + let sid = try openFixture("external-open-routing") + try waitForSelector("#post-submit", surfaceID: sid) + try socketResult(method: "browser.click", params: ["surface_id": sid, "selector": "#post-submit"]) + // Affirmative: the POST must open an embedded tab at the matched host + // (proves the submission happened and stayed embedded, keeping its body + // rather than being dropped or re-issued as a GET in the system browser), + // and it must not reach the system browser. + let postOpenedEmbedded = try waitForEmbeddedTab(urlContaining: "127.0.0.1:8397") + let tabsAfterPost = try openTabURLs() + XCTAssertTrue( + postOpenedEmbedded, + "target=_blank form POST should open an embedded tab. tabs=\(tabsAfterPost)" + ) + XCTAssertFalse( + captureLines().contains { $0.contains("/post") }, + "A form POST must keep its body embedded, never escape as a GET. capture=\(captureLines())" + ) + } +} diff --git a/cmuxUITests/BrowserFixtureInteractionUITests.swift b/cmuxUITests/BrowserFixtureInteractionUITests.swift index afd0dfd5321d..dbc741ec9190 100644 --- a/cmuxUITests/BrowserFixtureInteractionUITests.swift +++ b/cmuxUITests/BrowserFixtureInteractionUITests.swift @@ -19,6 +19,9 @@ class BrowserFixtureSocketTestCase: XCTestCase { private var diagnosticsPath = "" private var launchTag = "" private(set) var app: XCUIApplication? + /// Workspace id of the most recent `openBrowserSurface`/`openFixture`, so + /// tests can enumerate its tabs via `browser.tab.list`. + private(set) var lastWorkspaceID = "" override func setUp() { super.setUp() @@ -42,6 +45,11 @@ class BrowserFixtureSocketTestCase: XCTestCase { // MARK: - Launch + /// Subclass hooks for extra launch configuration (defaults registered via + /// NSArgumentDomain, capture-sink env keys, ...). + var extraLaunchArguments: [String] { [] } + var extraLaunchEnvironment: [String: String] { [:] } + @discardableResult func launchApp(additionalLaunchArguments: [String] = []) throws -> XCUIApplication { let app = XCUIApplication.cmuxTestApplication() @@ -50,6 +58,7 @@ class BrowserFixtureSocketTestCase: XCTestCase { "-AppleLanguages", "(en)", "-AppleLocale", "en_US", ] + additionalLaunchArguments + app.launchArguments += extraLaunchArguments app.launchEnvironment["CMUX_UI_TEST_MODE"] = "1" app.launchEnvironment["CMUX_SOCKET_ENABLE"] = "1" app.launchEnvironment["CMUX_SOCKET_MODE"] = "allowAll" @@ -57,12 +66,20 @@ class BrowserFixtureSocketTestCase: XCTestCase { app.launchEnvironment["CMUX_ALLOW_SOCKET_OVERRIDE"] = "1" app.launchEnvironment["CMUX_UI_TEST_SOCKET_SANITY"] = "1" app.launchEnvironment["CMUX_UI_TEST_DIAGNOSTICS_PATH"] = diagnosticsPath + // The UI-test host is frequently backgrounded (activation is best-effort + // on CI, see below), which marks the browser split "hidden" and lets the + // hidden-webview discarder tear down the WKWebView mid-eval — surfacing + // as "completion handler no longer reachable". Keep it alive for tests. + app.launchEnvironment["CMUX_BROWSER_HIDDEN_WEBVIEW_DISCARD_ENABLED"] = "false" // Debug launches require a tag outside reload.sh; provide one in UITests so CI // does not fail with "Application ... does not have a process ID". app.launchEnvironment["CMUX_TAG"] = launchTag if let path = ProcessInfo.processInfo.environment["PATH"], !path.isEmpty { app.launchEnvironment["PATH"] = path } + for (key, value) in extraLaunchEnvironment { + app.launchEnvironment[key] = value + } self.app = app // On headless CI runners (no GUI session), XCUIApplication.launch() // blocks ~60s then fails with "Failed to activate application @@ -92,6 +109,12 @@ class BrowserFixtureSocketTestCase: XCTestCase { /// Sends one V2 request and returns the raw response envelope /// (`{"id":…,"ok":…,"result"/"error":…}`), or nil if the socket did not answer. + /// Falls back to the `nc -U` transport when the in-process Darwin client + /// cannot connect (see `controlSocketCommandViaNetcat`), matching + /// `BrowserPaneNavigationKeybindUITests`, and finally to the bundled CLI + /// binary: some hosts refuse unix-socket connects from the UI-test runner + /// and its child processes entirely, while a full `cmux rpc` invocation + /// (the same client real users script with) connects fine. func socketEnvelope( method: String, params: [String: Any], @@ -103,6 +126,82 @@ class BrowserFixtureSocketTestCase: XCTestCase { "params": params, ] return ControlSocketClient(path: socketPath, responseTimeout: responseTimeout).sendJSON(request) + ?? controlSocketJSONViaNetcat(request, socketPath: socketPath, responseTimeout: responseTimeout) + ?? socketEnvelopeViaBundledCLI(method: method, params: params, responseTimeout: responseTimeout) + } + + /// Runs `cmux rpc ` from the app bundle the UI test built, + /// pointed at this test's socket. Returns a synthesized success envelope + /// (the CLI exits non-zero on `ok: false` responses and transport errors). + private func socketEnvelopeViaBundledCLI( + method: String, + params: [String: Any], + responseTimeout: TimeInterval + ) -> [String: Any]? { + guard let cli = Self.bundledCLIPath(), + JSONSerialization.isValidJSONObject(params), + let paramsData = try? JSONSerialization.data(withJSONObject: params), + let paramsJSON = String(data: paramsData, encoding: .utf8) else { + return nil + } + let process = Process() + process.executableURL = URL(fileURLWithPath: cli) + process.arguments = ["rpc", method, paramsJSON] + var environment = ProcessInfo.processInfo.environment + for key in environment.keys where key.hasPrefix("CMUX_") { + environment.removeValue(forKey: key) + } + environment["CMUX_SOCKET_PATH"] = socketPath + // Honor the caller's timeout budget on the fallback path too; the CLI + // otherwise uses its own default (CLI/cmux.swift responseTimeoutSeconds). + environment["CMUXTERM_CLI_RESPONSE_TIMEOUT_SEC"] = String(max(1.0, responseTimeout)) + process.environment = environment + let stdout = Pipe() + let stderr = Pipe() + process.standardOutput = stdout + process.standardError = stderr + do { + try process.run() + } catch { + return nil + } + // Drain stderr concurrently and read stdout before waiting on exit, so the + // CLI can't block on a full pipe buffer and hang the test run. + let stderrDrain = DispatchQueue(label: "cmux-ui-test-cli-stderr-drain") + stderrDrain.async { _ = stderr.fileHandleForReading.readDataToEndOfFile() } + let data = stdout.fileHandleForReading.readDataToEndOfFile() + process.waitUntilExit() + guard process.terminationStatus == 0 else { return nil } + guard let result = (try? JSONSerialization.jsonObject(with: data)) as? [String: Any] else { + return nil + } + return ["ok": true, "result": result] + } + + /// Locates `Contents/Resources/bin/cmux` inside the app the test target + /// built, starting from the test bundle's products directory. + private static func bundledCLIPath() -> String? { + // …/Build/Products/Debug/cmuxUITests-Runner.app/Contents/PlugIns/cmuxUITests.xctest + // Walk ancestors instead of counting components: the runner nests the + // test bundle one level deeper (Contents/) than a flat reading of the + // path suggests, and a fixed hop count silently lands inside the + // runner app, never finding the sibling cmux app. + var candidate = URL(fileURLWithPath: Bundle(for: BrowserFixtureSocketTestCase.self).bundlePath) + for _ in 0..<6 { + candidate.deleteLastPathComponent() + guard !candidate.path.hasSuffix(".app"), candidate.path != "/" else { continue } + let entries = (try? FileManager.default.contentsOfDirectory(atPath: candidate.path)) ?? [] + for entry in entries where entry.hasSuffix(".app") { + let cli = candidate + .appendingPathComponent(entry) + .appendingPathComponent("Contents/Resources/bin/cmux") + .path + if FileManager.default.isExecutableFile(atPath: cli) { + return cli + } + } + } + return nil } /// Sends one V2 request, asserts `ok == true`, and returns `result`. @@ -158,6 +257,7 @@ class BrowserFixtureSocketTestCase: XCTestCase { file: file, line: line ) + lastWorkspaceID = workspaceID let sourceSurfaceID = try XCTUnwrap( workspace["surface_id"] as? String, "workspace.create returned no surface_id: \(workspace)", @@ -201,16 +301,115 @@ class BrowserFixtureSocketTestCase: XCTestCase { file: file, line: line ) - try socketResult( - method: "browser.wait", - params: ["surface_id": surfaceID, "load_state": "complete", "timeout_ms": 10_000], - responseTimeout: 16.0, + // Assert readiness, but poll through the cold-start content-process + // transient (an eval can return "completion handler no longer + // reachable" while the web content process respawns on a fresh app), + // so this keeps its diagnostic value for callers that act right after + // openFixture without a separate waitForSelector gate. + try waitForCondition( + ["surface_id": surfaceID, "load_state": "complete"], + describedAs: "load_state=complete", file: file, line: line ) return surfaceID } + /// Polls a `browser.wait` condition through the transient js_error the + /// first browser interaction of a freshly launched app can hit, failing + /// only when the deadline passes with the condition still unmet. + func waitForCondition( + _ waitParams: [String: Any], + describedAs description: String, + timeout: TimeInterval = 15.0, + file: StaticString = #filePath, + line: UInt = #line + ) throws { + let deadline = Date().addingTimeInterval(timeout) + var params = waitParams + params["timeout_ms"] = 3_000 + var lastEnvelope: [String: Any]? + while Date() < deadline { + let envelope = socketEnvelope(method: "browser.wait", params: params, responseTimeout: 6.0) + lastEnvelope = envelope + if (envelope?["ok"] as? Bool) == true { return } + RunLoop.current.run(until: Date().addingTimeInterval(0.3)) + } + XCTFail( + "browser.wait(\(description)) timed out. last=\(String(describing: lastEnvelope))", + file: file, + line: line + ) + } + + /// Returns the tab URLs currently open in `lastWorkspaceID`. + func openTabURLs(file: StaticString = #filePath, line: UInt = #line) throws -> [String] { + let result = try socketResult( + method: "browser.tab.list", + params: ["workspace_id": lastWorkspaceID], + file: file, + line: line + ) + let tabs = result["tabs"] as? [[String: Any]] ?? [] + return tabs.compactMap { $0["url"] as? String } + } + + /// Polls until a workspace tab's URL contains `needle`, proving an embedded + /// tab was created (the affirmative outcome for "stayed embedded" tests). + @discardableResult + func waitForEmbeddedTab( + urlContaining needle: String, + timeout: TimeInterval = 10.0, + file: StaticString = #filePath, + line: UInt = #line + ) throws -> Bool { + let deadline = Date().addingTimeInterval(timeout) + while Date() < deadline { + // Poll through the transient socket errors the first browser + // interaction of a freshly launched app can hit (matching + // waitForCondition), retrying until the deadline instead of throwing + // out of the loop on the first miss. + let envelope = socketEnvelope( + method: "browser.tab.list", + params: ["workspace_id": lastWorkspaceID], + responseTimeout: 6.0 + ) + if (envelope?["ok"] as? Bool) == true, + let result = envelope?["result"] as? [String: Any], + let tabs = result["tabs"] as? [[String: Any]] { + let urls = tabs.compactMap { $0["url"] as? String } + if urls.contains(where: { $0.contains(needle) }) { return true } + } + RunLoop.current.run(until: Date().addingTimeInterval(0.3)) + } + return false + } + + /// Waits for a selector to resolve before interacting, polling through two + /// cold-start transients that hit the first browser interaction of a + /// freshly launched app: + /// - `load_state` "complete" can fire a beat before the fixture DOM is + /// queryable, so a bare wait races the load; and + /// - the web-content process is still spinning up, so an eval can come + /// back "Completion handler ... no longer reachable" (the handler is + /// dropped when the content process is replaced). Both are transient: + /// re-issue until the element resolves or the deadline passes. + func waitForSelector( + _ selector: String, + surfaceID: String, + timeout: TimeInterval = 15.0, + file: StaticString = #filePath, + line: UInt = #line + ) throws { + try waitForCondition( + ["surface_id": surfaceID, "selector": selector], + describedAs: "selector \(selector)", + timeout: timeout, + file: file, + line: line + ) + } + // MARK: - Read-only page state (browser.eval is for assertions only) func evalValue( @@ -282,10 +481,26 @@ class BrowserFixtureSocketTestCase: XCTestCase { for candidate in self.socketCandidates() { guard FileManager.default.fileExists(atPath: candidate) else { continue } if ControlSocketClient(path: candidate, responseTimeout: 1.0).sendLine("ping") == "PONG" { + NSLog("BrowserFixtureSocketTestCase readiness via in-process client: %@", candidate) + self.socketPath = candidate + return true + } + if self.controlSocketCommandViaNetcat("ping", socketPath: candidate) == "PONG" { + NSLog("BrowserFixtureSocketTestCase readiness via nc fallback: %@", candidate) self.socketPath = candidate return true } } + // The in-process client (and nc) can fail to connect on some + // hosts even while the app's own sanity probe proves the + // listener is bound and answering; accept that probe. + let diagnostics = self.loadDiagnostics() + if self.controlSocketDiagnosticsReportReady(diagnostics), + let expectedPath = diagnostics["socketExpectedPath"], !expectedPath.isEmpty { + NSLog("BrowserFixtureSocketTestCase readiness via app diagnostics only: %@", expectedPath) + self.socketPath = expectedPath + return true + } return false } ) @@ -389,7 +604,11 @@ class BrowserFixtureSocketTestCase: XCTestCase { Darwin.connect(fd, sockaddrPtr, addrLen) } } - guard connected == 0 else { return nil } + guard connected == 0 else { + let err = errno + NSLog("ControlSocketClient connect failed path=%@ errno=%d (%@)", path, err, String(cString: strerror(err))) + return nil + } let payload = Array((line + "\n").utf8) let wrote = payload.withUnsafeBytes { rawBuffer in diff --git a/cmuxUITests/BrowserFixtures/external-open-routing.html b/cmuxUITests/BrowserFixtures/external-open-routing.html new file mode 100644 index 000000000000..fb6d99152af0 --- /dev/null +++ b/cmuxUITests/BrowserFixtures/external-open-routing.html @@ -0,0 +1,14 @@ + + +external-open routing fixture + +

external-open routing fixture

+

matched host link

+

unmatched local link

+

+
+ + +
+ + diff --git a/cmuxUITests/BrowserFixtures/external-open-target.html b/cmuxUITests/BrowserFixtures/external-open-target.html new file mode 100644 index 000000000000..d1c2f936f0bb --- /dev/null +++ b/cmuxUITests/BrowserFixtures/external-open-target.html @@ -0,0 +1,7 @@ + + +external-open target fixture + +

external-open target fixture

+ + From 28adbe143336c14f36bba4ac98e5dc04d7e42eac Mon Sep 17 00:00:00 2001 From: ejc3 Date: Tue, 29 Sep 2026 13:20:05 -0700 Subject: [PATCH 4/6] browser: click links for real in the external-open UI tests A link now leaves for the system browser only while a real input event is in flight, and the socket browser.click runs JavaScript, so the matched and unmatched link cases click through accessibility the way a person does. The scripted popup and form cases keep the socket click, since a scripted action is what they test. --- .../BrowserExternalOpenRoutingUITests.swift | 25 +++++++++++++------ 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/cmuxUITests/BrowserExternalOpenRoutingUITests.swift b/cmuxUITests/BrowserExternalOpenRoutingUITests.swift index a91d06527db4..b6a08a1afcef 100644 --- a/cmuxUITests/BrowserExternalOpenRoutingUITests.swift +++ b/cmuxUITests/BrowserExternalOpenRoutingUITests.swift @@ -5,9 +5,12 @@ import Foundation /// browser. The app launches with a rule matching `127.0.0.1:8397` and with /// `CMUX_UI_TEST_CAPTURE_EXTERNAL_OPEN_PATH` configured, so system-browser /// escapes are written to a capture file instead of launching a real -/// browser. Clicks are delivered through the socket `browser.click` (real -/// WebKit link activations), which is exactly the layer where routing bugs -/// live — matcher unit tests cannot see delegate wiring. The capture line is +/// browser. Link clicks are real mouse clicks through accessibility, because a +/// link only leaves for the system browser while a real input event is in +/// flight; the socket `browser.click` runs JavaScript and would be refused. +/// The scripted-popup and form cases still use the socket, since a scripted +/// action is what they test. Either way this is the delegate layer where +/// routing bugs live — matcher unit tests cannot see that wiring. The capture line is /// written by the shared external-navigation handler's default opener, so /// every escape path (navigation delegate, target=_blank, popups) lands here. final class BrowserExternalOpenRoutingUITests: BrowserFixtureSocketTestCase { @@ -37,6 +40,14 @@ final class BrowserExternalOpenRoutingUITests: BrowserFixtureSocketTestCase { .split(separator: "\n").map(String.init) ?? [] } + /// A real mouse click on a link, so an input event is in flight when WebKit reports the + /// activation, the way it is when a person clicks. + private func clickLink(_ title: String, in app: XCUIApplication) throws { + let link = app.webViews.links[title].firstMatch + XCTAssertTrue(link.waitForExistence(timeout: 10), "link \"\(title)\" must be on screen to click") + link.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)).click() + } + private func waitForCaptureLine( containing needle: String, timeout: TimeInterval = 10.0 @@ -50,10 +61,10 @@ final class BrowserExternalOpenRoutingUITests: BrowserFixtureSocketTestCase { } func testMatchedLinkClickEscapesToSystemBrowser() throws { - try launchApp() + let app = try launchApp() let sid = try openFixture("external-open-routing") try waitForSelector("#matched-link", surfaceID: sid) - try socketResult(method: "browser.click", params: ["surface_id": sid, "selector": "#matched-link"]) + try clickLink("matched host link", in: app) XCTAssertTrue( waitForCaptureLine(containing: "http://127.0.0.1:8397/matched"), "Expected matched link click to escape to the system browser. capture=\(captureLines())" @@ -71,10 +82,10 @@ final class BrowserExternalOpenRoutingUITests: BrowserFixtureSocketTestCase { } func testUnmatchedLinkClickStaysEmbedded() throws { - try launchApp() + let app = try launchApp() let sid = try openFixture("external-open-routing") try waitForSelector("#unmatched-link", surfaceID: sid) - try socketResult(method: "browser.click", params: ["surface_id": sid, "selector": "#unmatched-link"]) + try clickLink("unmatched local link", in: app) try socketResult( method: "browser.wait", params: ["surface_id": sid, "load_state": "complete", "timeout_ms": 10_000], From 8711228e4b3dddee4ea7ddf759b1a18518a0ecee Mon Sep 17 00:00:00 2001 From: ejc3 Date: Sat, 3 Oct 2026 17:23:38 -0700 Subject: [PATCH 5/6] browser: keep only the sidebar links, drop the in-page activation change The in-page half of the external-open rules has landed separately. What remains here is the sidebar: pull-request and port links follow the rules, with tests for the matcher. --- Sources/ContentView.swift | 23 +- .../BrowserExternalNavigationHandler.swift | 65 ++--- .../Panels/BrowserNavigationDelegate.swift | 1 - .../Panels/BrowserNavigationPopupPolicy.swift | 5 +- Sources/Panels/BrowserPanel.swift | 8 +- .../Panels/BrowserPopupWindowController.swift | 4 +- cmux.xcodeproj/project.pbxproj | 4 - cmuxTests/BrowserConfigTests.swift | 163 +++++-------- .../BrowserExternalOpenRoutingUITests.swift | 142 ----------- .../BrowserFixtureInteractionUITests.swift | 229 +----------------- .../external-open-routing.html | 14 -- .../BrowserFixtures/external-open-target.html | 7 - 12 files changed, 95 insertions(+), 570 deletions(-) delete mode 100644 cmuxUITests/BrowserExternalOpenRoutingUITests.swift delete mode 100644 cmuxUITests/BrowserFixtures/external-open-routing.html delete mode 100644 cmuxUITests/BrowserFixtures/external-open-target.html diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index afc4d5fa5cbe..88d453520e75 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -11089,10 +11089,10 @@ struct ContentView: View { if BrowserLinkOpenSettings.openSidebarPullRequestLinksInCmuxBrowser() { let externalNavigationHandler = BrowserExternalNavigationHandler() for pullRequest in pullRequests { - // The external-open rules outrank the embedded-browser - // preference: rule-listed sites cannot work in the embedded - // web view at all. - let openedEmbedded = !externalNavigationHandler.linkEscapesToSystemBrowser(pullRequest.url) + let destination = externalNavigationHandler.sidebarLinkDestination( + for: pullRequest.url, prefersEmbeddedBrowser: true + ) + let openedEmbedded = destination == .embeddedBrowser && tabManager.openBrowser(url: pullRequest.url, insertAtEnd: true) != nil if openedEmbedded || NSWorkspace.shared.open(pullRequest.url) { openedCount += 1 @@ -12753,11 +12753,8 @@ struct VerticalTabsSidebar: View, Equatable { snapshotProvider: { [snapshot = input.workspace] in snapshot } ) let openInBrowser: @MainActor (URL, Bool) -> Void = { [weak tabManager, workspaceId = tab.id] url, preferBrowser in - // The external-open rules outrank the embedded-browser preference - // here just like on the SwiftUI sidebar path: rule-listed sites - // cannot work in the embedded web view at all. - if preferBrowser, - !BrowserExternalNavigationHandler().linkEscapesToSystemBrowser(url), + if BrowserExternalNavigationHandler() + .sidebarLinkDestination(for: url, prefersEmbeddedBrowser: preferBrowser) == .embeddedBrowser, let tabManager, tabManager.openBrowser( inWorkspace: workspaceId, @@ -15000,12 +14997,8 @@ struct VerticalTabsSidebar: View, Equatable { opensInCmuxBrowser: Bool ) { selectWorkspaceRow(workspace, index: index, modifiers: NSEvent.modifierFlags) - // The external-open rules outrank the embedded-browser preference: - // a matching link goes to the system browser even when the setting - // prefers embedded, because rule-listed sites cannot work in the - // embedded web view at all. - if opensInCmuxBrowser, - !BrowserExternalNavigationHandler().linkEscapesToSystemBrowser(url), + if BrowserExternalNavigationHandler() + .sidebarLinkDestination(for: url, prefersEmbeddedBrowser: opensInCmuxBrowser) == .embeddedBrowser, tabManager.openBrowser( inWorkspace: workspace.id, url: url, diff --git a/Sources/Panels/BrowserExternalNavigationHandler.swift b/Sources/Panels/BrowserExternalNavigationHandler.swift index 2a62fe1ca1fc..ce513dbdc535 100644 --- a/Sources/Panels/BrowserExternalNavigationHandler.swift +++ b/Sources/Panels/BrowserExternalNavigationHandler.swift @@ -3,7 +3,6 @@ import AppKit import CmuxBrowser import CmuxCore import CmuxSettings -import CmuxTestSupport import Foundation import WebKit @@ -24,29 +23,13 @@ struct BrowserExternalNavigationHandler { init( defaults: UserDefaults = .standard, - openURL: @escaping @MainActor @Sendable (URL) -> Bool = Self.openInSystemBrowser + openURL: @escaping @MainActor @Sendable (URL) -> Bool = { NSWorkspace.shared.open($0) } ) { self.defaults = defaults self.openURL = openURL self.policyCache = BrowserExternalURLPolicyCache(defaults: defaults) } - /// The default opener. UI tests observe external-open routing through the - /// capture sink; a configured sink intercepts the open so CI never - /// launches a real browser. - @MainActor - private static func openInSystemBrowser(_ url: URL) -> Bool { -#if DEBUG - if UITestCaptureSink().appendLineIfConfigured( - envKey: "CMUX_UI_TEST_CAPTURE_EXTERNAL_OPEN_PATH", - line: "externalOpen \(url.absoluteString)" - ) { - return true - } -#endif - return NSWorkspace.shared.open(url) - } - /// Returns whether a URL matches a configured external rule. func shouldOpenExternally(_ url: URL) -> Bool { let externalURL = canonicalURL(for: url) @@ -73,35 +56,31 @@ struct BrowserExternalNavigationHandler { return policyCache.currentPolicy().matches(target) } - /// True when a link the user chose outside a web view (a sidebar - /// pull-request or port link) should bypass the embedded browser and go - /// to the system browser. Restricted to web schemes; other schemes have - /// their own external-open routing. - func linkEscapesToSystemBrowser(_ url: URL) -> Bool { - guard Self.isWebNavigationURL(url) else { return false } - return shouldOpenExternally(url) + /// Where a pull-request or port link chosen in the sidebar opens. + enum SidebarLinkDestination: Equatable { + case embeddedBrowser + case systemBrowser + } + + /// The destination for a link the user chose in the sidebar. + /// `prefersEmbeddedBrowser` is the "open sidebar links in the cmux browser" + /// preference. A matching external-open rule outranks it: a site listed there + /// cannot work in the embedded web view at all. The rules are about web pages, + /// so other schemes keep following the preference. + func sidebarLinkDestination(for url: URL, prefersEmbeddedBrowser: Bool) -> SidebarLinkDestination { + guard prefersEmbeddedBrowser else { return .systemBrowser } + if Self.isWebNavigationURL(url), shouldOpenExternally(url) { return .systemBrowser } + return .embeddedBrowser } /// Returns whether a user-activated main-frame navigation should be external. - /// - /// Downloads keep the download flow. WebKit reports a script calling - /// `click()` on an anchor as `.linkActivated`, the same as a real click, - /// so the rules on their own would let a page hand itself a system-browser - /// open at a moment of its choosing; requiring an AppKit event in flight - /// makes the page ride a click the user actually made. It is a bound - /// rather than a proof — `NSApp.currentEvent` says an event is being - /// dispatched, not that this navigation is the thing the user asked for. func shouldOpenExternally( _ url: URL, navigationType: WKNavigationType, - targetFrameIsMain: Bool?, - shouldPerformDownload: Bool = false, - hasUserActivation: Bool = browserNavigationHasSimpleUserActivation() + targetFrameIsMain: Bool? ) -> Bool { guard navigationType == .linkActivated, targetFrameIsMain != false, - !shouldPerformDownload, - hasUserActivation, Self.isWebNavigationURL(url), !Self.isAppOwnedInternalURL(url) else { return false @@ -177,16 +156,12 @@ struct BrowserExternalNavigationHandler { _ url: URL, navigationType: WKNavigationType, targetFrameIsMain: Bool?, - shouldPerformDownload: Bool = false, - hasUserActivation: Bool = browserNavigationHasSimpleUserActivation(), onOpened: @escaping @MainActor () -> Void = {} ) -> Bool { if case .opened = openConfiguredExternallyResult( url, navigationType: navigationType, targetFrameIsMain: targetFrameIsMain, - shouldPerformDownload: shouldPerformDownload, - hasUserActivation: hasUserActivation, onOpened: onOpened ) { return true @@ -200,17 +175,13 @@ struct BrowserExternalNavigationHandler { _ url: URL, navigationType: WKNavigationType, targetFrameIsMain: Bool?, - shouldPerformDownload: Bool = false, - hasUserActivation: Bool = browserNavigationHasSimpleUserActivation(), onOpened: @escaping @MainActor () -> Void = {} ) -> OpenResult { let externalURL = canonicalURL(for: url) guard shouldOpenExternally( externalURL, navigationType: navigationType, - targetFrameIsMain: targetFrameIsMain, - shouldPerformDownload: shouldPerformDownload, - hasUserActivation: hasUserActivation + targetFrameIsMain: targetFrameIsMain ) else { return .notConfigured } diff --git a/Sources/Panels/BrowserNavigationDelegate.swift b/Sources/Panels/BrowserNavigationDelegate.swift index 6ba73a336069..f98bd25c9d0e 100644 --- a/Sources/Panels/BrowserNavigationDelegate.swift +++ b/Sources/Panels/BrowserNavigationDelegate.swift @@ -385,7 +385,6 @@ import WebKit url, navigationType: navigationAction.navigationType, targetFrameIsMain: navigationAction.targetFrame?.isMainFrame, - shouldPerformDownload: navigationAction.shouldPerformDownload, onOpened: { [self] in clearAttemptedRequest(discardPendingBypasses: true) let reportTerminalCancellation = terminalPolicyCancellationReporter?( diff --git a/Sources/Panels/BrowserNavigationPopupPolicy.swift b/Sources/Panels/BrowserNavigationPopupPolicy.swift index 8a58360e3d28..e5d491a475c0 100644 --- a/Sources/Panels/BrowserNavigationPopupPolicy.swift +++ b/Sources/Panels/BrowserNavigationPopupPolicy.swift @@ -67,10 +67,7 @@ func browserNavigationHasSimpleUserActivation( currentEventType: NSEvent.EventType? = NSApp.currentEvent?.type ) -> Bool { switch currentEventType { - case .keyDown, .keyUp, .leftMouseDown, .leftMouseUp, - .otherMouseDown, .otherMouseUp: - // Middle-clicks arrive as otherMouse events and are user input the - // same as a left click, so a matched link still escapes on them. + case .keyDown, .keyUp, .leftMouseDown, .leftMouseUp: return true default: return false diff --git a/Sources/Panels/BrowserPanel.swift b/Sources/Panels/BrowserPanel.swift index 16a7ba8b10e0..1923707274c4 100644 --- a/Sources/Panels/BrowserPanel.swift +++ b/Sources/Panels/BrowserPanel.swift @@ -6371,14 +6371,11 @@ extension BrowserPanel { } /// Routes the context-menu tab action through configured external rules. - /// Choosing a menu item is the user's own gesture, so the user-event - /// guard is satisfied by construction here. func openContextMenuLinkInNewTab(url: URL) { switch externalNavigationHandler.openConfiguredExternallyResult( url, navigationType: .linkActivated, - targetFrameIsMain: true, - hasUserActivation: true + targetFrameIsMain: true ) { case .opened: return @@ -8673,8 +8670,7 @@ final class BrowserUIDelegate: BrowserPDFPreviewActionUIDelegate { switch externalNavigationHandler.openConfiguredExternallyResult( url, navigationType: navigationAction.navigationType, - targetFrameIsMain: navigationAction.targetFrame?.isMainFrame, - shouldPerformDownload: navigationAction.shouldPerformDownload + targetFrameIsMain: navigationAction.targetFrame?.isMainFrame ) { case .opened: return nil diff --git a/Sources/Panels/BrowserPopupWindowController.swift b/Sources/Panels/BrowserPopupWindowController.swift index 2b245cfeb02a..1f3663e8931b 100644 --- a/Sources/Panels/BrowserPopupWindowController.swift +++ b/Sources/Panels/BrowserPopupWindowController.swift @@ -504,8 +504,7 @@ private final class PopupUIDelegate: BrowserPDFPreviewActionUIDelegate { switch externalNavigationHandler.openConfiguredExternallyResult( url, navigationType: navigationAction.navigationType, - targetFrameIsMain: navigationAction.targetFrame?.isMainFrame, - shouldPerformDownload: navigationAction.shouldPerformDownload + targetFrameIsMain: navigationAction.targetFrame?.isMainFrame ) { case .opened: return nil @@ -802,7 +801,6 @@ private final class PopupUIDelegate: BrowserPDFPreviewActionUIDelegate { url, navigationType: navigationAction.navigationType, targetFrameIsMain: navigationAction.targetFrame?.isMainFrame, - shouldPerformDownload: navigationAction.shouldPerformDownload, onOpened: { [self] in clearAttemptedRequest(discardPendingBypasses: true) } diff --git a/cmux.xcodeproj/project.pbxproj b/cmux.xcodeproj/project.pbxproj index 2b3c9017f6fe..265a26a97e82 100644 --- a/cmux.xcodeproj/project.pbxproj +++ b/cmux.xcodeproj/project.pbxproj @@ -446,7 +446,6 @@ B5A5E55B0000000000000001 /* BrowserEphemeralRestorationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B5A5E55B0000000000000002 /* BrowserEphemeralRestorationTests.swift */; }; C2035A010000000000000001 /* BrowserErrorPage.swift in Sources */ = {isa = PBXBuildFile; fileRef = C2035A010000000000000002 /* BrowserErrorPage.swift */; }; BEEA00010000000000000001 /* BrowserExternalNavigationHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = BEEA00010000000000000002 /* BrowserExternalNavigationHandler.swift */; }; - B90000E2A1B2C3D4E5F60719 /* BrowserExternalOpenRoutingUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B90000E1A1B2C3D4E5F60719 /* BrowserExternalOpenRoutingUITests.swift */; }; 129980000000000000000001 /* BrowserFailedNavigationReloadTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 129980000000000000000002 /* BrowserFailedNavigationReloadTests.swift */; }; D7632A11A1B2C3D4E5F60718 /* BrowserFileDropNavigationGuard.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7632A12A1B2C3D4E5F60718 /* BrowserFileDropNavigationGuard.swift */; }; A5008381 /* BrowserFindJavaScriptTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A5008380 /* BrowserFindJavaScriptTests.swift */; }; @@ -4974,7 +4973,6 @@ B5A5E55B0000000000000002 /* BrowserEphemeralRestorationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserEphemeralRestorationTests.swift; sourceTree = ""; }; C2035A010000000000000002 /* BrowserErrorPage.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserErrorPage.swift; sourceTree = ""; }; BEEA00010000000000000002 /* BrowserExternalNavigationHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Panels/BrowserExternalNavigationHandler.swift; sourceTree = ""; }; - B90000E1A1B2C3D4E5F60719 /* BrowserExternalOpenRoutingUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserExternalOpenRoutingUITests.swift; sourceTree = ""; }; 129980000000000000000002 /* BrowserFailedNavigationReloadTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserFailedNavigationReloadTests.swift; sourceTree = ""; }; D7632A12A1B2C3D4E5F60718 /* BrowserFileDropNavigationGuard.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserFileDropNavigationGuard.swift; sourceTree = ""; }; A5008380 /* BrowserFindJavaScriptTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BrowserFindJavaScriptTests.swift; sourceTree = ""; }; @@ -9165,7 +9163,6 @@ D0E0F0B3A1B2C3D4E5F60718 /* BrowserOmnibarSuggestionsUITests.swift */, D93410000000000000000002 /* BrowserDownloadsPopoverContrastUITests.swift */, FB100001A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift */, - B90000E1A1B2C3D4E5F60719 /* BrowserExternalOpenRoutingUITests.swift */, 7B5F1A2E9C0D4B6A8E217301 /* BrowserFixtureInteractionUITests.swift */, 81464EB0D515022903F26DD8 /* WorkspaceWorkingDirectorySpawnUITests.swift */, 8478B0000000000000000001 /* BrowserFixtureSocketTestCase+PendingRequest.swift */, @@ -17037,7 +17034,6 @@ B9000012A1B2C3D4E5F60719 /* AutomationSocketUITests.swift in Sources */, AA1B2C3D4E5F60718 /* BonsplitTabDragUITests.swift in Sources */, D93410000000000000000001 /* BrowserDownloadsPopoverContrastUITests.swift in Sources */, - B90000E2A1B2C3D4E5F60719 /* BrowserExternalOpenRoutingUITests.swift in Sources */, 7B5F1A2E9C0D4B6A8E217302 /* BrowserFixtureInteractionUITests.swift in Sources */, 8478B0000000000000000002 /* BrowserFixtureSocketTestCase+PendingRequest.swift in Sources */, FB100000A1B2C3D4E5F60718 /* BrowserImportProfilesUITests.swift in Sources */, diff --git a/cmuxTests/BrowserConfigTests.swift b/cmuxTests/BrowserConfigTests.swift index 79ce1792530b..80de9f1ff28e 100644 --- a/cmuxTests/BrowserConfigTests.swift +++ b/cmuxTests/BrowserConfigTests.swift @@ -5614,6 +5614,58 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { XCTAssertTrue(BrowserLinkOpenSettings.initialInterceptTerminalOpenCommandInCmuxBrowserValue(defaults: defaults)) } + // MARK: - Sidebar links + + /// A pull-request or port link chosen in the sidebar follows the "open in the + /// cmux browser" preference when no rule names its site. + func testSidebarLinkWithNoMatchingRuleFollowsThePreference() throws { + defaults.set("billing.example.com", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) + let handler = BrowserExternalNavigationHandler(defaults: defaults) + let url = try XCTUnwrap(URL(string: "https://github.com/manaflow-ai/cmux/pull/1")) + XCTAssertEqual(handler.sidebarLinkDestination(for: url, prefersEmbeddedBrowser: true), .embeddedBrowser) + XCTAssertEqual(handler.sidebarLinkDestination(for: url, prefersEmbeddedBrowser: false), .systemBrowser) + } + + /// A site listed in the external-open rules cannot work in the embedded web + /// view, so the rule outranks the preference for sidebar links too. + func testSidebarLinkMatchingAnExternalRuleGoesToTheSystemBrowser() throws { + defaults.set("github.example.com", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) + let handler = BrowserExternalNavigationHandler(defaults: defaults) + let pullRequest = try XCTUnwrap(URL(string: "https://github.example.com/org/repo/pull/42")) + XCTAssertEqual( + handler.sidebarLinkDestination(for: pullRequest, prefersEmbeddedBrowser: true), + .systemBrowser + ) + XCTAssertEqual( + handler.sidebarLinkDestination(for: pullRequest, prefersEmbeddedBrowser: false), + .systemBrowser + ) + } + + /// The same holds for a port link, which is a plain http URL on a host. + func testSidebarPortLinkMatchingAnExternalRuleGoesToTheSystemBrowser() throws { + defaults.set( + "re:^https?://dashboard\\.example\\.com(:[0-9]+)?/", + forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey + ) + let handler = BrowserExternalNavigationHandler(defaults: defaults) + let port = try XCTUnwrap(URL(string: "http://dashboard.example.com:8080/")) + let other = try XCTUnwrap(URL(string: "http://localhost:8080/")) + XCTAssertEqual(handler.sidebarLinkDestination(for: port, prefersEmbeddedBrowser: true), .systemBrowser) + XCTAssertEqual(handler.sidebarLinkDestination(for: other, prefersEmbeddedBrowser: true), .embeddedBrowser) + } + + /// The rules are about web pages. A link with another scheme keeps following + /// the preference even when a rule's text happens to match it. + func testSidebarLinkRuleAppliesOnlyToWebSchemes() throws { + defaults.set("example.com", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) + let handler = BrowserExternalNavigationHandler(defaults: defaults) + let web = try XCTUnwrap(URL(string: "https://example.com/pull/7")) + let notWeb = try XCTUnwrap(URL(string: "ssh://example.com/repo")) + XCTAssertEqual(handler.sidebarLinkDestination(for: web, prefersEmbeddedBrowser: true), .systemBrowser) + XCTAssertEqual(handler.sidebarLinkDestination(for: notWeb, prefersEmbeddedBrowser: true), .embeddedBrowser) + } + func testExternalOpenPatternsDefaultToEmpty() { XCTAssertTrue(BrowserExternalURLPolicy(defaults: defaults).patterns.isEmpty) } @@ -5708,8 +5760,7 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { handler.shouldOpenExternally( aliasedURL, navigationType: .linkActivated, - targetFrameIsMain: true, - hasUserActivation: true + targetFrameIsMain: true ) ) XCTAssertEqual(handler.openConfiguredExternallyResult(aliasedURL), .opened) @@ -5765,24 +5816,21 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( url, navigationType: .linkActivated, - targetFrameIsMain: true, - hasUserActivation: true + targetFrameIsMain: true ) ) XCTAssertFalse( BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( url, navigationType: .other, - targetFrameIsMain: true, - hasUserActivation: true + targetFrameIsMain: true ) ) XCTAssertFalse( BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( url, navigationType: .linkActivated, - targetFrameIsMain: false, - hasUserActivation: true + targetFrameIsMain: false ) ) let callbackURL = try XCTUnwrap( @@ -5793,8 +5841,7 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( callbackURL, navigationType: .linkActivated, - targetFrameIsMain: true, - hasUserActivation: true + targetFrameIsMain: true ) ) let siblingCallbackURL = try XCTUnwrap( @@ -5804,8 +5851,7 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( siblingCallbackURL, navigationType: .linkActivated, - targetFrameIsMain: true, - hasUserActivation: true + targetFrameIsMain: true ) ) XCTAssertFalse( @@ -5824,8 +5870,7 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( diffViewerURL, navigationType: .linkActivated, - targetFrameIsMain: true, - hasUserActivation: true + targetFrameIsMain: true ) ) let customAppURL = try XCTUnwrap(URL(string: "slack://open?token=secret")) @@ -5833,8 +5878,7 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { BrowserExternalNavigationHandler(defaults: defaults).shouldOpenExternally( customAppURL, navigationType: .linkActivated, - targetFrameIsMain: true, - hasUserActivation: true + targetFrameIsMain: true ), "Configured browser rules must not bypass the existing custom-scheme confirmation prompt." ) @@ -5859,7 +5903,6 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { url, navigationType: .linkActivated, targetFrameIsMain: true, - hasUserActivation: true, onOpened: { didRunAfterOpen = true } @@ -5883,92 +5926,6 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { XCTAssertEqual(result, .failed) } - - func testExternalOpenDomainPatternCoversSubdomainURLs() throws { - defaults.set("corp.example", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) - let handler = BrowserExternalNavigationHandler(defaults: defaults) - XCTAssertTrue(handler.shouldOpenExternally(try XCTUnwrap(URL(string: "https://corp.example/")))) - XCTAssertTrue(handler.shouldOpenExternally(try XCTUnwrap(URL(string: "https://sso.corp.example/login")))) - XCTAssertTrue(handler.shouldOpenExternally(try XCTUnwrap(URL(string: "https://CORP.EXAMPLE/tools")))) - XCTAssertFalse(handler.shouldOpenExternally(try XCTUnwrap(URL(string: "https://unrelated.example/")))) - } - - func testNavigationEscapeRequiresUserActivation() throws { - defaults.set("corp.example", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) - let url = try XCTUnwrap(URL(string: "https://sso.corp.example/")) - let handler = BrowserExternalNavigationHandler(defaults: defaults) - // A page can call click() on an anchor and WebKit still reports - // .linkActivated, so the rules alone are not enough to let it escape. - XCTAssertFalse( - handler.shouldOpenExternally( - url, - navigationType: .linkActivated, - targetFrameIsMain: true, - shouldPerformDownload: false, - hasUserActivation: false - ) - ) - XCTAssertTrue( - handler.shouldOpenExternally( - url, - navigationType: .linkActivated, - targetFrameIsMain: true, - shouldPerformDownload: false, - hasUserActivation: true - ) - ) - XCTAssertEqual( - handler.openConfiguredExternallyResult( - url, - navigationType: .linkActivated, - targetFrameIsMain: true, - hasUserActivation: false - ), - .notConfigured - ) - } - - func testNavigationEscapeStillRejectsDownloadsAndNonLinkNavigations() throws { - defaults.set("corp.example", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) - let url = try XCTUnwrap(URL(string: "https://sso.corp.example/")) - let handler = BrowserExternalNavigationHandler(defaults: defaults) - XCTAssertFalse( - handler.shouldOpenExternally( - url, - navigationType: .linkActivated, - targetFrameIsMain: true, - shouldPerformDownload: true, - hasUserActivation: true - ) - ) - XCTAssertFalse( - handler.shouldOpenExternally( - url, - navigationType: .other, - targetFrameIsMain: true, - shouldPerformDownload: false, - hasUserActivation: true - ) - ) - } - - func testSimpleUserActivationTracksTheEventInFlight() { - XCTAssertTrue(browserNavigationHasSimpleUserActivation(currentEventType: .leftMouseUp)) - XCTAssertTrue(browserNavigationHasSimpleUserActivation(currentEventType: .keyDown)) - XCTAssertTrue(browserNavigationHasSimpleUserActivation(currentEventType: .otherMouseDown)) - XCTAssertTrue(browserNavigationHasSimpleUserActivation(currentEventType: .otherMouseUp)) - XCTAssertFalse(browserNavigationHasSimpleUserActivation(currentEventType: nil)) - XCTAssertFalse(browserNavigationHasSimpleUserActivation(currentEventType: .mouseMoved)) - } - - func testSidebarLinkEscapesOnlyForWebSchemes() throws { - defaults.set("corp.example", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) - let handler = BrowserExternalNavigationHandler(defaults: defaults) - XCTAssertTrue(handler.linkEscapesToSystemBrowser(try XCTUnwrap(URL(string: "https://sso.corp.example/")))) - XCTAssertFalse(handler.linkEscapesToSystemBrowser(try XCTUnwrap(URL(string: "file:///tmp/corp.example.html")))) - defaults.removeObject(forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey) - XCTAssertFalse(handler.linkEscapesToSystemBrowser(try XCTUnwrap(URL(string: "https://corp.example/")))) - } } diff --git a/cmuxUITests/BrowserExternalOpenRoutingUITests.swift b/cmuxUITests/BrowserExternalOpenRoutingUITests.swift deleted file mode 100644 index b6a08a1afcef..000000000000 --- a/cmuxUITests/BrowserExternalOpenRoutingUITests.swift +++ /dev/null @@ -1,142 +0,0 @@ -import XCTest -import Foundation - -/// End-to-end coverage for external-open rule routing in the embedded -/// browser. The app launches with a rule matching `127.0.0.1:8397` and with -/// `CMUX_UI_TEST_CAPTURE_EXTERNAL_OPEN_PATH` configured, so system-browser -/// escapes are written to a capture file instead of launching a real -/// browser. Link clicks are real mouse clicks through accessibility, because a -/// link only leaves for the system browser while a real input event is in -/// flight; the socket `browser.click` runs JavaScript and would be refused. -/// The scripted-popup and form cases still use the socket, since a scripted -/// action is what they test. Either way this is the delegate layer where -/// routing bugs live — matcher unit tests cannot see that wiring. The capture line is -/// written by the shared external-navigation handler's default opener, so -/// every escape path (navigation delegate, target=_blank, popups) lands here. -final class BrowserExternalOpenRoutingUITests: BrowserFixtureSocketTestCase { - private var capturePath = "" - - override var extraLaunchArguments: [String] { - ["-browserExternalOpenPatterns", "127.0.0.1:8397"] - } - - override var extraLaunchEnvironment: [String: String] { - ["CMUX_UI_TEST_CAPTURE_EXTERNAL_OPEN_PATH": capturePath] - } - - override func setUp() { - super.setUp() - capturePath = "/tmp/cmux-ui-test-external-open-\(UUID().uuidString).log" - try? FileManager.default.removeItem(atPath: capturePath) - } - - override func tearDown() { - try? FileManager.default.removeItem(atPath: capturePath) - super.tearDown() - } - - private func captureLines() -> [String] { - (try? String(contentsOfFile: capturePath, encoding: .utf8))? - .split(separator: "\n").map(String.init) ?? [] - } - - /// A real mouse click on a link, so an input event is in flight when WebKit reports the - /// activation, the way it is when a person clicks. - private func clickLink(_ title: String, in app: XCUIApplication) throws { - let link = app.webViews.links[title].firstMatch - XCTAssertTrue(link.waitForExistence(timeout: 10), "link \"\(title)\" must be on screen to click") - link.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)).click() - } - - private func waitForCaptureLine( - containing needle: String, - timeout: TimeInterval = 10.0 - ) -> Bool { - let deadline = Date().addingTimeInterval(timeout) - while Date() < deadline { - if captureLines().contains(where: { $0.contains(needle) }) { return true } - RunLoop.current.run(until: Date().addingTimeInterval(0.2)) - } - return false - } - - func testMatchedLinkClickEscapesToSystemBrowser() throws { - let app = try launchApp() - let sid = try openFixture("external-open-routing") - try waitForSelector("#matched-link", surfaceID: sid) - try clickLink("matched host link", in: app) - XCTAssertTrue( - waitForCaptureLine(containing: "http://127.0.0.1:8397/matched"), - "Expected matched link click to escape to the system browser. capture=\(captureLines())" - ) - XCTAssertTrue( - captureLines().contains { $0.hasPrefix("externalOpen ") }, - "Expected the escape to route through the external-navigation handler. capture=\(captureLines())" - ) - // The embedded page must not have navigated to the matched URL. - let href = try evalString("location.href", surfaceID: sid) - XCTAssertTrue( - href.hasSuffix("external-open-routing.html"), - "Embedded page navigated away after an escape: \(href)" - ) - } - - func testUnmatchedLinkClickStaysEmbedded() throws { - let app = try launchApp() - let sid = try openFixture("external-open-routing") - try waitForSelector("#unmatched-link", surfaceID: sid) - try clickLink("unmatched local link", in: app) - try socketResult( - method: "browser.wait", - params: ["surface_id": sid, "load_state": "complete", "timeout_ms": 10_000], - responseTimeout: 16.0 - ) - let href = try evalString("location.href", surfaceID: sid) - XCTAssertTrue( - href.hasSuffix("external-open-target.html"), - "Expected unmatched link to navigate embedded, got: \(href)" - ) - XCTAssertEqual(captureLines(), [], "Unmatched navigation must not touch the system browser") - } - - func testScriptedWindowOpenToMatchedURLNeverEscapes() throws { - try launchApp() - let sid = try openFixture("external-open-routing") - try waitForSelector("#popup-button", surfaceID: sid) - try socketResult(method: "browser.click", params: ["surface_id": sid, "selector": "#popup-button"]) - // Affirmative: window.open must actually open an embedded tab (proves it - // fired and stayed embedded, not that it silently no-op'd), and it must - // not reach the system browser. - let popupOpenedEmbedded = try waitForEmbeddedTab(urlContaining: "127.0.0.1:8397") - let tabsAfterPopup = try openTabURLs() - XCTAssertTrue( - popupOpenedEmbedded, - "Scripted window.open should open an embedded tab. tabs=\(tabsAfterPopup)" - ) - XCTAssertFalse( - captureLines().contains { $0.contains("/popup") }, - "Scripted window.open must never reach the system browser. capture=\(captureLines())" - ) - } - - func testFormPostTargetBlankToMatchedHostStaysEmbedded() throws { - try launchApp() - let sid = try openFixture("external-open-routing") - try waitForSelector("#post-submit", surfaceID: sid) - try socketResult(method: "browser.click", params: ["surface_id": sid, "selector": "#post-submit"]) - // Affirmative: the POST must open an embedded tab at the matched host - // (proves the submission happened and stayed embedded, keeping its body - // rather than being dropped or re-issued as a GET in the system browser), - // and it must not reach the system browser. - let postOpenedEmbedded = try waitForEmbeddedTab(urlContaining: "127.0.0.1:8397") - let tabsAfterPost = try openTabURLs() - XCTAssertTrue( - postOpenedEmbedded, - "target=_blank form POST should open an embedded tab. tabs=\(tabsAfterPost)" - ) - XCTAssertFalse( - captureLines().contains { $0.contains("/post") }, - "A form POST must keep its body embedded, never escape as a GET. capture=\(captureLines())" - ) - } -} diff --git a/cmuxUITests/BrowserFixtureInteractionUITests.swift b/cmuxUITests/BrowserFixtureInteractionUITests.swift index 83bcb5b5be63..fe977ad45238 100644 --- a/cmuxUITests/BrowserFixtureInteractionUITests.swift +++ b/cmuxUITests/BrowserFixtureInteractionUITests.swift @@ -19,9 +19,6 @@ class BrowserFixtureSocketTestCase: XCTestCase { private var diagnosticsPath = "" private var launchTag = "" private(set) var app: XCUIApplication? - /// Workspace id of the most recent `openBrowserSurface`/`openFixture`, so - /// tests can enumerate its tabs via `browser.tab.list`. - private(set) var lastWorkspaceID = "" override func setUp() { super.setUp() @@ -45,11 +42,6 @@ class BrowserFixtureSocketTestCase: XCTestCase { // MARK: - Launch - /// Subclass hooks for extra launch configuration (defaults registered via - /// NSArgumentDomain, capture-sink env keys, ...). - var extraLaunchArguments: [String] { [] } - var extraLaunchEnvironment: [String: String] { [:] } - @discardableResult func launchApp(additionalLaunchArguments: [String] = []) throws -> XCUIApplication { let app = XCUIApplication.cmuxTestApplication() @@ -58,7 +50,6 @@ class BrowserFixtureSocketTestCase: XCTestCase { "-AppleLanguages", "(en)", "-AppleLocale", "en_US", ] + additionalLaunchArguments - app.launchArguments += extraLaunchArguments app.launchEnvironment["CMUX_UI_TEST_MODE"] = "1" app.launchEnvironment["CMUX_SOCKET_ENABLE"] = "1" app.launchEnvironment["CMUX_SOCKET_MODE"] = "allowAll" @@ -66,20 +57,12 @@ class BrowserFixtureSocketTestCase: XCTestCase { app.launchEnvironment["CMUX_ALLOW_SOCKET_OVERRIDE"] = "1" app.launchEnvironment["CMUX_UI_TEST_SOCKET_SANITY"] = "1" app.launchEnvironment["CMUX_UI_TEST_DIAGNOSTICS_PATH"] = diagnosticsPath - // The UI-test host is frequently backgrounded (activation is best-effort - // on CI, see below), which marks the browser split "hidden" and lets the - // hidden-webview discarder tear down the WKWebView mid-eval — surfacing - // as "completion handler no longer reachable". Keep it alive for tests. - app.launchEnvironment["CMUX_BROWSER_HIDDEN_WEBVIEW_DISCARD_ENABLED"] = "false" // Debug launches require a tag outside reload.sh; provide one in UITests so CI // does not fail with "Application ... does not have a process ID". app.launchEnvironment["CMUX_TAG"] = launchTag if let path = ProcessInfo.processInfo.environment["PATH"], !path.isEmpty { app.launchEnvironment["PATH"] = path } - for (key, value) in extraLaunchEnvironment { - app.launchEnvironment[key] = value - } self.app = app // On headless CI runners (no GUI session), XCUIApplication.launch() // blocks ~60s then fails with "Failed to activate application @@ -109,12 +92,6 @@ class BrowserFixtureSocketTestCase: XCTestCase { /// Sends one V2 request and returns the raw response envelope /// (`{"id":…,"ok":…,"result"/"error":…}`), or nil if the socket did not answer. - /// Falls back to the `nc -U` transport when the in-process Darwin client - /// cannot connect (see `controlSocketCommandViaNetcat`), matching - /// `BrowserPaneNavigationKeybindUITests`, and finally to the bundled CLI - /// binary: some hosts refuse unix-socket connects from the UI-test runner - /// and its child processes entirely, while a full `cmux rpc` invocation - /// (the same client real users script with) connects fine. func socketEnvelope( method: String, params: [String: Any], @@ -126,82 +103,6 @@ class BrowserFixtureSocketTestCase: XCTestCase { "params": params, ] return ControlSocketClient(path: socketPath, responseTimeout: responseTimeout).sendJSON(request) - ?? controlSocketJSONViaNetcat(request, socketPath: socketPath, responseTimeout: responseTimeout) - ?? socketEnvelopeViaBundledCLI(method: method, params: params, responseTimeout: responseTimeout) - } - - /// Runs `cmux rpc ` from the app bundle the UI test built, - /// pointed at this test's socket. Returns a synthesized success envelope - /// (the CLI exits non-zero on `ok: false` responses and transport errors). - private func socketEnvelopeViaBundledCLI( - method: String, - params: [String: Any], - responseTimeout: TimeInterval - ) -> [String: Any]? { - guard let cli = Self.bundledCLIPath(), - JSONSerialization.isValidJSONObject(params), - let paramsData = try? JSONSerialization.data(withJSONObject: params), - let paramsJSON = String(data: paramsData, encoding: .utf8) else { - return nil - } - let process = Process() - process.executableURL = URL(fileURLWithPath: cli) - process.arguments = ["rpc", method, paramsJSON] - var environment = ProcessInfo.processInfo.environment - for key in environment.keys where key.hasPrefix("CMUX_") { - environment.removeValue(forKey: key) - } - environment["CMUX_SOCKET_PATH"] = socketPath - // Honor the caller's timeout budget on the fallback path too; the CLI - // otherwise uses its own default (CLI/cmux.swift responseTimeoutSeconds). - environment["CMUXTERM_CLI_RESPONSE_TIMEOUT_SEC"] = String(max(1.0, responseTimeout)) - process.environment = environment - let stdout = Pipe() - let stderr = Pipe() - process.standardOutput = stdout - process.standardError = stderr - do { - try process.run() - } catch { - return nil - } - // Drain stderr concurrently and read stdout before waiting on exit, so the - // CLI can't block on a full pipe buffer and hang the test run. - let stderrDrain = DispatchQueue(label: "cmux-ui-test-cli-stderr-drain") - stderrDrain.async { _ = stderr.fileHandleForReading.readDataToEndOfFile() } - let data = stdout.fileHandleForReading.readDataToEndOfFile() - process.waitUntilExit() - guard process.terminationStatus == 0 else { return nil } - guard let result = (try? JSONSerialization.jsonObject(with: data)) as? [String: Any] else { - return nil - } - return ["ok": true, "result": result] - } - - /// Locates `Contents/Resources/bin/cmux` inside the app the test target - /// built, starting from the test bundle's products directory. - private static func bundledCLIPath() -> String? { - // …/Build/Products/Debug/cmuxUITests-Runner.app/Contents/PlugIns/cmuxUITests.xctest - // Walk ancestors instead of counting components: the runner nests the - // test bundle one level deeper (Contents/) than a flat reading of the - // path suggests, and a fixed hop count silently lands inside the - // runner app, never finding the sibling cmux app. - var candidate = URL(fileURLWithPath: Bundle(for: BrowserFixtureSocketTestCase.self).bundlePath) - for _ in 0..<6 { - candidate.deleteLastPathComponent() - guard !candidate.path.hasSuffix(".app"), candidate.path != "/" else { continue } - let entries = (try? FileManager.default.contentsOfDirectory(atPath: candidate.path)) ?? [] - for entry in entries where entry.hasSuffix(".app") { - let cli = candidate - .appendingPathComponent(entry) - .appendingPathComponent("Contents/Resources/bin/cmux") - .path - if FileManager.default.isExecutableFile(atPath: cli) { - return cli - } - } - } - return nil } /// Sends one V2 request, asserts `ok == true`, and returns `result`. @@ -257,7 +158,6 @@ class BrowserFixtureSocketTestCase: XCTestCase { file: file, line: line ) - lastWorkspaceID = workspaceID let sourceSurfaceID = try XCTUnwrap( workspace["surface_id"] as? String, "workspace.create returned no surface_id: \(workspace)", @@ -301,115 +201,16 @@ class BrowserFixtureSocketTestCase: XCTestCase { file: file, line: line ) - // Assert readiness, but poll through the cold-start content-process - // transient (an eval can return "completion handler no longer - // reachable" while the web content process respawns on a fresh app), - // so this keeps its diagnostic value for callers that act right after - // openFixture without a separate waitForSelector gate. - try waitForCondition( - ["surface_id": surfaceID, "load_state": "complete"], - describedAs: "load_state=complete", + try socketResult( + method: "browser.wait", + params: ["surface_id": surfaceID, "load_state": "complete", "timeout_ms": 10_000], + responseTimeout: 16.0, file: file, line: line ) return surfaceID } - /// Polls a `browser.wait` condition through the transient js_error the - /// first browser interaction of a freshly launched app can hit, failing - /// only when the deadline passes with the condition still unmet. - func waitForCondition( - _ waitParams: [String: Any], - describedAs description: String, - timeout: TimeInterval = 15.0, - file: StaticString = #filePath, - line: UInt = #line - ) throws { - let deadline = Date().addingTimeInterval(timeout) - var params = waitParams - params["timeout_ms"] = 3_000 - var lastEnvelope: [String: Any]? - while Date() < deadline { - let envelope = socketEnvelope(method: "browser.wait", params: params, responseTimeout: 6.0) - lastEnvelope = envelope - if (envelope?["ok"] as? Bool) == true { return } - RunLoop.current.run(until: Date().addingTimeInterval(0.3)) - } - XCTFail( - "browser.wait(\(description)) timed out. last=\(String(describing: lastEnvelope))", - file: file, - line: line - ) - } - - /// Returns the tab URLs currently open in `lastWorkspaceID`. - func openTabURLs(file: StaticString = #filePath, line: UInt = #line) throws -> [String] { - let result = try socketResult( - method: "browser.tab.list", - params: ["workspace_id": lastWorkspaceID], - file: file, - line: line - ) - let tabs = result["tabs"] as? [[String: Any]] ?? [] - return tabs.compactMap { $0["url"] as? String } - } - - /// Polls until a workspace tab's URL contains `needle`, proving an embedded - /// tab was created (the affirmative outcome for "stayed embedded" tests). - @discardableResult - func waitForEmbeddedTab( - urlContaining needle: String, - timeout: TimeInterval = 10.0, - file: StaticString = #filePath, - line: UInt = #line - ) throws -> Bool { - let deadline = Date().addingTimeInterval(timeout) - while Date() < deadline { - // Poll through the transient socket errors the first browser - // interaction of a freshly launched app can hit (matching - // waitForCondition), retrying until the deadline instead of throwing - // out of the loop on the first miss. - let envelope = socketEnvelope( - method: "browser.tab.list", - params: ["workspace_id": lastWorkspaceID], - responseTimeout: 6.0 - ) - if (envelope?["ok"] as? Bool) == true, - let result = envelope?["result"] as? [String: Any], - let tabs = result["tabs"] as? [[String: Any]] { - let urls = tabs.compactMap { $0["url"] as? String } - if urls.contains(where: { $0.contains(needle) }) { return true } - } - RunLoop.current.run(until: Date().addingTimeInterval(0.3)) - } - return false - } - - /// Waits for a selector to resolve before interacting, polling through two - /// cold-start transients that hit the first browser interaction of a - /// freshly launched app: - /// - `load_state` "complete" can fire a beat before the fixture DOM is - /// queryable, so a bare wait races the load; and - /// - the web-content process is still spinning up, so an eval can come - /// back "Completion handler ... no longer reachable" (the handler is - /// dropped when the content process is replaced). Both are transient: - /// re-issue until the element resolves or the deadline passes. - func waitForSelector( - _ selector: String, - surfaceID: String, - timeout: TimeInterval = 15.0, - file: StaticString = #filePath, - line: UInt = #line - ) throws { - try waitForCondition( - ["surface_id": surfaceID, "selector": selector], - describedAs: "selector \(selector)", - timeout: timeout, - file: file, - line: line - ) - } - // MARK: - Read-only page state (browser.eval is for assertions only) func evalValue( @@ -481,26 +282,10 @@ class BrowserFixtureSocketTestCase: XCTestCase { for candidate in self.socketCandidates() { guard FileManager.default.fileExists(atPath: candidate) else { continue } if ControlSocketClient(path: candidate, responseTimeout: 1.0).sendLine("ping") == "PONG" { - NSLog("BrowserFixtureSocketTestCase readiness via in-process client: %@", candidate) - self.socketPath = candidate - return true - } - if self.controlSocketCommandViaNetcat("ping", socketPath: candidate) == "PONG" { - NSLog("BrowserFixtureSocketTestCase readiness via nc fallback: %@", candidate) self.socketPath = candidate return true } } - // The in-process client (and nc) can fail to connect on some - // hosts even while the app's own sanity probe proves the - // listener is bound and answering; accept that probe. - let diagnostics = self.loadDiagnostics() - if self.controlSocketDiagnosticsReportReady(diagnostics), - let expectedPath = diagnostics["socketExpectedPath"], !expectedPath.isEmpty { - NSLog("BrowserFixtureSocketTestCase readiness via app diagnostics only: %@", expectedPath) - self.socketPath = expectedPath - return true - } return false } ) @@ -604,11 +389,7 @@ class BrowserFixtureSocketTestCase: XCTestCase { Darwin.connect(fd, sockaddrPtr, addrLen) } } - guard connected == 0 else { - let err = errno - NSLog("ControlSocketClient connect failed path=%@ errno=%d (%@)", path, err, String(cString: strerror(err))) - return nil - } + guard connected == 0 else { return nil } let payload = Array((line + "\n").utf8) let wrote = payload.withUnsafeBytes { rawBuffer in diff --git a/cmuxUITests/BrowserFixtures/external-open-routing.html b/cmuxUITests/BrowserFixtures/external-open-routing.html deleted file mode 100644 index fb6d99152af0..000000000000 --- a/cmuxUITests/BrowserFixtures/external-open-routing.html +++ /dev/null @@ -1,14 +0,0 @@ - - -external-open routing fixture - -

external-open routing fixture

-

matched host link

-

unmatched local link

-

-
- - -
- - diff --git a/cmuxUITests/BrowserFixtures/external-open-target.html b/cmuxUITests/BrowserFixtures/external-open-target.html deleted file mode 100644 index d1c2f936f0bb..000000000000 --- a/cmuxUITests/BrowserFixtures/external-open-target.html +++ /dev/null @@ -1,7 +0,0 @@ - - -external-open target fixture - -

external-open target fixture

- - From fbfe7659977b0f128901e87912ea4586062f8721 Mon Sep 17 00:00:00 2001 From: ejc3 Date: Sat, 3 Oct 2026 17:48:07 -0700 Subject: [PATCH 6/6] browser: use a rule the pattern safety check accepts in the port-link test --- cmuxTests/BrowserConfigTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmuxTests/BrowserConfigTests.swift b/cmuxTests/BrowserConfigTests.swift index 80de9f1ff28e..09795d78c628 100644 --- a/cmuxTests/BrowserConfigTests.swift +++ b/cmuxTests/BrowserConfigTests.swift @@ -5645,7 +5645,7 @@ final class BrowserLinkOpenSettingsTests: XCTestCase { /// The same holds for a port link, which is a plain http URL on a host. func testSidebarPortLinkMatchingAnExternalRuleGoesToTheSystemBrowser() throws { defaults.set( - "re:^https?://dashboard\\.example\\.com(:[0-9]+)?/", + "re:^https?://dashboard\\.example\\.com:[0-9]+/", forKey: BrowserLinkOpenSettings.browserExternalOpenPatternsKey ) let handler = BrowserExternalNavigationHandler(defaults: defaults)