Fix terminal find overlay crash and focus handoff (#1487)

* Add regression tests for terminal find overlay races

* Fix terminal find overlay crash and focus handoff
This commit is contained in:
Austin Wang 2026-03-15 18:32:24 -07:00 committed by GitHub
parent 6b138f7d9d
commit bd1788639d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 350 additions and 76 deletions

View file

@ -1696,6 +1696,32 @@ func shouldRouteTerminalFontZoomShortcutToGhostty(
) != nil
}
@discardableResult
func startOrFocusTerminalSearch(
_ terminalSurface: TerminalSurface,
searchFocusNotifier: @escaping (TerminalSurface) -> Void = {
NotificationCenter.default.post(name: .ghosttySearchFocus, object: $0)
}
) -> Bool {
if terminalSurface.searchState != nil {
searchFocusNotifier(terminalSurface)
return true
}
if terminalSurface.performBindingAction("start_search") {
DispatchQueue.main.async { [weak terminalSurface] in
guard let terminalSurface, terminalSurface.searchState == nil else { return }
terminalSurface.searchState = TerminalSurface.SearchState()
searchFocusNotifier(terminalSurface)
}
return true
}
terminalSurface.searchState = TerminalSurface.SearchState()
searchFocusNotifier(terminalSurface)
return true
}
/// Let AppKit own native Cmd+` window cycling so key-window changes do not
/// re-enter our direct-to-menu shortcut path.
func shouldRouteCommandEquivalentDirectlyToMainMenu(_ event: NSEvent) -> Bool {

View file

@ -5900,7 +5900,9 @@ final class GhosttySurfaceScrollView: NSView {
private let keyboardCopyModeBadgeIconView: NSImageView
private let keyboardCopyModeBadgeLabel: NSTextField
private var searchOverlayHostingView: NSHostingView<SurfaceSearchOverlay>?
private var deferredSearchOverlayMutationWorkItem: DispatchWorkItem?
private var lastSearchOverlayStateID: ObjectIdentifier?
private var searchOverlayMutationGeneration: UInt64 = 0
private var observers: [NSObjectProtocol] = []
private var windowObservers: [NSObjectProtocol] = []
private var isLiveScrolling = false
@ -6286,6 +6288,7 @@ final class GhosttySurfaceScrollView: NSView {
#endif
observers.forEach { NotificationCenter.default.removeObserver($0) }
windowObservers.forEach { NotificationCenter.default.removeObserver($0) }
deferredSearchOverlayMutationWorkItem?.cancel()
dropZoneOverlayView.removeFromSuperview()
cancelFocusRequest()
}
@ -6372,6 +6375,9 @@ final class GhosttySurfaceScrollView: NSView {
}
_ = setFrameIfNeeded(notificationRingOverlayView, to: bounds)
_ = setFrameIfNeeded(flashOverlayView, to: bounds)
if let overlay = searchOverlayHostingView {
_ = setFrameIfNeeded(overlay, to: bounds)
}
updateNotificationRingPath()
updateFlashPath(style: .standardFocus)
synchronizeScrollView()
@ -6609,50 +6615,42 @@ final class GhosttySurfaceScrollView: NSView {
CATransaction.commit()
}
func setSearchOverlay(searchState: TerminalSurface.SearchState?) {
if !Thread.isMainThread {
DispatchQueue.main.async { [weak self] in
self?.setSearchOverlay(searchState: searchState)
}
return
private func cancelDeferredSearchOverlayMutation() {
deferredSearchOverlayMutationWorkItem?.cancel()
deferredSearchOverlayMutationWorkItem = nil
}
private func scheduleDeferredSearchOverlayMutation(
generation: UInt64,
_ mutation: @escaping () -> Void
) {
cancelDeferredSearchOverlayMutation()
let work = DispatchWorkItem { [weak self] in
guard let self else { return }
guard self.searchOverlayMutationGeneration == generation else { return }
self.deferredSearchOverlayMutationWorkItem = nil
mutation()
}
deferredSearchOverlayMutationWorkItem = work
DispatchQueue.main.async(execute: work)
}
// Layering contract: keep terminal Cmd+F UI inside this portal-hosted AppKit view.
// SwiftUI panel-level overlays can fall behind portal-hosted terminal surfaces.
guard let terminalSurface = surfaceView.terminalSurface,
let searchState else {
let hadOverlay = searchOverlayHostingView != nil
lastSearchOverlayStateID = nil
guard hadOverlay else { return }
#if DEBUG
dlog("find.setSearchOverlay REMOVE surface=\(surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil") hadOverlay=\(hadOverlay)")
#endif
searchOverlayHostingView?.removeFromSuperview()
searchOverlayHostingView = nil
searchFocusTarget = .searchField
return
private func updateKeyboardCopyModeBadgeZOrder(relativeTo overlay: NSView?) {
guard !keyboardCopyModeBadgeContainerView.isHidden else { return }
if let overlay, overlay.superview === self {
addSubview(keyboardCopyModeBadgeContainerView, positioned: .above, relativeTo: overlay)
} else {
addSubview(keyboardCopyModeBadgeContainerView, positioned: .above, relativeTo: nil)
}
}
let searchStateID = ObjectIdentifier(searchState)
if let overlay = searchOverlayHostingView,
lastSearchOverlayStateID == searchStateID,
overlay.superview === self {
if !keyboardCopyModeBadgeContainerView.isHidden {
addSubview(keyboardCopyModeBadgeContainerView, positioned: .above, relativeTo: overlay)
}
return
}
let hadOverlay = searchOverlayHostingView != nil
#if DEBUG
dlog("find.setSearchOverlay MOUNT surface=\(terminalSurface.id.uuidString.prefix(5)) existingOverlay=\(hadOverlay ? "yes(update)" : "no(create)")")
#endif
let tabId = terminalSurface.tabId
let surfaceId = terminalSurface.id
let rootView = SurfaceSearchOverlay(
tabId: tabId,
surfaceId: surfaceId,
private func makeSearchOverlayRootView(
terminalSurface: TerminalSurface,
searchState: TerminalSurface.SearchState
) -> SurfaceSearchOverlay {
SurfaceSearchOverlay(
tabId: terminalSurface.tabId,
surfaceId: terminalSurface.id,
searchState: searchState,
onMoveFocusToTerminal: { [weak self] in
self?.searchFocusTarget = .terminal
@ -6670,41 +6668,165 @@ final class GhosttySurfaceScrollView: NSView {
self?.moveFocus()
}
)
}
private func findEditableSearchField(in view: NSView?) -> NSTextField? {
guard let view else { return nil }
if let field = view as? NSTextField, field.isEditable {
return field
}
for subview in view.subviews {
if let field = findEditableSearchField(in: subview) {
return field
}
}
return nil
}
private func requestMountedSearchFieldFocus(
generation: UInt64,
force: Bool,
attemptsRemaining: Int = 4
) {
guard searchOverlayMutationGeneration == generation else { return }
guard force || searchFocusTarget == .searchField else { return }
guard let overlay = searchOverlayHostingView,
overlay.superview === self,
let window,
window.isKeyWindow else { return }
guard let field = findEditableSearchField(in: overlay) else {
guard attemptsRemaining > 0 else { return }
DispatchQueue.main.asyncAfter(deadline: .now() + 0.03) { [weak self] in
self?.requestMountedSearchFieldFocus(
generation: generation,
force: force,
attemptsRemaining: attemptsRemaining - 1
)
}
return
}
let firstResponder = window.firstResponder
let alreadyFocused = firstResponder === field ||
field.currentEditor() != nil ||
((firstResponder as? NSTextView)?.delegate as? NSTextField) === field
guard !alreadyFocused else { return }
surfaceView.terminalSurface?.setFocus(false)
let result = window.makeFirstResponder(field)
#if DEBUG
dlog(
"find.mountedFieldFocus surface=\(surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil") " +
"result=\(result ? 1 : 0) attemptsRemaining=\(attemptsRemaining) " +
"firstResponder=\(String(describing: window.firstResponder))"
)
#endif
guard !result, attemptsRemaining > 0 else { return }
DispatchQueue.main.asyncAfter(deadline: .now() + 0.03) { [weak self] in
self?.requestMountedSearchFieldFocus(
generation: generation,
force: force,
attemptsRemaining: attemptsRemaining - 1
)
}
}
func setSearchOverlay(searchState: TerminalSurface.SearchState?) {
if !Thread.isMainThread {
DispatchQueue.main.async { [weak self] in
self?.setSearchOverlay(searchState: searchState)
}
return
}
searchOverlayMutationGeneration &+= 1
let mutationGeneration = searchOverlayMutationGeneration
// Layering contract: keep terminal Cmd+F UI inside this portal-hosted AppKit view.
// SwiftUI panel-level overlays can fall behind portal-hosted terminal surfaces.
guard let terminalSurface = surfaceView.terminalSurface,
let searchState else {
let hadOverlay = searchOverlayHostingView != nil
lastSearchOverlayStateID = nil
searchFocusTarget = .searchField
guard hadOverlay else {
cancelDeferredSearchOverlayMutation()
return
}
#if DEBUG
dlog("find.setSearchOverlay REMOVE surface=\(surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil") hadOverlay=\(hadOverlay)")
#endif
scheduleDeferredSearchOverlayMutation(generation: mutationGeneration) { [weak self] in
self?.searchOverlayHostingView?.removeFromSuperview()
self?.searchOverlayHostingView = nil
}
return
}
let searchStateID = ObjectIdentifier(searchState)
if let overlay = searchOverlayHostingView,
lastSearchOverlayStateID == searchStateID,
overlay.superview === self {
cancelDeferredSearchOverlayMutation()
_ = setFrameIfNeeded(overlay, to: bounds)
updateKeyboardCopyModeBadgeZOrder(relativeTo: overlay)
return
}
let hadOverlay = searchOverlayHostingView != nil
#if DEBUG
dlog("find.setSearchOverlay MOUNT surface=\(terminalSurface.id.uuidString.prefix(5)) existingOverlay=\(hadOverlay ? "yes(update)" : "no(create)")")
#endif
let rootView = makeSearchOverlayRootView(
terminalSurface: terminalSurface,
searchState: searchState
)
if let overlay = searchOverlayHostingView {
overlay.rootView = rootView
if overlay.superview !== self {
overlay.removeFromSuperview()
addSubview(overlay)
NSLayoutConstraint.activate([
overlay.topAnchor.constraint(equalTo: topAnchor),
overlay.bottomAnchor.constraint(equalTo: bottomAnchor),
overlay.leadingAnchor.constraint(equalTo: leadingAnchor),
overlay.trailingAnchor.constraint(equalTo: trailingAnchor),
])
}
if !keyboardCopyModeBadgeContainerView.isHidden {
addSubview(keyboardCopyModeBadgeContainerView, positioned: .above, relativeTo: overlay)
}
lastSearchOverlayStateID = searchStateID
if overlay.superview !== self {
scheduleDeferredSearchOverlayMutation(generation: mutationGeneration) { [weak self, weak overlay] in
guard let self, let overlay else { return }
overlay.removeFromSuperview()
overlay.frame = self.bounds
overlay.autoresizingMask = [.width, .height]
self.addSubview(overlay)
self.updateKeyboardCopyModeBadgeZOrder(relativeTo: overlay)
self.requestMountedSearchFieldFocus(
generation: mutationGeneration,
force: false
)
}
return
}
cancelDeferredSearchOverlayMutation()
_ = setFrameIfNeeded(overlay, to: bounds)
updateKeyboardCopyModeBadgeZOrder(relativeTo: overlay)
return
}
searchFocusTarget = .searchField
let overlay = NSHostingView(rootView: rootView)
overlay.translatesAutoresizingMaskIntoConstraints = false
addSubview(overlay)
NSLayoutConstraint.activate([
overlay.topAnchor.constraint(equalTo: topAnchor),
overlay.bottomAnchor.constraint(equalTo: bottomAnchor),
overlay.leadingAnchor.constraint(equalTo: leadingAnchor),
overlay.trailingAnchor.constraint(equalTo: trailingAnchor),
])
if !keyboardCopyModeBadgeContainerView.isHidden {
addSubview(keyboardCopyModeBadgeContainerView, positioned: .above, relativeTo: overlay)
}
overlay.frame = bounds
overlay.autoresizingMask = [.width, .height]
searchOverlayHostingView = overlay
lastSearchOverlayStateID = searchStateID
scheduleDeferredSearchOverlayMutation(generation: mutationGeneration) { [weak self, weak overlay] in
guard let self, let overlay else { return }
guard self.searchOverlayHostingView === overlay else { return }
overlay.removeFromSuperview()
overlay.frame = self.bounds
overlay.autoresizingMask = [.width, .height]
self.addSubview(overlay)
self.updateKeyboardCopyModeBadgeZOrder(relativeTo: overlay)
self.requestMountedSearchFieldFocus(
generation: mutationGeneration,
force: true
)
}
}
func syncKeyStateIndicator(text: String?) {
@ -6723,11 +6845,7 @@ final class GhosttySurfaceScrollView: NSView {
|| subviews.last !== keyboardCopyModeBadgeContainerView
keyboardCopyModeBadgeContainerView.isHidden = false
if needsReorder {
if let overlay = searchOverlayHostingView {
addSubview(keyboardCopyModeBadgeContainerView, positioned: .above, relativeTo: overlay)
} else {
addSubview(keyboardCopyModeBadgeContainerView, positioned: .above, relativeTo: nil)
}
updateKeyboardCopyModeBadgeZOrder(relativeTo: searchOverlayHostingView)
}
return
}
@ -7433,6 +7551,17 @@ final class GhosttySurfaceScrollView: NSView {
let surfaceShort = surfaceView.terminalSurface?.id.uuidString.prefix(5) ?? "nil"
switch searchFocusTarget {
case .searchField:
if let firstResponder = window.firstResponder,
isCurrentSurfaceSearchFieldResponder(firstResponder) {
surfaceView.terminalSurface?.setFocus(false)
#if DEBUG
dlog(
"find.restoreSearchFocus.skip surface=\(surfaceShort) target=searchField " +
"reason=alreadyFocused firstResponder=\(String(describing: firstResponder))"
)
#endif
return
}
if let firstResponder = window.firstResponder,
isSearchOverlayOrDescendant(firstResponder),
!isCurrentSurfaceSearchResponder(firstResponder) {
@ -7636,6 +7765,17 @@ final class GhosttySurfaceScrollView: NSView {
return view.isDescendant(of: self)
}
private func isCurrentSurfaceSearchFieldResponder(_ responder: NSResponder) -> Bool {
if let editor = responder as? NSTextView,
editor.isFieldEditor,
let editedView = editor.delegate as? NSTextField {
return editedView.isDescendant(of: self) && isSearchOverlayOrDescendant(editedView)
}
guard let textField = responder as? NSTextField else { return false }
return textField.isDescendant(of: self) && isSearchOverlayOrDescendant(textField)
}
#if DEBUG
struct DebugRenderStats {
let drawCount: Int

View file

@ -911,15 +911,16 @@ class TabManager: ObservableObject {
#endif
return
}
let wasNil = panel.searchState == nil
if wasNil {
panel.searchState = TerminalSurface.SearchState()
}
let hadExistingSearch = panel.searchState != nil
let handled = startOrFocusTerminalSearch(panel.surface)
#if DEBUG
dlog("find.startSearch workspace=\(panel.workspaceId.uuidString.prefix(5)) panel=\(panel.id.uuidString.prefix(5)) created=\(wasNil ? "yes" : "no(reuse)") firstResponder=\(String(describing: panel.surface.hostedView.window?.firstResponder))")
dlog(
"find.startSearch workspace=\(panel.workspaceId.uuidString.prefix(5)) " +
"panel=\(panel.id.uuidString.prefix(5)) existing=\(hadExistingSearch ? "yes" : "no") " +
"handled=\(handled ? 1 : 0) " +
"firstResponder=\(String(describing: panel.surface.hostedView.window?.firstResponder))"
)
#endif
NotificationCenter.default.post(name: .ghosttySearchFocus, object: panel.surface)
_ = panel.performBindingAction("start_search")
}
func searchSelection() {

View file

@ -12025,6 +12025,18 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
return nil
}
private func firstResponderOwnsTextField(_ firstResponder: NSResponder?, textField: NSTextField) -> Bool {
if firstResponder === textField {
return true
}
if let editor = firstResponder as? NSTextView,
editor.isFieldEditor,
editor.delegate as? NSTextField === textField {
return true
}
return false
}
func testTrackpadScrollRoutesToTerminalSurfaceAndPreservesKeyboardFocusPath() {
let window = NSWindow(
contentRect: NSRect(x: 0, y: 0, width: 360, height: 240),
@ -12157,12 +12169,105 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
let searchState = TerminalSurface.SearchState(needle: "example")
hostedView.setSearchOverlay(searchState: searchState)
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
XCTAssertTrue(hostedView.debugHasSearchOverlay())
hostedView.setSearchOverlay(searchState: nil)
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
XCTAssertFalse(hostedView.debugHasSearchOverlay())
}
func testRapidSearchOverlayToggleDoesNotLeaveStaleOverlayMounted() {
let surface = TerminalSurface(
tabId: UUID(),
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
configTemplate: nil,
workingDirectory: nil
)
let hostedView = surface.hostedView
hostedView.setSearchOverlay(searchState: TerminalSurface.SearchState(needle: "example"))
hostedView.setSearchOverlay(searchState: nil)
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
XCTAssertFalse(
hostedView.debugHasSearchOverlay(),
"A stale deferred mount must not resurrect the find overlay after it closes"
)
}
func testSearchOverlayFocusesSearchFieldAfterDeferredAttach() {
let surface = TerminalSurface(
tabId: UUID(),
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
configTemplate: nil,
workingDirectory: nil
)
let hostedView = surface.hostedView
let window = NSWindow(
contentRect: NSRect(x: 0, y: 0, width: 360, height: 240),
styleMask: [.titled, .closable],
backing: .buffered,
defer: false
)
defer { window.orderOut(nil) }
guard let contentView = window.contentView else {
XCTFail("Expected content view")
return
}
hostedView.frame = contentView.bounds
hostedView.autoresizingMask = [.width, .height]
contentView.addSubview(hostedView)
window.makeKeyAndOrderFront(nil)
window.displayIfNeeded()
contentView.layoutSubtreeIfNeeded()
hostedView.setVisibleInUI(true)
hostedView.setActive(true)
let searchState = TerminalSurface.SearchState(needle: "")
surface.searchState = searchState
hostedView.setSearchOverlay(searchState: searchState)
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
guard let searchField = findEditableTextField(in: hostedView) else {
XCTFail("Expected mounted find text field")
return
}
XCTAssertTrue(
firstResponderOwnsTextField(window.firstResponder, textField: searchField),
"Deferred search overlay attach should still move focus into the find field"
)
}
func testStartOrFocusTerminalSearchReusesExistingSearchState() {
let surface = TerminalSurface(
tabId: UUID(),
context: GHOSTTY_SURFACE_CONTEXT_SPLIT,
configTemplate: nil,
workingDirectory: nil
)
let existingSearchState = TerminalSurface.SearchState(needle: "existing")
surface.searchState = existingSearchState
var focusNotificationCount = 0
XCTAssertTrue(
startOrFocusTerminalSearch(surface) { _ in
focusNotificationCount += 1
}
)
XCTAssertTrue(surface.searchState === existingSearchState)
XCTAssertEqual(
focusNotificationCount,
1,
"Re-triggering terminal Find should refocus the existing overlay without recreating state"
)
}
func testEscapeDismissingFindOverlayDoesNotLeakEscapeKeyUpToTerminal() {
_ = NSApplication.shared
@ -12398,6 +12503,7 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
)
let hostedView = surface.hostedView
hostedView.setSearchOverlay(searchState: TerminalSurface.SearchState(needle: "split"))
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
XCTAssertTrue(hostedView.debugHasSearchOverlay())
portal.bind(hostedView: hostedView, to: anchorA, visibleInUI: true)
@ -12436,6 +12542,7 @@ final class GhosttySurfaceOverlayTests: XCTestCase {
)
let hostedView = surface.hostedView
hostedView.setSearchOverlay(searchState: TerminalSurface.SearchState(needle: "workspace"))
RunLoop.current.run(until: Date().addingTimeInterval(0.05))
XCTAssertTrue(hostedView.debugHasSearchOverlay())
portal.bind(hostedView: hostedView, to: anchor, visibleInUI: true)