From afbfb5a117c94a02afca0004ef6d2666c0899cf1 Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Mon, 23 Feb 2026 23:09:36 -0800 Subject: [PATCH] Use native WebKit middle-click handling for browser links (#416) * Add middle-click debug logging for browser links * Handle browser middle-click via native WebKit actions * Fix flaky middle-click new-tab detection in browser --- Sources/Panels/BrowserPanel.swift | 165 ++++++++++++++++-- Sources/Panels/BrowserPanelView.swift | 8 - Sources/Panels/CmuxWebView.swift | 74 ++++++-- Sources/TabManager.swift | 1 - cmuxTests/CmuxWebViewKeyEquivalentTests.swift | 108 ++++++++++++ 5 files changed, 314 insertions(+), 42 deletions(-) diff --git a/Sources/Panels/BrowserPanel.swift b/Sources/Panels/BrowserPanel.swift index e0f61109..71f297bb 100644 --- a/Sources/Panels/BrowserPanel.swift +++ b/Sources/Panels/BrowserPanel.swift @@ -1956,15 +1956,43 @@ extension BrowserPanel { /// Open a link in a new browser surface in the same pane func openLinkInNewTab(url: URL, bypassInsecureHTTPHostOnce: String? = nil) { - guard let tabManager = AppDelegate.shared?.tabManager, - let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }), - let paneId = workspace.paneId(forPanelId: id) else { return } +#if DEBUG + dlog( + "browser.newTab.open.begin panel=\(id.uuidString.prefix(5)) " + + "workspace=\(workspaceId.uuidString.prefix(5)) url=\(url.absoluteString) " + + "bypass=\(bypassInsecureHTTPHostOnce ?? "nil")" + ) +#endif + guard let tabManager = AppDelegate.shared?.tabManager else { +#if DEBUG + dlog("browser.newTab.open.abort panel=\(id.uuidString.prefix(5)) reason=missingTabManager") +#endif + return + } + guard let workspace = tabManager.tabs.first(where: { $0.id == workspaceId }) else { +#if DEBUG + dlog("browser.newTab.open.abort panel=\(id.uuidString.prefix(5)) reason=workspaceMissing") +#endif + return + } + guard let paneId = workspace.paneId(forPanelId: id) else { +#if DEBUG + dlog("browser.newTab.open.abort panel=\(id.uuidString.prefix(5)) reason=paneMissing") +#endif + return + } workspace.newBrowserSurface( inPane: paneId, url: url, focus: true, bypassInsecureHTTPHostOnce: bypassInsecureHTTPHostOnce ) +#if DEBUG + dlog( + "browser.newTab.open.done panel=\(id.uuidString.prefix(5)) " + + "workspace=\(workspace.id.uuidString.prefix(5)) pane=\(paneId.id.uuidString.prefix(5))" + ) +#endif } /// Reload the current page @@ -2664,6 +2692,39 @@ private class BrowserDownloadDelegate: NSObject, WKDownloadDelegate { // MARK: - Navigation Delegate +func browserNavigationShouldOpenInNewTab( + navigationType: WKNavigationType, + modifierFlags: NSEvent.ModifierFlags, + buttonNumber: Int, + hasRecentMiddleClickIntent: Bool = false, + currentEventType: NSEvent.EventType? = NSApp.currentEvent?.type, + currentEventButtonNumber: Int? = NSApp.currentEvent?.buttonNumber +) -> Bool { + guard navigationType == .linkActivated || navigationType == .other else { + return false + } + + if modifierFlags.contains(.command) { + return true + } + if buttonNumber == 2 { + return true + } + // In some WebKit paths, middle-click arrives as buttonNumber=4. + // Recover intent when we just observed a local middle-click. + if buttonNumber == 4, hasRecentMiddleClickIntent { + return true + } + + // WebKit can omit buttonNumber for middle-click link activations. + if let currentEventType, + (currentEventType == .otherMouseDown || currentEventType == .otherMouseUp), + currentEventButtonNumber == 2 { + return true + } + return false +} + private class BrowserNavigationDelegate: NSObject, WKNavigationDelegate { var didFinish: ((WKWebView) -> Void)? var didFailNavigation: ((WKWebView, String) -> Void)? @@ -2802,16 +2863,41 @@ private class BrowserNavigationDelegate: NSObject, WKNavigationDelegate { decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void ) { + let hasRecentMiddleClickIntent = CmuxWebView.hasRecentMiddleClickIntent(for: webView) + let shouldOpenInNewTab = browserNavigationShouldOpenInNewTab( + navigationType: navigationAction.navigationType, + modifierFlags: navigationAction.modifierFlags, + buttonNumber: navigationAction.buttonNumber, + hasRecentMiddleClickIntent: hasRecentMiddleClickIntent + ) +#if DEBUG + let currentEventType = NSApp.currentEvent.map { String(describing: $0.type) } ?? "nil" + let currentEventButton = NSApp.currentEvent.map { String($0.buttonNumber) } ?? "nil" + let navType = String(describing: navigationAction.navigationType) + dlog( + "browser.nav.decidePolicy navType=\(navType) button=\(navigationAction.buttonNumber) " + + "mods=\(navigationAction.modifierFlags.rawValue) targetNil=\(navigationAction.targetFrame == nil ? 1 : 0) " + + "eventType=\(currentEventType) eventButton=\(currentEventButton) " + + "recentMiddleIntent=\(hasRecentMiddleClickIntent ? 1 : 0) " + + "openInNewTab=\(shouldOpenInNewTab ? 1 : 0)" + ) +#endif + if let url = navigationAction.request.url, navigationAction.targetFrame?.isMainFrame != false, shouldBlockInsecureHTTPNavigation?(url) == true { let intent: BrowserInsecureHTTPNavigationIntent - if navigationAction.navigationType == .linkActivated, - navigationAction.modifierFlags.contains(.command) { + if shouldOpenInNewTab { intent = .newTab } else { intent = .currentTab } +#if DEBUG + dlog( + "browser.nav.decidePolicy.action kind=blockedInsecure intent=\(intent == .newTab ? "newTab" : "currentTab") " + + "url=\(url.absoluteString)" + ) +#endif handleBlockedInsecureHTTPNavigation?(navigationAction.request, intent) decisionHandler(.cancel) return @@ -2833,23 +2919,33 @@ private class BrowserNavigationDelegate: NSObject, WKNavigationDelegate { return } - // target=_blank or window.open() — navigate in the current webview - if navigationAction.targetFrame == nil, - navigationAction.request.url != nil { - webView.load(navigationAction.request) - decisionHandler(.cancel) - return - } - - // Cmd+click on a regular link — open in a new tab - if navigationAction.navigationType == .linkActivated, - navigationAction.modifierFlags.contains(.command), + // Cmd+click and middle-click on regular links should always open in a new tab. + if shouldOpenInNewTab, let url = navigationAction.request.url { +#if DEBUG + dlog("browser.nav.decidePolicy.action kind=openInNewTab url=\(url.absoluteString)") +#endif openInNewTab?(url) decisionHandler(.cancel) return } + // target=_blank or window.open() without explicit new-tab intent — navigate in-place. + if navigationAction.targetFrame == nil, + navigationAction.request.url != nil { +#if DEBUG + let targetURL = navigationAction.request.url?.absoluteString ?? "nil" + dlog("browser.nav.decidePolicy.action kind=loadInPlaceFromNilTarget url=\(targetURL)") +#endif + webView.load(navigationAction.request) + decisionHandler(.cancel) + return + } + +#if DEBUG + let targetURL = navigationAction.request.url?.absoluteString ?? "nil" + dlog("browser.nav.decidePolicy.action kind=allow url=\(targetURL)") +#endif decisionHandler(.allow) } @@ -2948,13 +3044,32 @@ private class BrowserUIDelegate: NSObject, WKUIDelegate { } /// Returning nil tells WebKit not to open a new window. - /// Cmd+click opens in a new tab; regular target=_blank navigates in-place. + /// Cmd+click and middle-click open in a new tab; regular target=_blank navigates in-place. func webView( _ webView: WKWebView, createWebViewWith configuration: WKWebViewConfiguration, for navigationAction: WKNavigationAction, windowFeatures: WKWindowFeatures ) -> WKWebView? { + let hasRecentMiddleClickIntent = CmuxWebView.hasRecentMiddleClickIntent(for: webView) + let shouldOpenInNewTab = browserNavigationShouldOpenInNewTab( + navigationType: navigationAction.navigationType, + modifierFlags: navigationAction.modifierFlags, + buttonNumber: navigationAction.buttonNumber, + hasRecentMiddleClickIntent: hasRecentMiddleClickIntent + ) +#if DEBUG + let currentEventType = NSApp.currentEvent.map { String(describing: $0.type) } ?? "nil" + let currentEventButton = NSApp.currentEvent.map { String($0.buttonNumber) } ?? "nil" + let navType = String(describing: navigationAction.navigationType) + dlog( + "browser.nav.createWebView navType=\(navType) button=\(navigationAction.buttonNumber) " + + "mods=\(navigationAction.modifierFlags.rawValue) targetNil=\(navigationAction.targetFrame == nil ? 1 : 0) " + + "eventType=\(currentEventType) eventButton=\(currentEventButton) " + + "recentMiddleIntent=\(hasRecentMiddleClickIntent ? 1 : 0) " + + "openInNewTab=\(shouldOpenInNewTab ? 1 : 0)" + ) +#endif if let url = navigationAction.request.url { if browserShouldOpenURLExternally(url) { let opened = NSWorkspace.shared.open(url) @@ -2968,11 +3083,23 @@ private class BrowserUIDelegate: NSObject, WKUIDelegate { } if let requestNavigation { let intent: BrowserInsecureHTTPNavigationIntent = - navigationAction.modifierFlags.contains(.command) ? .newTab : .currentTab + shouldOpenInNewTab ? .newTab : .currentTab +#if DEBUG + dlog( + "browser.nav.createWebView.action kind=requestNavigation intent=\(intent == .newTab ? "newTab" : "currentTab") " + + "url=\(url.absoluteString)" + ) +#endif requestNavigation(navigationAction.request, intent) - } else if navigationAction.modifierFlags.contains(.command) { + } else if shouldOpenInNewTab { +#if DEBUG + dlog("browser.nav.createWebView.action kind=openInNewTab url=\(url.absoluteString)") +#endif openInNewTab?(url) } else { +#if DEBUG + dlog("browser.nav.createWebView.action kind=loadInPlace url=\(url.absoluteString)") +#endif webView.load(navigationAction.request) } } diff --git a/Sources/Panels/BrowserPanelView.swift b/Sources/Panels/BrowserPanelView.swift index 71518994..82069f74 100644 --- a/Sources/Panels/BrowserPanelView.swift +++ b/Sources/Panels/BrowserPanelView.swift @@ -332,14 +332,6 @@ struct BrowserPanelView: View { #endif onRequestPanelFocus() } - .onReceive(NotificationCenter.default.publisher(for: .webViewMiddleClickedLink).filter { [weak panel] note in - guard let webView = note.object as? CmuxWebView else { return false } - return webView === panel?.webView - }) { note in - if let url = note.userInfo?["url"] as? URL { - panel.openLinkInNewTab(url: url) - } - } .onAppear { UserDefaults.standard.register(defaults: [ BrowserSearchSettings.searchEngineKey: BrowserSearchSettings.defaultSearchEngine.rawValue, diff --git a/Sources/Panels/CmuxWebView.swift b/Sources/Panels/CmuxWebView.swift index 8f2a3a28..68a13282 100644 --- a/Sources/Panels/CmuxWebView.swift +++ b/Sources/Panels/CmuxWebView.swift @@ -8,6 +8,37 @@ import WebKit /// key equivalents first so app-level shortcuts continue to work when WebKit is /// the first responder. final class CmuxWebView: WKWebView { + // Some sites/WebKit paths report middle-click link activations as + // WKNavigationAction.buttonNumber=4 instead of 2. Track a recent local + // middle-click so navigation delegates can recover intent reliably. + private struct MiddleClickIntent { + let webViewID: ObjectIdentifier + let uptime: TimeInterval + } + + private static var lastMiddleClickIntent: MiddleClickIntent? + private static let middleClickIntentMaxAge: TimeInterval = 0.8 + + static func hasRecentMiddleClickIntent(for webView: WKWebView) -> Bool { + guard let webView = webView as? CmuxWebView else { return false } + guard let intent = lastMiddleClickIntent else { return false } + + let age = ProcessInfo.processInfo.systemUptime - intent.uptime + if age > middleClickIntentMaxAge { + lastMiddleClickIntent = nil + return false + } + + return intent.webViewID == ObjectIdentifier(webView) + } + + private static func recordMiddleClickIntent(for webView: CmuxWebView) { + lastMiddleClickIntent = MiddleClickIntent( + webViewID: ObjectIdentifier(webView), + uptime: ProcessInfo.processInfo.systemUptime + ) + } + private final class ContextMenuFallbackBox: NSObject { weak var target: AnyObject? let action: Selector? @@ -136,16 +167,33 @@ final class CmuxWebView: WKWebView { } } - // MARK: - Mouse back/forward buttons & middle-click + // MARK: - Mouse back/forward buttons override func otherMouseDown(with event: NSEvent) { + if event.buttonNumber == 2 { + Self.recordMiddleClickIntent(for: self) + } +#if DEBUG + let point = convert(event.locationInWindow, from: nil) + let mods = event.modifierFlags.intersection(.deviceIndependentFlagsMask).rawValue + dlog( + "browser.mouse.otherDown web=\(ObjectIdentifier(self)) button=\(event.buttonNumber) " + + "clicks=\(event.clickCount) mods=\(mods) point=(\(Int(point.x)),\(Int(point.y)))" + ) +#endif // Button 3 = back, button 4 = forward (multi-button mice like Logitech). // Consume the event so WebKit doesn't handle it. switch event.buttonNumber { case 3: +#if DEBUG + dlog("browser.mouse.otherDown.action web=\(ObjectIdentifier(self)) kind=goBack canGoBack=\(canGoBack ? 1 : 0)") +#endif goBack() return case 4: +#if DEBUG + dlog("browser.mouse.otherDown.action web=\(ObjectIdentifier(self)) kind=goForward canGoForward=\(canGoForward ? 1 : 0)") +#endif goForward() return default: @@ -155,25 +203,23 @@ final class CmuxWebView: WKWebView { } override func otherMouseUp(with event: NSEvent) { - // Middle-click (button 2) on a link opens it in a new tab. if event.buttonNumber == 2 { - let point = convert(event.locationInWindow, from: nil) - findLinkAtPoint(point) { [weak self] url in - guard let self, let url else { return } - NotificationCenter.default.post( - name: .webViewMiddleClickedLink, - object: self, - userInfo: ["url": url] - ) - } - return + Self.recordMiddleClickIntent(for: self) } +#if DEBUG + let point = convert(event.locationInWindow, from: nil) + let mods = event.modifierFlags.intersection(.deviceIndependentFlagsMask).rawValue + dlog( + "browser.mouse.otherUp web=\(ObjectIdentifier(self)) button=\(event.buttonNumber) " + + "clicks=\(event.clickCount) mods=\(mods) point=(\(Int(point.x)),\(Int(point.y)))" + ) +#endif super.otherMouseUp(with: event) } - /// Use JavaScript to find the nearest anchor element at the given view-local point. + /// Finds the nearest anchor element at a given view-local point. + /// Used as a context-menu download fallback. private func findLinkAtPoint(_ point: NSPoint, completion: @escaping (URL?) -> Void) { - // WKWebView's coordinate system is flipped (origin top-left for web content). let flippedY = bounds.height - point.y let js = """ (() => { diff --git a/Sources/TabManager.swift b/Sources/TabManager.swift index c4f9b57e..5a11d1df 100644 --- a/Sources/TabManager.swift +++ b/Sources/TabManager.swift @@ -3240,5 +3240,4 @@ extension Notification.Name { static let browserDidFocusAddressBar = Notification.Name("browserDidFocusAddressBar") static let browserDidBlurAddressBar = Notification.Name("browserDidBlurAddressBar") static let webViewDidReceiveClick = Notification.Name("webViewDidReceiveClick") - static let webViewMiddleClickedLink = Notification.Name("webViewMiddleClickedLink") } diff --git a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift index 162441d4..12265867 100644 --- a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift +++ b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift @@ -1016,6 +1016,114 @@ final class BrowserDeveloperToolsConfigurationTests: XCTestCase { } } +final class BrowserNavigationNewTabDecisionTests: XCTestCase { + func testLinkActivatedCmdClickOpensInNewTab() { + XCTAssertTrue( + browserNavigationShouldOpenInNewTab( + navigationType: .linkActivated, + modifierFlags: [.command], + buttonNumber: 0 + ) + ) + } + + func testLinkActivatedMiddleClickOpensInNewTab() { + XCTAssertTrue( + browserNavigationShouldOpenInNewTab( + navigationType: .linkActivated, + modifierFlags: [], + buttonNumber: 2 + ) + ) + } + + func testLinkActivatedPlainLeftClickStaysInCurrentTab() { + XCTAssertFalse( + browserNavigationShouldOpenInNewTab( + navigationType: .linkActivated, + modifierFlags: [], + buttonNumber: 0 + ) + ) + } + + func testOtherNavigationMiddleClickOpensInNewTab() { + XCTAssertTrue( + browserNavigationShouldOpenInNewTab( + navigationType: .other, + modifierFlags: [], + buttonNumber: 2 + ) + ) + } + + func testOtherNavigationLeftClickStaysInCurrentTab() { + XCTAssertFalse( + browserNavigationShouldOpenInNewTab( + navigationType: .other, + modifierFlags: [], + buttonNumber: 0 + ) + ) + } + + func testLinkActivatedButtonFourWithoutMiddleIntentStaysInCurrentTab() { + XCTAssertFalse( + browserNavigationShouldOpenInNewTab( + navigationType: .linkActivated, + modifierFlags: [], + buttonNumber: 4, + hasRecentMiddleClickIntent: false + ) + ) + } + + func testLinkActivatedButtonFourWithRecentMiddleIntentOpensInNewTab() { + XCTAssertTrue( + browserNavigationShouldOpenInNewTab( + navigationType: .linkActivated, + modifierFlags: [], + buttonNumber: 4, + hasRecentMiddleClickIntent: true + ) + ) + } + + func testLinkActivatedUsesCurrentEventFallbackForMiddleClick() { + XCTAssertTrue( + browserNavigationShouldOpenInNewTab( + navigationType: .linkActivated, + modifierFlags: [], + buttonNumber: 0, + currentEventType: .otherMouseUp, + currentEventButtonNumber: 2 + ) + ) + } + + func testCurrentEventFallbackDoesNotAffectNonLinkNavigation() { + XCTAssertFalse( + browserNavigationShouldOpenInNewTab( + navigationType: .reload, + modifierFlags: [], + buttonNumber: 0, + currentEventType: .otherMouseUp, + currentEventButtonNumber: 2 + ) + ) + } + + func testNonLinkNavigationNeverForcesNewTab() { + XCTAssertFalse( + browserNavigationShouldOpenInNewTab( + navigationType: .reload, + modifierFlags: [.command], + buttonNumber: 2 + ) + ) + } +} + @MainActor final class BrowserJavaScriptDialogDelegateTests: XCTestCase { func testBrowserPanelUIDelegateImplementsJavaScriptDialogSelectors() {