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
This commit is contained in:
parent
4fb5da373d
commit
2499ba1bb2
5 changed files with 335 additions and 10 deletions
|
|
@ -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
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue