From a5d724cf28f8977ea993f99ab49778d92a6e5b2f Mon Sep 17 00:00:00 2001 From: Lawrence Chen <54008264+lawrencecchen@users.noreply.github.com> Date: Fri, 20 Feb 2026 16:09:42 -0800 Subject: [PATCH] Unify drag overlay routing and add regression coverage --- Sources/ContentView.swift | 257 ++++++++++++------- Sources/TerminalController.swift | 63 +++++ tests/cmux.py | 13 + tests/test_bonsplit_tab_drag_overlay_gate.py | 116 +++++++++ 4 files changed, 350 insertions(+), 99 deletions(-) create mode 100644 tests/test_bonsplit_tab_drag_overlay_gate.py diff --git a/Sources/ContentView.swift b/Sources/ContentView.swift index e5b5bea3..062318ad 100644 --- a/Sources/ContentView.swift +++ b/Sources/ContentView.swift @@ -160,6 +160,40 @@ final class SidebarState: ObservableObject { // MARK: - File Drop Overlay +enum DragOverlayRoutingPolicy { + static let bonsplitTabTransferType = NSPasteboard.PasteboardType("com.splittabbar.tabtransfer") + static let sidebarTabReorderType = NSPasteboard.PasteboardType(SidebarTabDragPayload.typeIdentifier) + + static func shouldCaptureFileDropOverlay( + pasteboardTypes: [NSPasteboard.PasteboardType]?, + eventType: NSEvent.EventType? + ) -> Bool { + guard let pasteboardTypes, pasteboardTypes.contains(.fileURL) else { return false } + guard isDragMouseEvent(eventType) else { return false } + + // Prefer explicit non-file drag types so stale fileURL entries cannot hijack + // Bonsplit tab drags or sidebar tab reorder drags. + if pasteboardTypes.contains(bonsplitTabTransferType) { return false } + if pasteboardTypes.contains(sidebarTabReorderType) { return false } + return true + } + + static func shouldCaptureSidebarExternalOverlay( + draggedTabId: UUID?, + pasteboardTypes: [NSPasteboard.PasteboardType]? + ) -> Bool { + guard draggedTabId != nil else { return false } + guard let pasteboardTypes else { return false } + return pasteboardTypes.contains(sidebarTabReorderType) + } + + private static func isDragMouseEvent(_ eventType: NSEvent.EventType?) -> Bool { + eventType == .leftMouseDragged + || eventType == .rightMouseDragged + || eventType == .otherMouseDragged + } +} + /// Transparent NSView installed on the window's theme frame (above the NSHostingView) to /// handle file/URL drags from Finder. Nested NSHostingController layers (created by bonsplit's /// SinglePaneWrapper) prevent AppKit's NSDraggingDestination routing from reaching deeply @@ -182,22 +216,15 @@ final class FileDropOverlayView: NSView { required init?(coder: NSCoder) { fatalError("init(coder:) not implemented") } - // MARK: Hit-testing — only participate when the system drag pasteboard contains file - // URLs (i.e. a Finder file drag is in progress). For everything else — mouse events, - // sidebar tab reorder, bonsplit tab drags — return nil so events route to the content - // view below and SwiftUI / bonsplit drag-and-drop works normally. + // MARK: Hit-testing — participation is routed by DragOverlayRoutingPolicy so + // file-drop, bonsplit tab drags, and sidebar tab reorder drags cannot conflict. override func hitTest(_ point: NSPoint) -> NSView? { let pb = NSPasteboard(name: .drag) - guard let types = pb.types, types.contains(.fileURL) else { return nil } - - // The drag pasteboard can retain stale file types after a completed drag. - // Only participate during active drag-motion events. - let eventType = NSApp.currentEvent?.type - let isDragMouseEvent = eventType == .leftMouseDragged - || eventType == .rightMouseDragged - || eventType == .otherMouseDragged - guard isDragMouseEvent else { return nil } + guard DragOverlayRoutingPolicy.shouldCaptureFileDropOverlay( + pasteboardTypes: pb.types, + eventType: NSApp.currentEvent?.type + ) else { return nil } return super.hitTest(point) } @@ -659,13 +686,11 @@ struct ContentView: View { return dir.isEmpty ? nil : dir } - var body: some View { - let useOverlay = sidebarBlendMode == SidebarBlendModeOption.withinWindow.rawValue - - Group { - if useOverlay { - // Overlay mode: terminal extends full width, sidebar on top - // This allows withinWindow blur to see the terminal content + private var contentAndSidebarLayout: AnyView { + if sidebarBlendMode == SidebarBlendModeOption.withinWindow.rawValue { + // Overlay mode: terminal extends full width, sidebar on top + // This allows withinWindow blur to see the terminal content + return AnyView( ZStack(alignment: .leading) { terminalContentWithSidebarDropOverlay .padding(.leading, sidebarState.isVisible ? sidebarWidth : 0) @@ -673,26 +698,35 @@ struct ContentView: View { sidebarView } } - } else { - // Standard HStack mode for behindWindow blur - HStack(spacing: 0) { - if sidebarState.isVisible { - sidebarView - } - terminalContentWithSidebarDropOverlay + ) + } + + // Standard HStack mode for behindWindow blur + return AnyView( + HStack(spacing: 0) { + if sidebarState.isVisible { + sidebarView } + terminalContentWithSidebarDropOverlay } - } - .overlay(alignment: .topLeading) { - if isFullScreen && sidebarState.isVisible { - fullscreenControls - .padding(.leading, 10) - .padding(.top, 4) - } - } - .frame(minWidth: 800, minHeight: 600) - .background(Color.clear) - .onAppear { + ) + } + + var body: some View { + var view = AnyView( + contentAndSidebarLayout + .overlay(alignment: .topLeading) { + if isFullScreen && sidebarState.isVisible { + fullscreenControls + .padding(.leading, 10) + .padding(.top, 4) + } + } + .frame(minWidth: 800, minHeight: 600) + .background(Color.clear) + ) + + view = AnyView(view.onAppear { tabManager.applyWindowBackgroundForSelectedTab() reconcileMountedWorkspaceIds() previousSelectedWorkspaceId = tabManager.selectedTabId @@ -701,8 +735,9 @@ struct ContentView: View { lastSidebarSelectionIndex = tabManager.tabs.firstIndex { $0.id == selectedId } } updateTitlebarText() - } - .onChange(of: tabManager.selectedTabId) { newValue in + }) + + view = AnyView(view.onChange(of: tabManager.selectedTabId) { newValue in #if DEBUG if let snapshot = tabManager.debugCurrentWorkspaceSwitchSnapshot() { let dtMs = (CACurrentMediaTime() - snapshot.startedAt) * 1000 @@ -722,8 +757,9 @@ struct ContentView: View { lastSidebarSelectionIndex = tabManager.tabs.firstIndex { $0.id == newValue } } updateTitlebarText() - } - .onChange(of: tabManager.isWorkspaceCycleHot) { _ in + }) + + view = AnyView(view.onChange(of: tabManager.isWorkspaceCycleHot) { _ in #if DEBUG if let snapshot = tabManager.debugCurrentWorkspaceSwitchSnapshot() { let dtMs = (CACurrentMediaTime() - snapshot.startedAt) * 1000 @@ -735,37 +771,45 @@ struct ContentView: View { } #endif reconcileMountedWorkspaceIds() - } - .onChange(of: retiringWorkspaceId) { _ in + }) + + view = AnyView(view.onChange(of: retiringWorkspaceId) { _ in reconcileMountedWorkspaceIds() - } - .onReceive(NotificationCenter.default.publisher(for: .ghosttyDidSetTitle)) { notification in + }) + + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: .ghosttyDidSetTitle)) { notification in guard let tabId = notification.userInfo?[GhosttyNotificationKey.tabId] as? UUID, tabId == tabManager.selectedTabId else { return } updateTitlebarText() - } - .onReceive(NotificationCenter.default.publisher(for: .ghosttyDidFocusTab)) { _ in + }) + + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: .ghosttyDidFocusTab)) { _ in sidebarSelectionState.selection = .tabs updateTitlebarText() - } - .onReceive(NotificationCenter.default.publisher(for: .ghosttyDidFocusSurface)) { notification in + }) + + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: .ghosttyDidFocusSurface)) { notification in guard let tabId = notification.userInfo?[GhosttyNotificationKey.tabId] as? UUID, tabId == tabManager.selectedTabId else { return } completeWorkspaceHandoffIfNeeded(focusedTabId: tabId, reason: "focus") updateTitlebarText() - } - .onReceive(NotificationCenter.default.publisher(for: .ghosttyConfigDidReload)) { _ in + }) + + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: Notification.Name("ghosttyConfigDidReload"))) { _ in titlebarThemeGeneration &+= 1 - } - .onReceive(NotificationCenter.default.publisher(for: .ghosttyDefaultBackgroundDidChange)) { _ in + }) + + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: Notification.Name("ghosttyDefaultBackgroundDidChange"))) { _ in titlebarThemeGeneration &+= 1 - } - .onReceive(NotificationCenter.default.publisher(for: .ghosttyDidBecomeFirstResponderSurface)) { notification in + }) + + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: .ghosttyDidBecomeFirstResponderSurface)) { notification in guard let tabId = notification.userInfo?[GhosttyNotificationKey.tabId] as? UUID, tabId == tabManager.selectedTabId else { return } completeWorkspaceHandoffIfNeeded(focusedTabId: tabId, reason: "first_responder") - } - .onReceive(tabManager.$tabs) { tabs in + }) + + view = AnyView(view.onReceive(tabManager.$tabs) { tabs in let existingIds = Set(tabs.map { $0.id }) if let retiringWorkspaceId, !existingIds.contains(retiringWorkspaceId) { self.retiringWorkspaceId = nil @@ -787,8 +831,9 @@ struct ContentView: View { lastSidebarSelectionIndex = nil } } - } - .onReceive(NotificationCenter.default.publisher(for: SidebarDragLifecycleNotification.stateDidChange)) { notification in + }) + + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: SidebarDragLifecycleNotification.stateDidChange)) { notification in let tabId = SidebarDragLifecycleNotification.tabId(from: notification) sidebarDraggedTabId = tabId #if DEBUG @@ -797,60 +842,67 @@ struct ContentView: View { "reason=\(SidebarDragLifecycleNotification.reason(from: notification))" ) #endif - } - .onPreferenceChange(SidebarFramePreferenceKey.self) { frame in + }) + + view = AnyView(view.onPreferenceChange(SidebarFramePreferenceKey.self) { frame in sidebarMinX = frame.minX - } - .onChange(of: bgGlassTintHex) { _ in + }) + + view = AnyView(view.onChange(of: bgGlassTintHex) { _ in updateWindowGlassTint() - } - .onChange(of: bgGlassTintOpacity) { _ in + }) + + view = AnyView(view.onChange(of: bgGlassTintOpacity) { _ in updateWindowGlassTint() - } - .onReceive(NotificationCenter.default.publisher(for: NSWindow.didEnterFullScreenNotification)) { notification in + }) + + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: NSWindow.didEnterFullScreenNotification)) { notification in guard let window = notification.object as? NSWindow, window === observedWindow else { return } isFullScreen = true setTitlebarControlsHidden(true, in: window) AppDelegate.shared?.fullscreenControlsViewModel = fullscreenControlsViewModel - } - .onReceive(NotificationCenter.default.publisher(for: NSWindow.didExitFullScreenNotification)) { notification in + }) + + view = AnyView(view.onReceive(NotificationCenter.default.publisher(for: NSWindow.didExitFullScreenNotification)) { notification in guard let window = notification.object as? NSWindow, window === observedWindow else { return } isFullScreen = false setTitlebarControlsHidden(false, in: window) AppDelegate.shared?.fullscreenControlsViewModel = nil - } - .ignoresSafeArea() - .background(WindowAccessor { [sidebarBlendMode, bgGlassEnabled, bgGlassTintHex, bgGlassTintOpacity] window in - window.identifier = NSUserInterfaceItemIdentifier(windowIdentifier) - window.titlebarAppearsTransparent = true - // Do not make the entire background draggable; it interferes with drag gestures - // like sidebar tab reordering in multi-window mode. - window.isMovableByWindowBackground = false - window.styleMask.insert(.fullSizeContentView) + }) - // Track this window for fullscreen notifications - if observedWindow !== window { - DispatchQueue.main.async { - observedWindow = window - isFullScreen = window.styleMask.contains(.fullScreen) - } - } + view = AnyView(view.ignoresSafeArea()) - // Keep content below the titlebar so drags on Bonsplit's tab bar don't - // get interpreted as window drags. - let computedTitlebarHeight = window.frame.height - window.contentLayoutRect.height - let nextPadding = max(28, min(72, computedTitlebarHeight)) - if abs(titlebarPadding - nextPadding) > 0.5 { - DispatchQueue.main.async { - titlebarPadding = nextPadding - } + view = AnyView(view.background(WindowAccessor { [sidebarBlendMode, bgGlassEnabled, bgGlassTintHex, bgGlassTintOpacity] window in + window.identifier = NSUserInterfaceItemIdentifier(windowIdentifier) + window.titlebarAppearsTransparent = true + // Do not make the entire background draggable; it interferes with drag gestures + // like sidebar tab reordering in multi-window mode. + window.isMovableByWindowBackground = false + window.styleMask.insert(.fullSizeContentView) + + // Track this window for fullscreen notifications + if observedWindow !== window { + DispatchQueue.main.async { + observedWindow = window + isFullScreen = window.styleMask.contains(.fullScreen) } + } + + // Keep content below the titlebar so drags on Bonsplit's tab bar don't + // get interpreted as window drags. + let computedTitlebarHeight = window.frame.height - window.contentLayoutRect.height + let nextPadding = max(28, min(72, computedTitlebarHeight)) + if abs(titlebarPadding - nextPadding) > 0.5 { + DispatchQueue.main.async { + titlebarPadding = nextPadding + } + } #if DEBUG - if ProcessInfo.processInfo.environment["CMUX_UI_TEST_MODE"] == "1" { - UpdateLogStore.shared.append("ui test window accessor: id=\(windowIdentifier) visible=\(window.isVisible)") - } + if ProcessInfo.processInfo.environment["CMUX_UI_TEST_MODE"] == "1" { + UpdateLogStore.shared.append("ui test window accessor: id=\(windowIdentifier) visible=\(window.isVisible)") + } #endif // Background glass: skip on macOS 26+ where NSGlassEffectView can cause blank // or incorrectly tinted SwiftUI content. Keep native window rendering there so @@ -886,7 +938,9 @@ struct ContentView: View { sidebarSelectionState: sidebarSelectionState ) installFileDropOverlay(on: window, tabManager: tabManager) - }) + })) + + return view } private func reconcileMountedWorkspaceIds(tabs: [Workspace]? = nil, selectedId: UUID? = nil) { @@ -1360,9 +1414,14 @@ private struct SidebarExternalDropOverlay: View { let draggedTabId: UUID? var body: some View { + let dragPasteboardTypes = NSPasteboard(name: .drag).types + let shouldCapture = DragOverlayRoutingPolicy.shouldCaptureSidebarExternalOverlay( + draggedTabId: draggedTabId, + pasteboardTypes: dragPasteboardTypes + ) Color.clear .contentShape(Rectangle()) - .allowsHitTesting(draggedTabId != nil) + .allowsHitTesting(shouldCapture) .onDrop( of: [SidebarTabDragPayload.typeIdentifier], delegate: SidebarExternalDropDelegate(draggedTabId: draggedTabId) diff --git a/Sources/TerminalController.swift b/Sources/TerminalController.swift index 96166d03..5f9b3d50 100644 --- a/Sources/TerminalController.swift +++ b/Sources/TerminalController.swift @@ -481,12 +481,18 @@ class TerminalController { case "seed_drag_pasteboard_tabtransfer": return seedDragPasteboardTabTransfer() + case "seed_drag_pasteboard_sidebar_reorder": + return seedDragPasteboardSidebarReorder() + case "clear_drag_pasteboard": return clearDragPasteboard() case "drop_hit_test": return dropHitTest(args) + case "overlay_hit_gate": + return overlayHitGate(args) + case "activate_app": return activateApp() @@ -6277,8 +6283,10 @@ class TerminalController { simulate_file_drop - Simulate dropping file path(s) on terminal (test-only) seed_drag_pasteboard_fileurl - Seed NSDrag pasteboard with public.file-url (test-only) seed_drag_pasteboard_tabtransfer - Seed NSDrag pasteboard with tab transfer type (test-only) + seed_drag_pasteboard_sidebar_reorder - Seed NSDrag pasteboard with sidebar reorder type (test-only) clear_drag_pasteboard - Clear NSDrag pasteboard (test-only) drop_hit_test - Hit-test file-drop overlay at normalised coords (test-only) + overlay_hit_gate - Return true/false if file-drop overlay would capture hit-testing for event type (test-only) activate_app - Bring app + main window to front (test-only) is_terminal_focused - Return true/false if terminal surface is first responder (test-only) read_terminal_text [id|idx] - Read visible terminal text (base64, test-only) @@ -6522,6 +6530,15 @@ class TerminalController { return "OK" } + private func seedDragPasteboardSidebarReorder() -> String { + DispatchQueue.main.sync { + _ = NSPasteboard(name: .drag).declareTypes([ + DragOverlayRoutingPolicy.sidebarTabReorderType + ], owner: nil) + } + return "OK" + } + private func clearDragPasteboard() -> String { DispatchQueue.main.sync { _ = NSPasteboard(name: .drag).clearContents() @@ -6529,6 +6546,52 @@ class TerminalController { return "OK" } + private func overlayHitGate(_ args: String) -> String { + let token = args.trimmingCharacters(in: .whitespacesAndNewlines).lowercased() + guard !token.isEmpty else { + return "ERROR: Usage: overlay_hit_gate " + } + + let eventType: NSEvent.EventType? + switch token { + case "leftmousedragged": + eventType = .leftMouseDragged + case "rightmousedragged": + eventType = .rightMouseDragged + case "othermousedragged": + eventType = .otherMouseDragged + case "leftmousedown": + eventType = .leftMouseDown + case "leftmouseup": + eventType = .leftMouseUp + case "rightmousedown": + eventType = .rightMouseDown + case "rightmouseup": + eventType = .rightMouseUp + case "othermousedown": + eventType = .otherMouseDown + case "othermouseup": + eventType = .otherMouseUp + case "scrollwheel": + eventType = .scrollWheel + case "none": + eventType = nil + default: + return "ERROR: Unknown event type '\(args.trimmingCharacters(in: .whitespacesAndNewlines))'" + } + + var shouldCapture = false + DispatchQueue.main.sync { + let pb = NSPasteboard(name: .drag) + shouldCapture = DragOverlayRoutingPolicy.shouldCaptureFileDropOverlay( + pasteboardTypes: pb.types, + eventType: eventType + ) + } + + return shouldCapture ? "true" : "false" + } + /// Hit-tests the file-drop overlay's coordinate-to-terminal mapping. /// Takes normalised (0-1) x,y within the content area where (0,0) is the /// top-left corner and (1,1) is the bottom-right corner. Returns the diff --git a/tests/cmux.py b/tests/cmux.py index 224825d2..1be85e10 100755 --- a/tests/cmux.py +++ b/tests/cmux.py @@ -845,12 +845,25 @@ class cmux: if not response.startswith("OK"): raise cmuxError(response) + def seed_drag_pasteboard_sidebar_reorder(self) -> None: + """Seed NSDrag pasteboard with sidebar reorder type in the app process (debug builds only).""" + response = self._send_command("seed_drag_pasteboard_sidebar_reorder") + if not response.startswith("OK"): + raise cmuxError(response) + def clear_drag_pasteboard(self) -> None: """Clear NSDrag pasteboard in the app process (debug builds only).""" response = self._send_command("clear_drag_pasteboard") if not response.startswith("OK"): raise cmuxError(response) + def overlay_hit_gate(self, event_type: str) -> bool: + """Return whether FileDropOverlayView would capture hit-testing for event_type.""" + response = self._send_command(f"overlay_hit_gate {event_type}") + if response.startswith("ERROR"): + raise cmuxError(response) + return response.strip().lower() == "true" + def drop_hit_test(self, x: float, y: float) -> Optional[str]: """Hit-test the file-drop overlay at normalised (0-1) coords. diff --git a/tests/test_bonsplit_tab_drag_overlay_gate.py b/tests/test_bonsplit_tab_drag_overlay_gate.py new file mode 100644 index 00000000..2acd087d --- /dev/null +++ b/tests/test_bonsplit_tab_drag_overlay_gate.py @@ -0,0 +1,116 @@ +#!/usr/bin/env python3 +""" +Regression test: file-drop overlay must not intercept bonsplit tab-transfer drags. + +This test is socket-only (no System Events / Accessibility permissions required). +It validates the FileDropOverlayView hit-test gate logic: + +1) tabtransfer pasteboard type never captures hit-testing +2) sidebar reorder pasteboard type never captures hit-testing +3) fileURL pasteboard captures only drag-motion mouse events +4) stale/no-event contexts do not capture hit-testing +""" + +import os +import sys +import time +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) + +from cmux import cmux, cmuxError + + +DRAG_EVENTS = [ + "leftMouseDragged", + "rightMouseDragged", + "otherMouseDragged", +] + +NON_DRAG_EVENTS = [ + "leftMouseDown", + "leftMouseUp", + "rightMouseDown", + "rightMouseUp", + "otherMouseDown", + "otherMouseUp", + "scrollWheel", +] + + +def wait_for_overlay_probe_ready(client: cmux, timeout_s: float = 8.0) -> None: + start = time.time() + last_error = None + while time.time() - start < timeout_s: + try: + _ = client.overlay_hit_gate("none") + return + except Exception as e: + last_error = e + time.sleep(0.1) + raise cmuxError(f"overlay_hit_gate probe unavailable: {last_error}") + + +def assert_gate(client: cmux, event_type: str, expected: bool, reason: str) -> None: + got = client.overlay_hit_gate(event_type) + if got != expected: + raise cmuxError( + f"overlay_hit_gate({event_type}) expected {expected} got {got} ({reason})" + ) + + +def main() -> int: + socket_path = cmux.default_socket_path() + if not os.path.exists(socket_path): + print(f"SKIP: Socket not found at {socket_path}") + print("Tip: start cmux first (or set CMUX_TAG / CMUX_SOCKET_PATH).") + return 0 + + with cmux(socket_path) as client: + ws_id = None + try: + client.activate_app() + time.sleep(0.2) + + ws_id = client.new_workspace() + client.select_workspace(ws_id) + time.sleep(0.4) + + wait_for_overlay_probe_ready(client) + + client.clear_drag_pasteboard() + for event in DRAG_EVENTS + NON_DRAG_EVENTS + ["none"]: + assert_gate(client, event, expected=False, reason="empty drag pasteboard") + + client.seed_drag_pasteboard_tabtransfer() + for event in DRAG_EVENTS + NON_DRAG_EVENTS + ["none"]: + assert_gate(client, event, expected=False, reason="tabtransfer drag must pass through") + + client.seed_drag_pasteboard_sidebar_reorder() + for event in DRAG_EVENTS + NON_DRAG_EVENTS + ["none"]: + assert_gate(client, event, expected=False, reason="sidebar reorder drag must pass through") + + client.seed_drag_pasteboard_fileurl() + for event in DRAG_EVENTS: + assert_gate(client, event, expected=True, reason="file URL drag should be captured") + for event in NON_DRAG_EVENTS + ["none"]: + assert_gate(client, event, expected=False, reason="non-drag events should pass through") + + print("PASS: overlay hit-test gate preserves bonsplit tab drags and file-drop behavior") + return 0 + finally: + try: + client.clear_drag_pasteboard() + except Exception: + pass + if ws_id: + try: + client.close_workspace(ws_id) + except Exception: + pass + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except cmuxError as e: + print(f"FAIL: {e}") + raise SystemExit(1)