From 68dee1137d094db93245197af5cb85dc27092bb3 Mon Sep 17 00:00:00 2001 From: Kaede Akino Date: Wed, 2 Sep 2026 15:41:35 +0800 Subject: [PATCH] Stop disabled drop targets from claiming file drags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On the Assistant page, dragging a file onto the sidebar attached it to `window.sidebar.assistantDraft` — the Workspace sidebar's new-chat draft, which is not on screen there, so the file appeared to vanish. `AssistantPanelView` already declared the intent with `isEnabled: isFocusedOnWorkspace`, but the flag never reached AppKit. `FileURLDropNSView` registered its dragged types unconditionally in `init` and never unregistered, and only `hitTest` consulted `isEnabled` — the three dragging callbacks did not. A view that refuses the hit test but stays in the window's registered destination list still wins the drop. This is the asymmetry `AssistantConversationListContainer.isInteractive` records for conversation drags; the shared file receiver had only the hit-test half. Give it both: follow `isEnabled` when registering and unregistering, and gate `supportedPayload(from:)`, the one chokepoint `hitTest` and all three callbacks pass through. The callbacks already treat a nil payload as "not mine", so their bodies are unchanged. The second half is what holds when `isEnabled` flips during a drag already in flight, after AppKit has resolved its destinations. Fixing the shared view also covers two sites with the same flaw: hidden Workspaces, which all stay mounted at `opacity(0)` over the same frame carrying a pane drop target, and Keychain accepting drops mid-load. `FileURLDropTargetTests` becomes `.serialized` — every test there stages its case on the one drag pasteboard the system shares, and the suite was already racing with itself before these two tests made it fail. Verification: macOS build, full test suite (remaining failures match the clean-tree baseline and are unrelated), swift-format lint adds nothing new, and Scripts/check-zh-localization.sh passes. The end-to-end Finder drag is still to be confirmed by hand. Co-Authored-By: Claude Opus 5 (1M context) --- Termind/UI/Components/FileURLDropTarget.swift | 39 +++++++++++++- TermindTests/FileURLDropTargetTests.swift | 53 +++++++++++++++++++ 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/Termind/UI/Components/FileURLDropTarget.swift b/Termind/UI/Components/FileURLDropTarget.swift index f886c7b9..d1a5724e 100644 --- a/Termind/UI/Components/FileURLDropTarget.swift +++ b/Termind/UI/Components/FileURLDropTarget.swift @@ -74,7 +74,34 @@ nonisolated struct NativeDropPayload: Sendable { final class FileURLDropNSView: NSView { var acceptedKinds: Set - var isEnabled: Bool + + /// Whether this receiver is the one on screen. + /// + /// Layers that are mounted but invisible keep their drop targets alive — + /// the Assistant panel holds the Workspace layer behind the sidebar chat + /// list, and every Workspace stays in the deck at `opacity(0)`. AppKit + /// honours neither `allowsHitTesting(false)` nor `opacity(0)` when it + /// looks for a drag destination, so a hidden copy used to win the drop + /// and file it somewhere nobody could see. + /// + /// Both halves of the retreat are needed, for the same reasons spelled + /// out on `AssistantConversationListContainer.isInteractive`: leaving + /// this view in the window's registered destination list hands it drags + /// it refused by hit test, and unregistering cannot reach a drag already + /// in flight, because AppKit resolved its destinations when the drag + /// began. `supportedPayload(from:)` is the second half — every entry + /// point passes through it. + var isEnabled: Bool { + didSet { + guard isEnabled != oldValue else { return } + if isEnabled { + registerForDraggedTypes(Self.registeredTypes) + } else { + unregisterDraggedTypes() + } + } + } + var targetedKind: (NativeDropKind?) -> Void var receivePayload: (NativeDropPayload) -> Bool @@ -89,7 +116,9 @@ nonisolated struct NativeDropPayload: Sendable { self.targetedKind = targetedKind self.receivePayload = receivePayload super.init(frame: .zero) - registerForDraggedTypes(Self.registeredTypes) + // Registered here rather than left to `isEnabled`: a property + // assigned in `init` does not run its own `didSet`. + if isEnabled { registerForDraggedTypes(Self.registeredTypes) } } convenience init( @@ -173,7 +202,13 @@ nonisolated struct NativeDropPayload: Sendable { super.concludeDragOperation(sender) } + /// The one gate `hitTest` and all three dragging callbacks pass through. + /// A disabled receiver answers nil to each, which already means "not + /// mine" everywhere: the two `dragging*` methods clear the highlight and + /// return no operation, and `performDragOperation` declines. See + /// `isEnabled` for why refusing the hit test is not enough on its own. func supportedPayload(from pasteboard: NSPasteboard) -> NativeDropPayload? { + guard isEnabled else { return nil } let payload = Self.payload(from: pasteboard).restricted(to: acceptedKinds) return payload.preferredKind == nil ? nil : payload } diff --git a/TermindTests/FileURLDropTargetTests.swift b/TermindTests/FileURLDropTargetTests.swift index 52aaf73e..ba6a0e42 100644 --- a/TermindTests/FileURLDropTargetTests.swift +++ b/TermindTests/FileURLDropTargetTests.swift @@ -4,6 +4,10 @@ import UniformTypeIdentifiers @testable import Termind + /// Serialized because every test here stages its case on the one drag + /// pasteboard the whole system shares. Run concurrently, they clear each + /// other's fixture mid-assertion. + @Suite(.serialized) @MainActor struct FileURLDropTargetTests { @Test @@ -82,6 +86,55 @@ #expect(payload.preferredKind == .files) } + /// Hidden layers keep their receivers mounted — the Assistant panel + /// holds the Workspace layer behind the sidebar chat list, and every + /// Workspace stays in the deck at `opacity(0)`. AppKit consults neither + /// when it picks a drag destination, so a disabled copy used to accept + /// the drop and attach the file to a conversation nobody could see. + @Test + func disabledTargetLeavesTheDragToWhateverIsOnScreen() throws { + let pasteboard = NSPasteboard(name: .drag) + pasteboard.clearContents() + defer { pasteboard.clearContents() } + + let file = FileManager.default.temporaryDirectory + .appendingPathComponent("termind-disabled-drop-target-test.txt") + try Data("probe".utf8).write(to: file) + defer { try? FileManager.default.removeItem(at: file) } + #expect(pasteboard.writeObjects([file as NSURL])) + + let target = FileURLDropNSView(isTargeted: { _ in }, receive: { _ in true }) + target.frame = CGRect(x: 0, y: 0, width: 200, height: 100) + let point = CGPoint(x: 50, y: 50) + + #expect(target.registeredDraggedTypes.contains(.fileURL)) + #expect(target.hitTest(point) === target) + #expect(target.supportedPayload(from: pasteboard) != nil) + + target.isEnabled = false + + #expect(target.registeredDraggedTypes.isEmpty) + #expect(target.hitTest(point) == nil) + #expect(target.supportedPayload(from: pasteboard) == nil) + + target.isEnabled = true + + #expect(target.registeredDraggedTypes.contains(.fileURL)) + #expect(target.hitTest(point) === target) + #expect(target.supportedPayload(from: pasteboard) != nil) + } + + /// The half no `didSet` can cover: a property assigned in `init` never + /// runs its own observer. + @Test + func targetBornDisabledNeverRegistersAsADestination() { + let target = FileURLDropNSView( + isEnabled: false, isTargeted: { _ in }, receive: { _ in true } + ) + + #expect(target.registeredDraggedTypes.isEmpty) + } + @Test func staleFileURLPasteboardCannotCaptureNormalComposerInteraction() { #expect(FileURLDropNSView.isOrdinaryPointerEvent(.leftMouseDown))