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
2026-02-23 23:09:36 -08:00 · 2026-02-23 23:09:36 -08:00 · afbfb5a117
commit afbfb5a117
parent 7a75988c5b
5 changed files with 314 additions and 42 deletions
--- 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)
            }
        }
--- 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,
--- 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 = """
        (() => {
--- 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")
 }
--- 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() {