From 2499ba1bb2fbabfaff40522de8d12e1a985a06ae Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Mon, 23 Feb 2026 03:09:19 -0800 Subject: [PATCH] Fix browser-surface click focus without regressing open (#355) * Allow click-to-focus for unfocused browser surfaces * Add browser click-focus diagnostics and guard fix * Allow pointer-initiated browser focus through responder guard --- Sources/AppDelegate.swift | 110 +++++++++++++++- Sources/Panels/BrowserPanel.swift | 10 ++ Sources/Panels/BrowserPanelView.swift | 38 +++++- Sources/Panels/CmuxWebView.swift | 64 ++++++++- cmuxTests/CmuxWebViewKeyEquivalentTests.swift | 123 ++++++++++++++++++ 5 files changed, 335 insertions(+), 10 deletions(-) diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index 7bafbc8e..f9003b88 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -1548,6 +1548,18 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent _ = didInstallWindowFirstResponderSwizzle } +#if DEBUG + static func setWindowFirstResponderGuardTesting(currentEvent: NSEvent?, hitView: NSView?) { + cmuxFirstResponderGuardCurrentEventOverride = currentEvent + cmuxFirstResponderGuardHitViewOverride = hitView + } + + static func clearWindowFirstResponderGuardTesting() { + cmuxFirstResponderGuardCurrentEventOverride = nil + cmuxFirstResponderGuardHitViewOverride = nil + } +#endif + private func installWindowResponderSwizzles() { _ = Self.didInstallWindowKeyEquivalentSwizzle _ = Self.didInstallWindowFirstResponderSwizzle @@ -2887,6 +2899,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent ) { [weak self] notification in guard let self else { return } guard let panelId = notification.object as? UUID else { return } + self.browserPanel(for: panelId)?.endSuppressWebViewFocusForAddressBar() if self.browserAddressBarFocusedPanelId == panelId { self.browserAddressBarFocusedPanelId = nil self.stopBrowserOmnibarSelectionRepeat() @@ -3853,19 +3866,59 @@ enum MenuBarIconRenderer { } +#if DEBUG +private var cmuxFirstResponderGuardCurrentEventOverride: NSEvent? +private var cmuxFirstResponderGuardHitViewOverride: NSView? +#endif + private extension NSWindow { @objc func cmux_makeFirstResponder(_ responder: NSResponder?) -> Bool { if let responder, let webView = Self.cmuxOwningWebView(for: responder), - !webView.allowsFirstResponderAcquisition { -#if DEBUG - dlog( - "focus.guard blockedFirstResponder responder=\(String(describing: type(of: responder))) " + - "window=\(ObjectIdentifier(self))" + !webView.allowsFirstResponderAcquisitionEffective { + let currentEvent = Self.cmuxCurrentEvent(for: self) + let pointerInitiatedFocus = Self.cmuxShouldAllowPointerInitiatedWebViewFocus( + window: self, + webView: webView, + event: currentEvent ) + if pointerInitiatedFocus { +#if DEBUG + dlog( + "focus.guard allowPointerFirstResponder responder=\(String(describing: type(of: responder))) " + + "window=\(ObjectIdentifier(self)) " + + "web=\(ObjectIdentifier(webView)) " + + "policy=\(webView.allowsFirstResponderAcquisition ? 1 : 0) " + + "pointerDepth=\(webView.debugPointerFocusAllowanceDepth) " + + "eventType=\(currentEvent.map { String(describing: $0.type) } ?? "nil")" + ) #endif - return false + } else { +#if DEBUG + dlog( + "focus.guard blockedFirstResponder responder=\(String(describing: type(of: responder))) " + + "window=\(ObjectIdentifier(self)) " + + "web=\(ObjectIdentifier(webView)) " + + "policy=\(webView.allowsFirstResponderAcquisition ? 1 : 0) " + + "pointerDepth=\(webView.debugPointerFocusAllowanceDepth) " + + "eventType=\(currentEvent.map { String(describing: $0.type) } ?? "nil")" + ) +#endif + return false + } } +#if DEBUG + if let responder, + let webView = Self.cmuxOwningWebView(for: responder) { + dlog( + "focus.guard allowFirstResponder responder=\(String(describing: type(of: responder))) " + + "window=\(ObjectIdentifier(self)) " + + "web=\(ObjectIdentifier(webView)) " + + "policy=\(webView.allowsFirstResponderAcquisition ? 1 : 0) " + + "pointerDepth=\(webView.debugPointerFocusAllowanceDepth)" + ) + } +#endif return cmux_makeFirstResponder(responder) } @@ -4024,4 +4077,49 @@ private extension NSWindow { return nil } + + private static func cmuxCurrentEvent(for _: NSWindow) -> NSEvent? { +#if DEBUG + if let override = cmuxFirstResponderGuardCurrentEventOverride { + return override + } +#endif + return NSApp.currentEvent + } + + private static func cmuxHitViewForCurrentEvent(in window: NSWindow, event: NSEvent) -> NSView? { +#if DEBUG + if let override = cmuxFirstResponderGuardHitViewOverride { + return override + } +#endif + return window.contentView?.hitTest(event.locationInWindow) + } + + private static func cmuxShouldAllowPointerInitiatedWebViewFocus( + window: NSWindow, + webView: CmuxWebView, + event: NSEvent? + ) -> Bool { + guard let event else { return false } + switch event.type { + case .leftMouseDown, .rightMouseDown, .otherMouseDown: + break + default: + return false + } + + if event.windowNumber != 0, event.windowNumber != window.windowNumber { + return false + } + if let eventWindow = event.window, eventWindow !== window { + return false + } + + guard let hitView = cmuxHitViewForCurrentEvent(in: window, event: event), + let hitWebView = cmuxOwningWebView(for: hitView) else { + return false + } + return hitWebView === webView + } } diff --git a/Sources/Panels/BrowserPanel.swift b/Sources/Panels/BrowserPanel.swift index 2a683ba5..6255672e 100644 --- a/Sources/Panels/BrowserPanel.swift +++ b/Sources/Panels/BrowserPanel.swift @@ -2117,10 +2117,20 @@ extension BrowserPanel { } func beginSuppressWebViewFocusForAddressBar() { + if !suppressWebViewFocusForAddressBar { +#if DEBUG + dlog("browser.focus.addressBarSuppress.begin panel=\(id.uuidString.prefix(5))") +#endif + } suppressWebViewFocusForAddressBar = true } func endSuppressWebViewFocusForAddressBar() { + if suppressWebViewFocusForAddressBar { +#if DEBUG + dlog("browser.focus.addressBarSuppress.end panel=\(id.uuidString.prefix(5))") +#endif + } suppressWebViewFocusForAddressBar = false } diff --git a/Sources/Panels/BrowserPanelView.swift b/Sources/Panels/BrowserPanelView.swift index f0c65a81..0294766f 100644 --- a/Sources/Panels/BrowserPanelView.swift +++ b/Sources/Panels/BrowserPanelView.swift @@ -291,6 +291,13 @@ struct BrowserPanelView: View { guard let webView = note.object as? CmuxWebView else { return false } return webView === panel?.webView }) { _ in +#if DEBUG + dlog( + "browser.focus.clickIntent panel=\(panel.id.uuidString.prefix(5)) " + + "isFocused=\(isFocused ? 1 : 0) " + + "addressFocused=\(addressBarFocused ? 1 : 0)" + ) +#endif onRequestPanelFocus() } .onReceive(NotificationCenter.default.publisher(for: .webViewMiddleClickedLink).filter { [weak panel] note in @@ -317,6 +324,7 @@ struct BrowserPanelView: View { syncURLFromPanel() // If the browser surface is focused but has no URL loaded yet, auto-focus the omnibar. autoFocusOmnibarIfBlank() + syncWebViewResponderPolicyWithViewState(reason: "onAppear") BrowserHistoryStore.shared.loadIfNeeded() } .onChange(of: panel.focusFlashToken) { _ in @@ -356,6 +364,7 @@ struct BrowserPanelView: View { hideSuggestions() addressBarFocused = false } + syncWebViewResponderPolicyWithViewState(reason: "panelFocusChanged") } .onChange(of: addressBarFocused) { focused in let urlString = panel.preferredURLStringForOmnibar() ?? "" @@ -383,6 +392,7 @@ struct BrowserPanelView: View { } inlineCompletion = nil } + syncWebViewResponderPolicyWithViewState(reason: "addressBarFocusChanged") } .onReceive(NotificationCenter.default.publisher(for: .browserMoveOmnibarSelection)) { notification in guard let panelId = notification.object as? UUID, panelId == panel.id else { return } @@ -715,6 +725,21 @@ struct BrowserPanelView: View { } } + private func syncWebViewResponderPolicyWithViewState(reason: String) { + guard let cmuxWebView = panel.webView as? CmuxWebView else { return } + let next = isFocused && !panel.shouldSuppressWebViewFocus() + if cmuxWebView.allowsFirstResponderAcquisition != next { +#if DEBUG + dlog( + "browser.focus.policy.resync panel=\(panel.id.uuidString.prefix(5)) " + + "web=\(ObjectIdentifier(cmuxWebView)) old=\(cmuxWebView.allowsFirstResponderAcquisition ? 1 : 0) " + + "new=\(next ? 1 : 0) reason=\(reason)" + ) +#endif + } + cmuxWebView.allowsFirstResponderAcquisition = next + } + private func syncURLFromPanel() { let urlString = panel.preferredURLStringForOmnibar() ?? "" let effects = omnibarReduce(state: &omnibarState, event: .panelURLChanged(currentURLString: urlString)) @@ -3379,7 +3404,18 @@ struct WebViewRepresentable: NSViewRepresentable { isPanelFocused: Bool ) { guard let cmuxWebView = webView as? CmuxWebView else { return } - cmuxWebView.allowsFirstResponderAcquisition = isPanelFocused && !panel.shouldSuppressWebViewFocus() + let next = isPanelFocused && !panel.shouldSuppressWebViewFocus() + if cmuxWebView.allowsFirstResponderAcquisition != next { +#if DEBUG + dlog( + "browser.focus.policy panel=\(panel.id.uuidString.prefix(5)) " + + "web=\(ObjectIdentifier(cmuxWebView)) old=\(cmuxWebView.allowsFirstResponderAcquisition ? 1 : 0) " + + "new=\(next ? 1 : 0) isPanelFocused=\(isPanelFocused ? 1 : 0) " + + "suppress=\(panel.shouldSuppressWebViewFocus() ? 1 : 0)" + ) +#endif + } + cmuxWebView.allowsFirstResponderAcquisition = next } static func dismantleNSView(_ nsView: NSView, coordinator: Coordinator) { diff --git a/Sources/Panels/CmuxWebView.swift b/Sources/Panels/CmuxWebView.swift index 93f5a321..83941484 100644 --- a/Sources/Panels/CmuxWebView.swift +++ b/Sources/Panels/CmuxWebView.swift @@ -1,4 +1,5 @@ import AppKit +import Bonsplit import ObjectiveC import WebKit @@ -25,10 +26,56 @@ final class CmuxWebView: WKWebView { /// Guard against background panes stealing first responder (e.g. page autofocus). /// BrowserPanelView updates this as pane focus state changes. var allowsFirstResponderAcquisition: Bool = true + private var pointerFocusAllowanceDepth: Int = 0 + var allowsFirstResponderAcquisitionEffective: Bool { + allowsFirstResponderAcquisition || pointerFocusAllowanceDepth > 0 + } + var debugPointerFocusAllowanceDepth: Int { pointerFocusAllowanceDepth } override func becomeFirstResponder() -> Bool { - guard allowsFirstResponderAcquisition else { return false } - return super.becomeFirstResponder() + guard allowsFirstResponderAcquisitionEffective else { +#if DEBUG + let eventType = NSApp.currentEvent.map { String(describing: $0.type) } ?? "nil" + dlog( + "browser.focus.blockedBecome web=\(ObjectIdentifier(self)) " + + "policy=\(allowsFirstResponderAcquisition ? 1 : 0) " + + "pointerDepth=\(pointerFocusAllowanceDepth) eventType=\(eventType)" + ) +#endif + return false + } + let result = super.becomeFirstResponder() +#if DEBUG + let eventType = NSApp.currentEvent.map { String(describing: $0.type) } ?? "nil" + dlog( + "browser.focus.become web=\(ObjectIdentifier(self)) result=\(result ? 1 : 0) " + + "policy=\(allowsFirstResponderAcquisition ? 1 : 0) " + + "pointerDepth=\(pointerFocusAllowanceDepth) eventType=\(eventType)" + ) +#endif + return result + } + + /// Temporarily permits focus acquisition for explicit pointer-driven interactions + /// (mouse click into this webview) while keeping background autofocus blocked. + func withPointerFocusAllowance(_ body: () -> Void) { + pointerFocusAllowanceDepth += 1 +#if DEBUG + dlog( + "browser.focus.pointerAllowance.enter web=\(ObjectIdentifier(self)) " + + "depth=\(pointerFocusAllowanceDepth)" + ) +#endif + defer { + pointerFocusAllowanceDepth = max(0, pointerFocusAllowanceDepth - 1) +#if DEBUG + dlog( + "browser.focus.pointerAllowance.exit web=\(ObjectIdentifier(self)) " + + "depth=\(pointerFocusAllowanceDepth)" + ) +#endif + } + body() } override func performKeyEquivalent(with event: NSEvent) -> Bool { @@ -71,8 +118,19 @@ final class CmuxWebView: WKWebView { // NSView (WKWebView), not to sibling SwiftUI overlays. Notify the panel system so // bonsplit focus tracks which pane the user clicked in. override func mouseDown(with event: NSEvent) { +#if DEBUG + let windowNumber = window?.windowNumber ?? -1 + let firstResponderType = window?.firstResponder.map { String(describing: type(of: $0)) } ?? "nil" + dlog( + "browser.focus.mouseDown web=\(ObjectIdentifier(self)) " + + "policy=\(allowsFirstResponderAcquisition ? 1 : 0) " + + "pointerDepth=\(pointerFocusAllowanceDepth) win=\(windowNumber) fr=\(firstResponderType)" + ) +#endif NotificationCenter.default.post(name: .webViewDidReceiveClick, object: self) - super.mouseDown(with: event) + withPointerFocusAllowance { + super.mouseDown(with: event) + } } // MARK: - Mouse back/forward buttons & middle-click diff --git a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift index 45e2a54e..05af20eb 100644 --- a/cmuxTests/CmuxWebViewKeyEquivalentTests.swift +++ b/cmuxTests/CmuxWebViewKeyEquivalentTests.swift @@ -136,6 +136,38 @@ final class CmuxWebViewKeyEquivalentTests: XCTestCase { } } + @MainActor + func testPointerFocusAllowanceCanTemporarilyOverrideBlockedFirstResponderAcquisition() { + _ = NSApplication.shared + + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 640, height: 420), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + let container = NSView(frame: window.contentRect(forFrameRect: window.frame)) + window.contentView = container + + let webView = CmuxWebView(frame: container.bounds, configuration: WKWebViewConfiguration()) + webView.autoresizingMask = [.width, .height] + container.addSubview(webView) + + window.makeKeyAndOrderFront(nil) + defer { window.orderOut(nil) } + + webView.allowsFirstResponderAcquisition = false + _ = window.makeFirstResponder(nil) + XCTAssertFalse(webView.becomeFirstResponder(), "Expected focus to stay blocked by policy") + + webView.withPointerFocusAllowance { + XCTAssertTrue(webView.becomeFirstResponder(), "Expected explicit pointer intent to bypass policy") + } + + _ = window.makeFirstResponder(nil) + XCTAssertFalse(webView.becomeFirstResponder(), "Expected pointer allowance to be temporary") + } + @MainActor func testWindowFirstResponderGuardBlocksDescendantWhenPaneIsUnfocused() { _ = NSApplication.shared @@ -172,6 +204,97 @@ final class CmuxWebViewKeyEquivalentTests: XCTestCase { } } + @MainActor + func testWindowFirstResponderGuardAllowsDescendantDuringPointerFocusAllowance() { + _ = NSApplication.shared + AppDelegate.installWindowResponderSwizzlesForTesting() + + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 640, height: 420), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + let container = NSView(frame: window.contentRect(forFrameRect: window.frame)) + window.contentView = container + + let webView = CmuxWebView(frame: container.bounds, configuration: WKWebViewConfiguration()) + webView.autoresizingMask = [.width, .height] + container.addSubview(webView) + + let descendant = FirstResponderView(frame: NSRect(x: 0, y: 0, width: 10, height: 10)) + webView.addSubview(descendant) + + window.makeKeyAndOrderFront(nil) + defer { window.orderOut(nil) } + + webView.allowsFirstResponderAcquisition = false + _ = window.makeFirstResponder(nil) + XCTAssertFalse(window.makeFirstResponder(descendant), "Expected blocked focus outside pointer allowance") + + _ = window.makeFirstResponder(nil) + webView.withPointerFocusAllowance { + XCTAssertTrue(window.makeFirstResponder(descendant), "Expected pointer allowance to bypass guard") + } + + _ = window.makeFirstResponder(nil) + XCTAssertFalse(window.makeFirstResponder(descendant), "Expected pointer allowance to remain temporary") + } + + @MainActor + func testWindowFirstResponderGuardAllowsPointerInitiatedClickFocusWhenPolicyIsBlocked() { + _ = NSApplication.shared + AppDelegate.installWindowResponderSwizzlesForTesting() + + let window = NSWindow( + contentRect: NSRect(x: 0, y: 0, width: 640, height: 420), + styleMask: [.titled, .closable], + backing: .buffered, + defer: false + ) + let container = NSView(frame: window.contentRect(forFrameRect: window.frame)) + window.contentView = container + + let webView = CmuxWebView(frame: container.bounds, configuration: WKWebViewConfiguration()) + webView.autoresizingMask = [.width, .height] + container.addSubview(webView) + + let descendant = FirstResponderView(frame: NSRect(x: 0, y: 0, width: 10, height: 10)) + webView.addSubview(descendant) + + window.makeKeyAndOrderFront(nil) + defer { + AppDelegate.clearWindowFirstResponderGuardTesting() + window.orderOut(nil) + } + + webView.allowsFirstResponderAcquisition = false + _ = window.makeFirstResponder(nil) + XCTAssertFalse(window.makeFirstResponder(descendant), "Expected blocked focus without pointer click context") + + let timestamp = ProcessInfo.processInfo.systemUptime + let pointerDownEvent = NSEvent.mouseEvent( + with: .leftMouseDown, + location: NSPoint(x: 5, y: 5), + modifierFlags: [], + timestamp: timestamp, + windowNumber: window.windowNumber, + context: nil, + eventNumber: 1, + clickCount: 1, + pressure: 1.0 + ) + XCTAssertNotNil(pointerDownEvent) + + AppDelegate.setWindowFirstResponderGuardTesting(currentEvent: pointerDownEvent, hitView: descendant) + _ = window.makeFirstResponder(nil) + XCTAssertTrue(window.makeFirstResponder(descendant), "Expected pointer click context to bypass blocked policy") + + AppDelegate.clearWindowFirstResponderGuardTesting() + _ = window.makeFirstResponder(nil) + XCTAssertFalse(window.makeFirstResponder(descendant), "Expected pointer bypass to be limited to click context") + } + private func installMenu(spy: ActionSpy, key: String, modifiers: NSEvent.ModifierFlags) { let mainMenu = NSMenu()