diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5cbe5fda..5cf24332 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -238,9 +238,9 @@ jobs: tests-build-and-lag: # Build the full cmux scheme and run the lag regression on WarpBuild. - # XCUITests cannot run on WarpBuild (Virtualization.framework limitation: - # XCUIApplication stuck "Running Background", 62s activation timeout per - # test). Interactive UI tests run via test-e2e.yml on GitHub-hosted runners. + # Keep lag validation separate from UI regressions so functional UI failures + # and performance regressions stay isolated. Broader interactive UI suites + # still run via test-e2e.yml on GitHub-hosted runners. if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository runs-on: warp-macos-15-arm64-6x timeout-minutes: 20 @@ -350,6 +350,7 @@ jobs: VDISPLAY_PID=$! echo "VDISPLAY_PID=$VDISPLAY_PID" >> "$GITHUB_ENV" sleep 3 + kill -0 "$VDISPLAY_PID" - name: Run workspace churn typing-lag regression run: | @@ -393,7 +394,16 @@ jobs: CMUX_LAG_KEY_EVENTS=180 \ python3 tests/test_workspace_churn_up_arrow_lag.py - ui-display-resolution-regression: + - name: Cleanup virtual display + if: always() + run: | + set -euo pipefail + if [ -n "${VDISPLAY_PID:-}" ]; then + kill "$VDISPLAY_PID" >/dev/null 2>&1 || true + fi + rm -f /tmp/create-virtual-display + + ui-regressions: if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository runs-on: warp-macos-15-arm64-6x timeout-minutes: 25 @@ -445,8 +455,8 @@ jobs: uses: actions/cache@5a3ec84eff668545956fd18022155c47e93e2684 # v4 with: path: .ci-source-packages - key: spm-ui-display-resolution-${{ hashFiles('GhosttyTabs.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved') }} - restore-keys: spm-ui-display-resolution- + key: spm-ui-regressions-${{ hashFiles('GhosttyTabs.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved') }} + restore-keys: spm-ui-regressions- - name: Resolve Swift packages run: | @@ -491,3 +501,15 @@ jobs: -destination "platform=macOS" \ -only-testing:cmuxUITests/DisplayResolutionRegressionUITests \ test + + - name: Run browser find focus UI regression + run: | + set -euo pipefail + SOURCE_PACKAGES_DIR="$PWD/.ci-source-packages" + xcodebuild -project GhosttyTabs.xcodeproj -scheme cmux -configuration Debug \ + -clonedSourcePackagesDirPath "$SOURCE_PACKAGES_DIR" \ + -disableAutomaticPackageResolution \ + -destination "platform=macOS" \ + -maximum-test-execution-time-allowance 120 \ + -only-testing:cmuxUITests/BrowserPaneNavigationKeybindUITests/testCmdFFocusesBrowserFindFieldAfterCmdDCmdLNavigation \ + test diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml index eb99dcbf..76eaa72f 100644 --- a/.github/workflows/test-e2e.yml +++ b/.github/workflows/test-e2e.yml @@ -217,9 +217,7 @@ jobs: clang -framework Foundation -framework CoreGraphics \ -o "$HELPER_PATH" scripts/create-virtual-display.m - cat >"$MANIFEST_PATH" < "$MANIFEST_PATH" fi # Start recording right before the test (after build/resolve). diff --git a/Sources/AppDelegate.swift b/Sources/AppDelegate.swift index d2f3ad41..d7da8180 100644 --- a/Sources/AppDelegate.swift +++ b/Sources/AppDelegate.swift @@ -12473,6 +12473,13 @@ private extension NSWindow { in window: NSWindow, event: NSEvent? ) -> CmuxWebView? { + // Browser find runs in the portal slot alongside the hosted WKWebView. + // Treat its native field editor chain as browser chrome, not as web content, + // so Cmd+F can move first responder into the find field while web focus is suppressed. + if BrowserWindowPortalRegistry.searchOverlayPanelId(for: responder, in: window) != nil { + return nil + } + if let webView = cmuxOwningWebView(for: responder) { return webView } diff --git a/Sources/BrowserWindowPortal.swift b/Sources/BrowserWindowPortal.swift index e68dbbdc..b5f2bdc3 100644 --- a/Sources/BrowserWindowPortal.swift +++ b/Sources/BrowserWindowPortal.swift @@ -1777,13 +1777,38 @@ final class WindowBrowserSlotView: NSView { logSearchOverlayEvent("create", panelId: configuration.panelId) } - func searchOverlayPanelId(for responder: NSResponder) -> UUID? { - guard let overlay = searchOverlayHostingView, - let view = responder.browserPortalOwningView, - view.isDescendant(of: overlay) else { - return nil + private func searchOverlayOwnsFieldEditor(_ fieldEditor: NSTextView, in root: NSView) -> Bool { + guard fieldEditor.isFieldEditor else { return false } + + if let textField = root as? NSTextField, textField.currentEditor() === fieldEditor { + return true } - return objc_getAssociatedObject(overlay, &cmuxBrowserSearchOverlayPanelIdAssociationKey) as? UUID + + for subview in root.subviews { + if searchOverlayOwnsFieldEditor(fieldEditor, in: subview) { + return true + } + } + + return false + } + + func searchOverlayPanelId(for responder: NSResponder) -> UUID? { + guard let overlay = searchOverlayHostingView else { return nil } + + let panelId = objc_getAssociatedObject(overlay, &cmuxBrowserSearchOverlayPanelIdAssociationKey) as? UUID + + if let view = responder as? NSView, + view === overlay || view.isDescendant(of: overlay) { + return panelId + } + + if let fieldEditor = responder as? NSTextView, + searchOverlayOwnsFieldEditor(fieldEditor, in: overlay) { + return panelId + } + + return nil } @discardableResult diff --git a/Sources/Find/BrowserSearchOverlay.swift b/Sources/Find/BrowserSearchOverlay.swift index 5fde1163..66f66581 100644 --- a/Sources/Find/BrowserSearchOverlay.swift +++ b/Sources/Find/BrowserSearchOverlay.swift @@ -14,78 +14,35 @@ struct BrowserSearchOverlay: View { @State private var corner: Corner = .topRight @State private var dragOffset: CGSize = .zero @State private var barSize: CGSize = .zero - @FocusState private var isSearchFieldFocused: Bool + @State private var isSearchFieldFocused: Bool = true private let padding: CGFloat = 8 -#if DEBUG - private func debugFirstResponderSummary() -> String { - guard let window = NSApp.keyWindow else { return "nil" } - guard let firstResponder = window.firstResponder else { return "nil" } - if let editor = firstResponder as? NSTextView, editor.isFieldEditor { - let delegateSummary = editor.delegate.map { String(describing: type(of: $0)) } ?? "nil" - return "fieldEditor(delegate=\(delegateSummary))" - } - return String(describing: type(of: firstResponder)) - } -#endif - - private func logFocusState(_ event: String) { -#if DEBUG - let keyWindow = NSApp.keyWindow - dlog( - "browser.findbar.focus panel=\(panelId.uuidString.prefix(5)) " + - "event=\(event) keyWindow=\(keyWindow?.windowNumber ?? -1) " + - "firstResponder=\(debugFirstResponderSummary()) " + - "focused=\(isSearchFieldFocused ? 1 : 0)" - ) -#endif - } - - private func requestSearchFieldFocus(maxAttempts: Int = 3, origin: String) { - guard maxAttempts > 0 else { return } - guard canApplyFocusRequest(focusRequestGeneration) else { -#if DEBUG - logFocusState("request.skip origin=\(origin) generation=\(focusRequestGeneration)") -#endif - return - } - logFocusState("request.begin origin=\(origin) remaining=\(maxAttempts)") - isSearchFieldFocused = true -#if DEBUG - DispatchQueue.main.async { - guard canApplyFocusRequest(focusRequestGeneration) else { - logFocusState("request.skipAsync origin=\(origin) generation=\(focusRequestGeneration)") - return - } - logFocusState("request.afterAsync origin=\(origin) remaining=\(maxAttempts)") - } -#endif - guard maxAttempts > 1 else { return } - DispatchQueue.main.asyncAfter(deadline: .now() + 0.05) { - guard canApplyFocusRequest(focusRequestGeneration) else { -#if DEBUG - logFocusState("request.skipRetry origin=\(origin) generation=\(focusRequestGeneration)") -#endif - return - } - requestSearchFieldFocus(maxAttempts: maxAttempts - 1, origin: origin) - } - } - var body: some View { GeometryReader { geo in HStack(spacing: 4) { - TextField("Search", text: $searchState.needle) - .textFieldStyle(.plain) - .accessibilityIdentifier("BrowserFindSearchTextField") + BrowserSearchTextFieldRepresentable( + text: $searchState.needle, + isFocused: $isSearchFieldFocused, + panelId: panelId, + focusRequestGeneration: focusRequestGeneration, + canApplyFocusRequest: canApplyFocusRequest, + onFieldDidFocus: onFieldDidFocus, + onEscape: onClose, + onReturn: { isShift in + if isShift { + onPrevious() + } else { + onNext() + } + } + ) .frame(width: 180) .padding(.leading, 8) .padding(.trailing, 50) .padding(.vertical, 6) .background(Color.primary.opacity(0.1)) .cornerRadius(6) - .focused($isSearchFieldFocused) .overlay(alignment: .trailing) { if let selected = searchState.selected { let totalText = searchState.total.map { String($0) } ?? "?" @@ -102,18 +59,6 @@ struct BrowserSearchOverlay: View { .padding(.trailing, 8) } } - .onExitCommand { - onClose() - } - .onSubmit { - // onSubmit fires only after IME composition is committed. - if NSEvent.modifierFlags.contains(.shift) { - onPrevious() - } else { - onNext() - } - } - Button(action: { #if DEBUG dlog("browser.findbar.next panel=\(panelId.uuidString.prefix(5))") @@ -155,22 +100,7 @@ struct BrowserSearchOverlay: View { #if DEBUG dlog("browser.findbar.appear panel=\(panelId.uuidString.prefix(5))") #endif - logFocusState("appear") - requestSearchFieldFocus(origin: "appear") - } - .onChange(of: isSearchFieldFocused) { _, focused in - logFocusState("focusState.change next=\(focused ? 1 : 0)") - if focused { - onFieldDidFocus() - } - } - .onReceive(NotificationCenter.default.publisher(for: .browserSearchFocus)) { notification in - guard let notifiedPanelId = notification.object as? UUID, - notifiedPanelId == panelId else { return } - logFocusState("notification.received") - DispatchQueue.main.async { - requestSearchFieldFocus(origin: "notification") - } + isSearchFieldFocused = true } .background( GeometryReader { barGeo in @@ -249,3 +179,176 @@ struct BrowserSearchOverlay: View { return point.y < midY ? .topRight : .bottomRight } } + +private final class BrowserSearchNativeTextField: NSTextField { + override init(frame frameRect: NSRect) { + super.init(frame: frameRect) + isBordered = false + isBezeled = false + drawsBackground = false + focusRingType = .none + usesSingleLineMode = true + } + + required init?(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } +} + +private struct BrowserSearchTextFieldRepresentable: NSViewRepresentable { + @Binding var text: String + @Binding var isFocused: Bool + let panelId: UUID + let focusRequestGeneration: UInt64 + let canApplyFocusRequest: (UInt64) -> Bool + let onFieldDidFocus: () -> Void + let onEscape: () -> Void + let onReturn: (_ isShift: Bool) -> Void + + final class Coordinator: NSObject, NSTextFieldDelegate { + var parent: BrowserSearchTextFieldRepresentable + var isProgrammaticMutation = false + weak var parentField: BrowserSearchNativeTextField? + var pendingFocusRequest: Bool? + var searchFocusObserver: NSObjectProtocol? + + init(parent: BrowserSearchTextFieldRepresentable) { + self.parent = parent + } + + deinit { + if let searchFocusObserver { + NotificationCenter.default.removeObserver(searchFocusObserver) + } + } + + func controlTextDidChange(_ obj: Notification) { + guard !isProgrammaticMutation else { return } + guard let field = obj.object as? NSTextField else { return } + parent.text = field.stringValue + } + + func controlTextDidBeginEditing(_ obj: Notification) { + parent.onFieldDidFocus() + if !parent.isFocused { + DispatchQueue.main.async { + self.parent.isFocused = true + } + } + } + + func controlTextDidEndEditing(_ obj: Notification) { + if parent.isFocused { + DispatchQueue.main.async { + self.parent.isFocused = false + } + } + } + + func control(_ control: NSControl, textView: NSTextView, doCommandBy commandSelector: Selector) -> Bool { + switch commandSelector { + case #selector(NSResponder.cancelOperation(_:)): + if textView.hasMarkedText() { return false } + parent.onEscape() + return true + case #selector(NSResponder.insertNewline(_:)): + if textView.hasMarkedText() { return false } + let isShift = NSApp.currentEvent?.modifierFlags.contains(.shift) ?? false + parent.onReturn(isShift) + return true + default: + return false + } + } + } + + func makeCoordinator() -> Coordinator { + Coordinator(parent: self) + } + + func makeNSView(context: Context) -> BrowserSearchNativeTextField { + let field = BrowserSearchNativeTextField(frame: .zero) + field.font = .systemFont(ofSize: NSFont.systemFontSize) + field.placeholderString = String(localized: "search.placeholder", defaultValue: "Search") + field.setAccessibilityIdentifier("BrowserFindSearchTextField") + field.delegate = context.coordinator + field.target = nil + field.action = nil + field.isEditable = true + field.isSelectable = true + field.isEnabled = true + field.stringValue = text + context.coordinator.parentField = field + context.coordinator.searchFocusObserver = NotificationCenter.default.addObserver( + forName: .browserSearchFocus, + object: nil, + queue: .main + ) { [weak field, weak coordinator = context.coordinator] notification in + guard let field, let coordinator else { return } + guard let notifiedPanelId = notification.object as? UUID, + notifiedPanelId == coordinator.parent.panelId else { return } + guard coordinator.parent.canApplyFocusRequest(coordinator.parent.focusRequestGeneration) else { return } + guard let window = field.window else { return } + let fr = window.firstResponder + let alreadyFocused = fr === field || + field.currentEditor() != nil || + ((fr as? NSTextView)?.delegate as? NSTextField) === field + guard !alreadyFocused else { return } + window.makeFirstResponder(field) + } + return field + } + + func updateNSView(_ nsView: BrowserSearchNativeTextField, context: Context) { + context.coordinator.parent = self + context.coordinator.parentField = nsView + + if let editor = nsView.currentEditor() as? NSTextView { + if editor.string != text, !editor.hasMarkedText() { + context.coordinator.isProgrammaticMutation = true + editor.string = text + nsView.stringValue = text + context.coordinator.isProgrammaticMutation = false + } + } else if nsView.stringValue != text { + nsView.stringValue = text + } + + if let window = nsView.window { + let fr = window.firstResponder + let isFirstResponder = + fr === nsView || + nsView.currentEditor() != nil || + ((fr as? NSTextView)?.delegate as? NSTextField) === nsView + + if isFocused, + canApplyFocusRequest(focusRequestGeneration), + !isFirstResponder, + context.coordinator.pendingFocusRequest != true { + context.coordinator.pendingFocusRequest = true + DispatchQueue.main.async { [weak nsView, weak coordinator = context.coordinator] in + coordinator?.pendingFocusRequest = nil + guard let coordinator, + coordinator.parent.isFocused, + coordinator.parent.canApplyFocusRequest(coordinator.parent.focusRequestGeneration) else { return } + guard let nsView, let window = nsView.window else { return } + let fr = window.firstResponder + let alreadyFocused = fr === nsView || + nsView.currentEditor() != nil || + ((fr as? NSTextView)?.delegate as? NSTextField) === nsView + guard !alreadyFocused else { return } + window.makeFirstResponder(nsView) + } + } + } + } + + static func dismantleNSView(_ nsView: BrowserSearchNativeTextField, coordinator: Coordinator) { + if let observer = coordinator.searchFocusObserver { + NotificationCenter.default.removeObserver(observer) + coordinator.searchFocusObserver = nil + } + nsView.delegate = nil + coordinator.parentField = nil + } +} diff --git a/Sources/Panels/BrowserPanel.swift b/Sources/Panels/BrowserPanel.swift index 827b6b28..aed4cade 100644 --- a/Sources/Panels/BrowserPanel.swift +++ b/Sources/Panels/BrowserPanel.swift @@ -4721,6 +4721,8 @@ extension BrowserPanel { if created { searchState = BrowserSearchState() } + pendingAddressBarFocusRequestId = nil + NotificationCenter.default.post(name: .browserDidBlurAddressBar, object: id) let generation = beginSearchFocusRequest(reason: "startFind") #if DEBUG let window = webView.window diff --git a/Sources/Panels/BrowserPanelView.swift b/Sources/Panels/BrowserPanelView.swift index 83f072a8..70694a0e 100644 --- a/Sources/Panels/BrowserPanelView.swift +++ b/Sources/Panels/BrowserPanelView.swift @@ -1300,7 +1300,12 @@ struct BrowserPanelView: View { } private func shouldApplyAddressBarExitFallback(in window: NSWindow) -> Bool { - panel.webView.window === window && isPanelFocusedInModel() + // Navigation-triggered omnibar blur can still be unwinding when Cmd+F opens + // the browser find bar. Once find is visible, any delayed omnibar-exit + // handoff must not reclaim first responder for WebKit. + panel.webView.window === window && + isPanelFocusedInModel() && + panel.searchState == nil } #if DEBUG diff --git a/cmuxUITests/BrowserPaneNavigationKeybindUITests.swift b/cmuxUITests/BrowserPaneNavigationKeybindUITests.swift index 1c2bd61b..6e11c3c8 100644 --- a/cmuxUITests/BrowserPaneNavigationKeybindUITests.swift +++ b/cmuxUITests/BrowserPaneNavigationKeybindUITests.swift @@ -792,6 +792,67 @@ final class BrowserPaneNavigationKeybindUITests: XCTestCase { runFindFocusPersistenceScenario(route: .cmdOptionArrows, useAutofocusRacePage: true) } + func testCmdFFocusesBrowserFindFieldAfterCmdDCmdLNavigation() { + let app = XCUIApplication() + app.launchEnvironment["CMUX_SOCKET_PATH"] = socketPath + app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_RECORD_ONLY"] = "1" + app.launchEnvironment["CMUX_UI_TEST_GOTO_SPLIT_PATH"] = dataPath + launchAndEnsureForeground(app) + + let window = app.windows.firstMatch + // On some CI runners the app accepts key events before XCUI exposes the window tree. + _ = window.waitForExistence(timeout: 2.0) + + app.typeKey("d", modifierFlags: [.command]) + XCTAssertTrue( + waitForDataMatch(timeout: 6.0) { data in + guard data["lastSplitDirection"] == "right" else { return false } + guard let paneCountAfterSplit = Int(data["paneCountAfterSplit"] ?? "") else { return false } + return paneCountAfterSplit >= 2 + }, + "Expected Cmd+D to create a split before opening the browser. data=\(String(describing: loadData()))" + ) + + app.typeKey("l", modifierFlags: [.command]) + + let omnibar = app.textFields["BrowserOmnibarTextField"].firstMatch + XCTAssertTrue(omnibar.waitForExistence(timeout: 8.0), "Expected browser omnibar after Cmd+L") + + app.typeKey("a", modifierFlags: [.command]) + app.typeKey(XCUIKeyboardKey.delete.rawValue, modifierFlags: []) + app.typeText("example.com") + app.typeKey(XCUIKeyboardKey.return.rawValue, modifierFlags: []) + + XCTAssertTrue( + waitForOmnibarToContainExampleDomain(omnibar, timeout: 8.0), + "Expected browser navigation to example domain before opening find. value=\(String(describing: omnibar.value))" + ) + + app.typeKey("f", modifierFlags: [.command]) + + let findField = app.textFields["BrowserFindSearchTextField"].firstMatch + XCTAssertTrue(findField.waitForExistence(timeout: 6.0), "Expected browser find field after Cmd+F") + + let omnibarValueBeforeFindTyping = (omnibar.value as? String) ?? "" + app.typeText("needle") + + XCTAssertTrue( + waitForCondition(timeout: 4.0) { + ((findField.value as? String) ?? "") == "needle" + }, + "Expected Cmd+F to focus browser find after Cmd+D, Cmd+L, and navigation. " + + "findValue=\(String(describing: findField.value)) omnibarValue=\(String(describing: omnibar.value))" + ) + let omnibarValueAfterFindTyping = (omnibar.value as? String) ?? "" + XCTAssertFalse( + omnibarValueAfterFindTyping.contains("needle"), + "Expected typing after Cmd+F to stay out of the omnibar. " + + "omnibarValueBefore=\(omnibarValueBeforeFindTyping) " + + "omnibarValueAfter=\(String(describing: omnibar.value)) " + + "findValue=\(String(describing: findField.value))" + ) + } + private enum FindFocusRoute { case cmdOptionArrows case cmdCtrlLetters diff --git a/tests/test_ci_self_hosted_guard.sh b/tests/test_ci_self_hosted_guard.sh index 5e22c00f..5d034f2b 100755 --- a/tests/test_ci_self_hosted_guard.sh +++ b/tests/test_ci_self_hosted_guard.sh @@ -39,18 +39,18 @@ if ! awk ' exit 1 fi -# ui-display-resolution-regression: must use WarpBuild runner with fork guard (paid runner) +# ui-regressions: must use WarpBuild runner with fork guard (paid runner) if ! awk ' - /^ ui-display-resolution-regression:/ { in_tests=1; next } + /^ ui-regressions:/ { in_tests=1; next } in_tests && /^ [^[:space:]]/ { in_tests=0 } in_tests && /runs-on: warp-macos-15-arm64-6x/ { saw_warp=1 } in_tests && /github.event.pull_request.head.repo.full_name == github.repository/ { saw_guard=1 } END { exit !(saw_warp && saw_guard) } ' "$WORKFLOW_FILE"; then - echo "FAIL: ui-display-resolution-regression block must keep both warp-macos-15-arm64-6x runner and fork guard" + echo "FAIL: ui-regressions block must keep both warp-macos-15-arm64-6x runner and fork guard" exit 1 fi echo "PASS: tests WarpBuild runner fork guard is present" echo "PASS: tests-build-and-lag WarpBuild runner fork guard is present" -echo "PASS: ui-display-resolution-regression WarpBuild runner fork guard is present" +echo "PASS: ui-regressions WarpBuild runner fork guard is present"