Skip to content

fix(desktop_drop): accept web drops whose items have no FileSystemEntry - #503

Open
jamesaorson wants to merge 1 commit into
MixinNetwork:mainfrom
autobutler-org:fix/web-drop-non-file-items
Open

fix(desktop_drop): accept web drops whose items have no FileSystemEntry#503
jamesaorson wants to merge 1 commit into
MixinNetwork:mainfrom
autobutler-org:fix/web-drop-non-file-items

Conversation

@jamesaorson

@jamesaorson jamesaorson commented Sep 10, 2026

Copy link
Copy Markdown

Fixes the web ondrop handler crashing on any drag whose items are not all backed by a FileSystemEntry.

The bug

desktop_drop_web.dart calls webkitGetAsEntry()! on every item in the drag:

Future.wait(List.generate(items.length, (index) {
  final item = items[index];
  final entry = item.webkitGetAsEntry()!;   // throws
  return _entryToWebDropItem(entry);
}))

webkitGetAsEntry() returns null for kind == 'string' items, and for file items with no filesystem entry behind them. The null assert throws synchronously inside List.generate, before the .catchError further down the chain is attached — so the error is not even caught by the handler that looks like it should catch it. performOperation_web is never invoked, no DropDoneEvent reaches the app, and the drop target is left in a non-idle state: nothing is dropped, and the drag highlight stays stuck until the widget is rebuilt.

How it is reached

This is #456 (dragging a file out of a zip viewer). We hit the same crash from a different direction: dragging from the browser's own downloads menu, which is a very ordinary thing for a user to do into an upload target.

Both Chrome and Firefox put a real, disk-backed File on the dataTransfer, plus string items next to it. Firefox is explicit about it in downloads.js:

dataTransfer.mozSetDataAt("application/x-moz-file", file, 0);  // the real file
let spec = NetUtil.newURI(file).spec;
dataTransfer.setData("text/uri-list", spec);                   // string item
dataTransfer.setData("text/plain", spec);                      // string item

One file item, two string items. The string items hit the null assert and take the whole drop down with them — including the perfectly good file sitting in the same drag. Ordinary HTML5 drop handlers accept these drags today, which is why the same drag works on other web upload targets but not on a desktop_drop one.

The fix

Null-guard the entry and fall back to getAsFile(). The two APIs are not equivalent: getAsFile() returns the file for exactly the items the entry API cannot describe. Items that yield neither are skipped.

webkitGetAsEntry() is still preferred when present, since it is what makes directory recursion possible.

One deliberate difference from #459

#459 (draft, by @copilot-swe-agent) fixes the null assert the same way, and this PR keeps that approach. It also returns early when no usable items come out:

if (items.isEmpty) { return; }

This PR instead invokes performOperation_web with the empty list. Skipping the call means a drag carrying no files at all (a dragged link, selected text) never produces a DropDoneEvent — and since that event is what resets DropTarget's status, the drag highlight stays stuck exactly as it does in the crashing case. channel.dart already handles an empty list correctly, emitting DropDoneEvent(files: []), so notifying unconditionally clears the drop target and lets the app decide there was nothing to take.

Happy to close this in favor of #459 with the empty-list change folded in, if the maintainers would rather take that branch — the important half is the null-guard either way.

Testing

dart analyze clean.

Verified at runtime on macOS. A drag from the browser's downloads menu into an upload target built against this branch now uploads the file. The same drag against a stock 0.8.4 build reproduces the original failure — nothing uploads and the drag highlight stays stuck until the widget rebuilds. The two builds were run side by side on different ports, and the served bundle was checked to confirm the patched handler was the one actually loaded (getAsFile appears in the compiled output of this branch and not in stock 0.8.4).

The report that prompted this was an ordinary user hitting it in normal use, not a synthetic case.

The package's only existing test is channel_linux_test.dart; there is no web test harness here, so this change has no automated coverage either. Glad to add one if you would like to point me at a preferred shape.

I did not touch CHANGELOG.md or the version, on the assumption releases are maintainer-driven — say the word and I will add an entry.

The web ondrop handler called webkitGetAsEntry()! on every item in the
drag. That API returns null for string items, and a drag routinely
carries them alongside its files -- a drag from the browser's downloads
menu sends text/uri-list and text/plain next to the file. The null
assert threw synchronously inside List.generate, before the catchError
was attached, so performOperation_web was never invoked: no DropDoneEvent
reached the app, the file never uploaded, and the drop target's highlight
stayed stuck until a rebuild.

Null-guard the entry and fall back to getAsFile(), which returns the file
for exactly the items the entry API cannot describe. Skip items that
yield neither.

Also notify with the resulting list even when it is empty. The event is
what resets DropTarget's status, so swallowing it on a file-less drag
(a dragged link, selected text) leaves the highlight stuck -- the same
symptom, one step further along.

Refs MixinNetwork#456, MixinNetwork#459

Signed-off-by: James Orson <jamesaorson@gmail.com>
@jamesaorson

Copy link
Copy Markdown
Author

Runtime verification is in — the PR body has been updated accordingly (it previously said this was unverified).

A drag from the browser's downloads menu into an upload target now works on a build against this branch. Side by side, same drag, same machine (macOS):

  • stock 0.8.4 — nothing uploads, drag highlight stays stuck until the widget rebuilds
  • this branch — file uploads normally

I checked the served bundle in each case to be sure the difference was really this patch: getAsFile appears in the compiled output of this branch and is absent from stock 0.8.4.

Worth adding for @jtmcdole and #456: the trigger here was not a zip viewer but the browser's own downloads menu, which is a very ordinary thing for a user to drag from into an upload area. Both browsers put a real File on the dataTransfer with string items (text/uri-list, text/plain) beside it, and those string items are what take the whole drop down. So the blast radius of this bug is wider than the zip case suggests — any drag carrying a string item alongside its files hits it, and the file is right there in the drag the whole time.

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.

1 participant