Stop disabled drop targets from claiming file drags
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) <noreply@anthropic.com>
This commit is contained in:
1 parent
1c429a301b
commit
68dee1137d
2 files changed
+90
-2
No files matched your search
@@ -74,7 +74,34 @@ nonisolated struct NativeDropPayload: Sendable {
|
||||
|
||||
final class FileURLDropNSView: NSView {
|
||||
var acceptedKinds: Set<NativeDropKind>
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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))
|
||||
|
||||
Reference in new issue
Block a user