fix(desktop_drop): accept web drops whose items have no FileSystemEntry - #503
fix(desktop_drop): accept web drops whose items have no FileSystemEntry#503jamesaorson wants to merge 1 commit into
Conversation
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>
|
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):
I checked the served bundle in each case to be sure the difference was really this patch: 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 |
Fixes the web
ondrophandler crashing on any drag whose items are not all backed by aFileSystemEntry.The bug
desktop_drop_web.dartcallswebkitGetAsEntry()!on every item in the drag:webkitGetAsEntry()returns null forkind == 'string'items, and for file items with no filesystem entry behind them. The null assert throws synchronously insideList.generate, before the.catchErrorfurther down the chain is attached — so the error is not even caught by the handler that looks like it should catch it.performOperation_webis never invoked, noDropDoneEventreaches 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
Fileon the dataTransfer, plus string items next to it. Firefox is explicit about it indownloads.js: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_dropone.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:
This PR instead invokes
performOperation_webwith the empty list. Skipping the call means a drag carrying no files at all (a dragged link, selected text) never produces aDropDoneEvent— and since that event is what resetsDropTarget's status, the drag highlight stays stuck exactly as it does in the crashing case.channel.dartalready handles an empty list correctly, emittingDropDoneEvent(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 analyzeclean.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 (
getAsFileappears 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.mdor the version, on the assumption releases are maintainer-driven — say the word and I will add an entry.