fix(next): menus opened inside a native popup keep the popup open (bookmark bubble's Folder picker)

Packages/macOS/CmuxNext/Sources/CmuxNextDesign/Overlay/NativePopupHost.swift

@@ -12,6 +12,9 @@ public import AppKit
 /// - dismiss rules: a click outside the popup (``keepsOpen`` lists the clicks that do not count),
 ///   Escape in the popup, the app going inactive, the window moving, resizing, closing or
 ///   minimizing, and for a ``NativePopupPanel`` the popup losing the keys.
+/// - children: a menu opened from inside the popup (a picker's, a context menu) and a window
+///   parented under the panel are part of it. While one is up, its keys, clicks and Escape belong
+///   to it: a click, Escape or a pick closes only the inner menu, and the popup takes the keys back.
 ///
 /// The content (native rows, controls, SwiftUI) is the owner's.
 @MainActor
@@ -30,32 +33,109 @@ public final class NativePopupHost {
     private var generation = 0
     private var observers: [NSObjectProtocol] = []
     private var monitors: [Any] = []
+    /// The menus opened from inside the popup that are tracking now (their top menus).
+    private var childMenus: Set<ObjectIdentifier> = []
+    /// Where the host observes the app's and windows' notices (tests inject their own).
+    private let center: NotificationCenter
 
-    public init(panel: NSPanel) {
+    public init(panel: NSPanel, notificationCenter: NotificationCenter = .default) {
         self.panel = panel
+        center = notificationCenter
         if let keyed = panel as? NativePopupPanel {
-            keyed.onLostKeys = { [weak self, weak keyed] elsewhere in
-                guard let self, let keyed else { return }
-                // A press on the control that toggles the popup makes its window key first; the
-                // control closes the popup itself, so that key loss is not a dismiss.
+            keyed.onLostKeys = { [weak self] elsewhere in
                 let press = NSApp.currentEvent.flatMap { [.leftMouseDown, .rightMouseDown, .otherMouseDown].contains($0.type) ? $0 : nil }
-                if let press, press.window !== keyed, keepsOpen(press) { return }
-                // Shown without the keys, a window of the app took them without a press (Cmd-Tab
-                // into the app, a focus request): the popup takes them over, as a popup shown with
-                // the app active has them; it never takes them while the app is inactive
-                // (ActiveAppKeyPanel). A press elsewhere closes it through the click monitor.
-                if elsewhere, press == nil, isShown {
-                    keyed.makeKey()
-                    return
+                self?.lostKeys(elsewhere: elsewhere, press: press)
+            }
+        }
+    }
+
+    /// The panel lost the keys (``NativePopupPanel/onLostKeys``) during `press`, the mouse-down
+    /// being handled then (nil for none).
+    func lostKeys(elsewhere: Bool, press: NSEvent?) {
+        guard let keyed = panel as? NativePopupPanel else { return }
+        // A menu or window opened from inside the popup took the keys: the popup stays, and takes
+        // them back when the menu closes.
+        if hasOpenChild { return }
+        // A press on the control that toggles the popup makes its window key first; the control
+        // closes the popup itself, so that key loss is not a dismiss.
+        if let press, press.window !== keyed, keepsOpen(press) { return }
+        // A press inside the popup lost it the keys before a menu it opened began tracking (AppKit
+        // may resign the panel first). Once the press is handled (the menu's tracking, which
+        // starts inside it, is noted synchronously) the popup closes unless a child took them.
+        if let press, contains(press.window) {
+            let showing = generation
+            DispatchQueue.main.async { [weak self] in
+                MainActor.assumeIsolated { // main-proof: DispatchQueue.main
+                    guard let self, self.generation == showing, self.isShown, !self.hasOpenChild, !self.panel.isKeyWindow else { return }
+                    self.dismiss()
                 }
-                dismiss()
             }
+            return
+        }
+        // Shown without the keys, a window of the app took them without a press (Cmd-Tab into the
+        // app, a focus request): the popup takes them over, as a popup shown with the app active
+        // has them; it never takes them while the app is inactive (ActiveAppKeyPanel). A press
+        // elsewhere closes it through the click monitor.
+        if elsewhere, press == nil, isShown {
+            keyed.makeKey()
+            return
         }
+        dismiss()
     }
 
     /// The popup is on screen and not closing.
     public var isShown: Bool { panel.isVisible && !isClosing }
 
+    /// A menu opened from inside the popup is tracking, or the key window is one parented under it.
+    var hasOpenChild: Bool {
+        if !childMenus.isEmpty { return true }
+        guard let key = NSApp.keyWindow, key !== panel else { return false }
+        return contains(key)
+    }
+
+    /// `window` is the panel or a window parented under it.
+    private func contains(_ window: NSWindow?) -> Bool {
+        var window = window
+        while let current = window {
+            if current === panel { return true }
+            window = current.parent
+        }
+        return false
+    }
+
+    /// `menu` (a top menu) opened from inside the popup: it belongs to a view in the panel (a
+    /// pop-up button's, a view's context menu), or the event that opened it was in the popup.
+    /// The menu bar's menus never are.
+    private func opensFromPopup(_ menu: NSMenu) -> Bool {
+        if let main = NSApp.mainMenu, menu === main { return false }
+        if let root = panel.contentView, Self.view(in: root, owns: menu) { return true }
+        return contains(NSApp.currentEvent?.window)
+    }
+
+    private static func view(in view: NSView, owns menu: NSMenu) -> Bool {
+        if view.menu === menu { return true }
+        return view.subviews.contains { Self.view(in: $0, owns: menu) }
+    }
+
+    private static func topMenu(_ menu: NSMenu) -> NSMenu {
+        var top = menu
+        while let parent = top.supermenu { top = parent }
+        return top
+    }
+
+    private func menuBegan(_ menu: NSMenu) {
+        let top = Self.topMenu(menu)
+        guard isShown, opensFromPopup(top) else { return }
+        childMenus.insert(ObjectIdentifier(top))
+    }
+
+    private func menuEnded(_ menu: NSMenu) {
+        guard childMenus.remove(ObjectIdentifier(Self.topMenu(menu))) != nil, childMenus.isEmpty, isShown else { return }
+        // The menu gave the keys up: they come back to the popup (only while the app is active,
+        // for an ActiveAppKeyPanel).
+        if !panel.isKeyWindow { panel.makeKey() }
+    }
+
     /// The card's frame (screen), without the shadow band.
     public var cardFrame: CGRect { PopupStyle.standard.cardFrame(inWindow: panel.frame) }
 
@@ -136,7 +216,7 @@ public final class NativePopupHost {
 
     private func watch(_ window: NSWindow) {
         guard observers.isEmpty else { return }
-        let center = NotificationCenter.default
+        let center = self.center
         let names: [(NSNotification.Name, AnyObject?)] = [
             (NSApplication.didResignActiveNotification, nil), (NSWindow.willMoveNotification, window),
             (NSWindow.didResizeNotification, window), (NSWindow.willCloseNotification, window),
@@ -147,25 +227,38 @@ public final class NativePopupHost {
                 Task { @MainActor in self?.dismiss() }
             }
         }
+        // A menu opened from inside the popup, noted as AppKit posts it (no queue: delivered at
+        // once on the posting thread, inside the press that opens the menu) so a key loss checked
+        // after the press sees it.
+        observers.append(center.addObserver(forName: NSMenu.didBeginTrackingNotification, object: nil, queue: nil) { [weak self] note in
+            guard Thread.isMainThread, let menu = note.object as? NSMenu else { return } // main-proof: returns off the main thread
+            MainActor.assumeIsolated { self?.menuBegan(menu) }
+        })
+        observers.append(center.addObserver(forName: NSMenu.didEndTrackingNotification, object: nil, queue: nil) { [weak self] note in
+            guard Thread.isMainThread, let menu = note.object as? NSMenu else { return } // main-proof: returns off the main thread
+            MainActor.assumeIsolated { self?.menuEnded(menu) }
+        })
         let clicks = NSEvent.addLocalMonitorForEvents(matching: [.leftMouseDown, .rightMouseDown, .otherMouseDown]) { [weak self] event in
-            guard let self, event.window !== self.panel, !self.keepsOpen(event) else { return event }
+            guard let self, !self.contains(event.window), !self.hasOpenChild, !self.keepsOpen(event) else { return event }
             self.dismiss()
             return event
         }
-        // Escape in the popup closes it (a text field in it would otherwise take it as cancel).
+        // Escape in the popup closes it (a text field in it would otherwise take it as cancel);
+        // with a menu of the popup open, Escape closes only that menu.
         let escape = NSEvent.addLocalMonitorForEvents(matching: [.keyDown]) { [weak self] event in
-            guard let self, event.window === self.panel, event.keyCode == 53, self.isShown else { return event }
+            guard let self, event.window === self.panel, event.keyCode == 53, self.isShown, self.childMenus.isEmpty else { return event }
             self.dismiss()
             return nil
         }
         monitors = [clicks, escape].compactMap { $0 }
     }
 
     private func unwatch() {
-        observers.forEach(NotificationCenter.default.removeObserver)
+        observers.forEach(center.removeObserver)
         observers = []
         monitors.forEach(NSEvent.removeMonitor)
         monitors = []
+        childMenus = []
     }
 }
 

Packages/macOS/CmuxNext/Tests/CmuxNextDesignTests/NativePopupHostChildMenuTests.swift

@@ -0,0 +1,127 @@
+import AppKit
+@testable import CmuxNextDesign
+import Testing
+
+/// The bookmark edit bubble (a ``NativePopupHost`` panel) holds a Folder picker whose menu is
+/// AppKit's, a window of its own. Opening it took the keys from the bubble, and the host read
+/// that key loss as a dismiss: a click on the picker closed the bubble. A menu opened from inside
+/// a popup is part of it: a click, Escape or a pick closes only the menu, and the popup stays.
+@MainActor @Suite(.serialized) struct NativePopupHostChildMenuTests {
+    /// The notices this test posts go here only (a fresh center per test).
+    private let center = NotificationCenter()
+
+    /// The bubble's shape: a visible window, and over it a keyed popup panel whose card holds the
+    /// Folder picker (a `ThemedPopUpButton`, as `ThemedPicker` makes). The host observes `center`,
+    /// the test's own, so the menu notices reach no other observer in the process.
+    private static func bubble(center: NotificationCenter) throws -> (NSWindow, NativePopupHost, ThemedPopUpButton) {
+        let window = NSWindow(contentRect: NSRect(x: 240, y: 240, width: 400, height: 300), styleMask: [.titled],
+                              backing: .buffered, defer: false)
+        window.isReleasedWhenClosed = false
+        window.orderFrontRegardless()
+        try #require(window.screen != nil)
+        let panel = NativePopupPanel(identifier: "cmux.tests.popup-child-menu")
+        let picker = ThemedPopUpButton(frame: NSRect(x: 0, y: 0, width: 160, height: 24), pullsDown: false)
+        for title in ["Bookmarks Bar", "    Work", "Other Bookmarks"] {
+            picker.menu?.addItem(withTitle: title, action: nil, keyEquivalent: "")
+        }
+        let content = NSView(frame: NSRect(x: 0, y: 0, width: 220, height: 80))
+        content.addSubview(picker)
+        panel.adoptPopupStyle(card: Glass.makeOverlayPanel(content: content, cornerRadius: PopupStyle.standard.cornerRadius))
+        let host = NativePopupHost(panel: panel, notificationCenter: center)
+        let anchor = CGRect(x: window.frame.midX, y: window.frame.maxY - 60, width: 20, height: 20)
+        try #require(host.show(size: CGSize(width: 220, height: 80), anchor: anchor, in: window) != nil)
+        return (window, host, picker)
+    }
+
+    private static func close(_ window: NSWindow, _ host: NativePopupHost) {
+        host.closePanel()
+        window.close()
+    }
+
+    /// Fails before the fix: the picker's menu took the keys and the bubble closed under it.
+    @Test func thePickersMenuTakingTheKeysLeavesTheBubbleOpen() throws {
+        let (window, host, picker) = try Self.bubble(center: center)
+        defer { Self.close(window, host) }
+        var dismissed = 0
+        host.onDismiss = { dismissed += 1 }
+        let menu = try #require(picker.menu)
+
+        center.post(name: NSMenu.didBeginTrackingNotification, object: menu)
+        // The menu's window becomes key: the popup panel resigns.
+        host.panel.resignKey()
+        #expect(host.isShown, "opening the Folder picker closed the bubble")
+        #expect(dismissed == 0)
+
+        // A pick (or a click elsewhere, or Escape) ends the menu; the bubble stays.
+        picker.selectItem(at: 1)
+        center.post(name: NSMenu.didEndTrackingNotification, object: menu)
+        #expect(host.isShown, "closing the Folder picker's menu closed the bubble")
+        #expect(dismissed == 0)
+    }
+
+    /// Passes before and after: with no menu of its own open, the bubble losing the keys (another
+    /// window or a menu that is not the bubble's took them) still closes it.
+    @Test func losingTheKeysToSomethingElseStillClosesTheBubble() throws {
+        let (window, host, _) = try Self.bubble(center: center)
+        defer { Self.close(window, host) }
+        var dismissed = 0
+        host.onDismiss = { dismissed += 1 }
+        let elsewhere = NSMenu(title: "Elsewhere")
+        elsewhere.addItem(withTitle: "Item", action: nil, keyEquivalent: "")
+
+        center.post(name: NSMenu.didBeginTrackingNotification, object: elsewhere)
+        host.panel.resignKey()
+        center.post(name: NSMenu.didEndTrackingNotification, object: elsewhere)
+
+        #expect(!host.isShown)
+        #expect(dismissed == 1)
+    }
+
+    /// A press in the popup at its window (the event AppKit is handling when the panel resigns).
+    private static func press(in panel: NSPanel) throws -> NSEvent {
+        try #require(NSEvent.mouseEvent(with: .leftMouseDown, location: NSPoint(x: 20, y: 20), modifierFlags: [],
+                                        timestamp: ProcessInfo.processInfo.systemUptime, windowNumber: panel.windowNumber,
+                                        context: nil, eventNumber: 0, clickCount: 1, pressure: 1))
+    }
+
+    /// The main queue gets a turn (the check a key loss during a press schedules).
+    private static func afterThePress() async throws {
+        try await Task.sleep(for: .milliseconds(50))
+    }
+
+    /// AppKit may resign the panel before the picker's menu begins tracking: the press in the
+    /// popup holds the dismiss until the menu shows, and the menu keeps the bubble open.
+    @Test func theKeysLostBeforeThePickersMenuBeginsLeaveTheBubbleOpen() async throws {
+        let (window, host, picker) = try Self.bubble(center: center)
+        defer { Self.close(window, host) }
+        var dismissed = 0
+        host.onDismiss = { dismissed += 1 }
+        let menu = try #require(picker.menu)
+
+        host.lostKeys(elsewhere: false, press: try Self.press(in: host.panel))
+        center.post(name: NSMenu.didBeginTrackingNotification, object: menu)
+        try await Self.afterThePress()
+        #expect(host.isShown, "the Folder picker's menu closed the bubble")
+
+        center.post(name: NSMenu.didEndTrackingNotification, object: menu)
+        try await Self.afterThePress()
+        #expect(host.isShown)
+        #expect(dismissed == 0)
+    }
+
+    /// A press in the popup that gives the keys to a window that is not its child (no menu of the
+    /// popup began) still closes it: the popup never sticks open.
+    @Test func aPressInThePopupThatGivesTheKeysAwayStillClosesIt() async throws {
+        let (window, host, _) = try Self.bubble(center: center)
+        defer { Self.close(window, host) }
+        var dismissed = 0
+        host.onDismiss = { dismissed += 1 }
+
+        host.lostKeys(elsewhere: false, press: try Self.press(in: host.panel))
+        #expect(host.isShown, "the check waits until the press is handled")
+        try await Self.afterThePress()
+
+        #expect(!host.isShown, "a press that opened no menu of the popup left it stuck open")
+        #expect(dismissed == 1)
+    }
+}