Skip to content

Conversation

@omar-hanafy
Copy link
Contributor

Summary

This PR upgrades the macOS implementation to handle real-world drag sources reliably and predictably:

solves #433

  • Prefer public.file-url and NSFilenamesPboardType (legacy filename arrays) when present.
  • Fall back to NSFilePromiseReceiver for sources like VS Code/Electron that deliver file promises.
  • Correctly detect directories and surface them as DropItemDirectory.
  • Add a first-class fromPromise flag on DropItem so apps can tailor UX when the original path is unavailable.
  • Create security-scoped bookmarks only for paths outside the app container/tmp.
  • Use a per-drop unique destination for promised files to avoid name collisions.
  • Make promise reception thread-safe (guard shared state with a lock).
  • Guard Dart start/stopAccessingSecurityScopedResource calls when the bookmark is empty.

Version bump: 0.7.0. Minimum macOS set to 10.13 (SwiftPM/Podspec).


Motivation & context

Different apps advertise drag data differently:

  • Finder/JetBrains usually supply file URLs (true original paths).
  • VS Code/Electron often supply file promises (no original path; the receiver chooses a destination and the source writes bytes there).

Previously, multi-file drags could be incomplete, directory drops weren’t differentiated, and promise handling could yield race conditions and unnecessary bookmark attempts. This PR addresses those issues and codifies behavior so plugin consumers can build robust UX across sources.


What’s changed (highlights)

  • macOS (Swift)

    • Register all relevant pasteboard types (file URLs, filenames array, file promises).
    • Prefer URLs/legacy arrays; only fallback to promises.
    • Add per-drop Drops/<timestamp> destination; safer timestamp format.
    • Thread-safe aggregation of results (lock around seen/items).
    • Generate security-scoped bookmarks only for paths outside container/tmp; NSNull otherwise.
    • Include isDirectory and fromPromise in channel payloads.
  • Dart

    • DropItem now has fromPromise and documented extraAppleBookmark.
    • Map isDirectoryDropItemDirectory.
    • Safeguard start/stopAccessingSecurityScopedResource against empty bookmarks (no-ops).
    • Minor docs in code.
  • Meta

    • pubspec.yaml: version → 0.7.0.
    • Package.swift/Podspec: macOS min → 10.13.
    • CHANGELOG.md updated.

API changes for consumers

  • New: DropItem.fromPromise (bool).

    • true means the item came via a file promise and was written into the plugin’s Drops/<timestamp> folder. The original source path is not available by design.
  • Existing: DropItem.extraAppleBookmark (Uint8List?) is often null/empty for promise files and non-empty for file URLs outside the app container.

  • Behavior: DesktopDrop.start/stopAccessingSecurityScopedResource no-op when bookmark is empty.

Typical usage:

for (final item in details.files) {
  if (item.fromPromise) {
    // Working from a delivered copy under Drops/<timestamp>/...
    // No original path to reveal; read directly.
  } else if ((item.extraAppleBookmark?.isNotEmpty ?? false)) {
    await DesktopDrop.instance.startAccessingSecurityScopedResource(
      bookmark: item.extraAppleBookmark!,
    );
    try {
      // Read the file at item.path
    } finally {
      await DesktopDrop.instance.stopAccessingSecurityScopedResource(
        bookmark: item.extraAppleBookmark!,
      );
    }
  } else {
    // File URL inside container/tmp or non-macOS; just read it.
  }
}

Backward compatibility

  • Additive Dart fields; existing apps continue to work.
  • Promised files now land under Drops/<timestamp> (still under tmp); previous behavior used a shared Drops folder — this reduces collisions but changes the exact path. Consumers reading from the returned path are unaffected.

Known limitations (documented)

  • For file promises, macOS does not expose the original source path. This is expected. The plugin now marks such items with fromPromise = true and provides a stable, readable copy path.

Testing

Manual matrix

  • Finder → single/multi files: all items reported; real paths; bookmarks present.
  • JetBrains/IntelliJ → multi files: all items reported; real paths; bookmarks present.
  • VS Code/Electron → single/multi: files in Drops/<timestamp>/...; fromPromise = true; bookmarks empty; bytes readable.
  • Folder drops (Finder): DropItemDirectory surfaced; isDirectory = true.
  • Sandboxed build: reading promise files within container works without bookmarks; reading external files works with bookmarks.

Edge cases

  • Mix of URLs and promises: URLs win; promises processed only when URLs absent.
  • Duplicate items from mixed pasteboard types: de-duplicated by path (thread-safe).

- Prefer public.file-url / NSFilenamesPboardType; fallback to NSFilePromiseReceiver
- Add fromPromise flag; map directories to DropItemDirectory
- Generate security-scoped bookmarks only for paths outside container/tmp
- Per-drop unique destination for promises
- Guards in Dart for empty bookmarks
- Thread-safe collection of drop results
@boyan01 boyan01 requested a review from Copilot September 12, 2025 02:15
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR significantly enhances the macOS implementation of the desktop_drop plugin to handle diverse drag sources more reliably, adding robust support for file promises, directories, and multi-source scenarios. The changes address incomplete multi-file drags and race conditions while introducing new API features for better UX differentiation.

  • Implements comprehensive drag source handling (file URLs, legacy filename arrays, and file promises)
  • Adds thread-safe promise reception with per-drop unique destinations
  • Introduces fromPromise flag and directory detection capabilities

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pubspec.yaml Version bump to 0.7.0 and flutter_lints upgrade
DesktopDropPlugin.swift Complete rewrite of drag handling logic with multi-source support and thread safety
Package.swift macOS minimum version bump to 10.13
desktop_drop.podspec macOS minimum version bump to 10.13
drop_item.dart Added fromPromise field and comprehensive documentation
channel.dart Enhanced platform communication with directory support and bookmark guards
CHANGELOG.md Documented all new features and changes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

let base = FileManager.default.temporaryDirectory.appendingPathComponent("Drops", isDirectory: true)
let formatter = DateFormatter()
formatter.locale = Locale(identifier: "en_US_POSIX")
formatter.dateFormat = "yyyyMMdd_HHmmss_SSS'Z'"
Copy link

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The date format string 'yyyyMMdd_HHmmss_SSS'Z'' includes a literal 'Z' suffix but doesn't actually format the date in UTC timezone. Either remove the 'Z' or set the formatter's timeZone to UTC to match the format.

Suggested change
formatter.dateFormat = "yyyyMMdd_HHmmss_SSS'Z'"
formatter.dateFormat = "yyyyMMdd_HHmmss_SSS'Z'"
formatter.timeZone = TimeZone(secondsFromGMT: 0)

Copilot uses AI. Check for mistakes.
Comment on lines +49 to +50
this.extraAppleBookmark,
this.fromPromise = false,
Copy link

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DropItem.fromData constructor is missing the extraAppleBookmark parameter in its super() call. This will cause extraAppleBookmark to always be null for items created via fromData, which may not be the intended behavior.

Copilot uses AI. Check for mistakes.
super.name,
super.length,
super.lastModified,
super.path,
Copy link

Copilot AI Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DropItemFile.fromData constructor is missing the extraAppleBookmark parameter. It should pass super.extraAppleBookmark to match the pattern used in other constructors.

Suggested change
super.path,
super.path,
super.extraAppleBookmark,

Copilot uses AI. Check for mistakes.
@boyan01
Copy link
Contributor

boyan01 commented Sep 12, 2025

LGTM, thanks for the contribution 💙

@boyan01 boyan01 merged commit 4181b95 into MixinNetwork:main Sep 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants