Bug 188490 - [macOS 10.15] Image dragged from Safari does not appear in Notes
Summary: [macOS 10.15] Image dragged from Safari does not appear in Notes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-08-12 02:16 PDT by Wenson Hsieh
Modified: 2019-08-02 11:50 PDT (History)
8 users (show)

See Also:


Attachments
Patch (4.51 KB, patch)
2018-08-12 14:47 PDT, Wenson Hsieh
ews: commit-queue-
Details | Formatted Diff | Diff
Drop the image into the area below (37.28 KB, application/zip)
2018-08-13 20:00 PDT, Wenson Hsieh
no flags Details
Archive of layout-test-results from ews205 for win-future (12.79 MB, application/zip)
2018-08-16 15:37 PDT, Build Bot
no flags Details
Patch (7.04 KB, patch)
2019-08-02 09:57 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2018-08-12 02:16:04 PDT Comment hidden (obsolete)
Comment 1 Wenson Hsieh 2018-08-12 14:47:02 PDT
Created attachment 346990 [details]
Patch
Comment 2 Wenson Hsieh 2018-08-13 19:58:57 PDT
I realized on second glance that this patch is not quite right...

Suppose we're dragging a file promise within a `WKWebView` (or dragging from one `WKWebView` to another in the same process), and dropping in a custom file upload area that uses DataTransfer API to read files from the pasteboard.

Upon drop, we'll call out to the web process to perform the drag operation; the web process then makes a request to the UI process to grab the image on behalf of DataTransfer API, which then causes AppKit to request the data by invoking `-[WKWebView pasteboard:provideDataForType:]`.

However, in the meantime, AppKit has already assumed that we've finished from the drag pasteboard after calling `-[WKWebView performDragOperation:]` on the destination-side, and calls `-[WKWebView draggedImage:endedAt:operation:]` on the source. If this patch were to land, we'd end up clearing out our data early, before the destination web view has a chance to ask for it.

This can be observed via a simple test case, to be attached shortly. I don't think there's a simple way to fix this; we'd probably have to rewrite our macOS drop handling logic to become like iOS, where we have a heuristic to front-load any data on the pasteboard that the drop destination might access, and then provide a mechanism to answer any questions the web process might have using this subset of the pasteboard. iOS is only implemented this way because, unlike macOS, the data of a drag session backed by NSItemProviders is accessible *only* within the scope of the call to perform a drop. On the other hand, due to the persistent nature of NSPasteboard on macOS, we have the ability to just lazily fetch data as needed.

I guess a cheap (non-?)solution is to just keep the drag data around on WebViewImpl until the next drag happens...but this is far less than ideal.
Comment 3 Wenson Hsieh 2018-08-13 20:00:24 PDT
Created attachment 347062 [details]
Drop the image into the area below

Test case
Comment 4 Build Bot 2018-08-16 15:36:50 PDT Comment hidden (obsolete)
Comment 5 Build Bot 2018-08-16 15:37:01 PDT Comment hidden (obsolete)
Comment 6 Wenson Hsieh 2019-08-02 09:43:23 PDT
<rdar://problem/39462717>
Comment 7 Wenson Hsieh 2019-08-02 09:57:20 PDT
Created attachment 375417 [details]
Patch
Comment 8 Wenson Hsieh 2019-08-02 11:20:21 PDT
Comment on attachment 375417 [details]
Patch

Thanks for the review!
Comment 9 Wenson Hsieh 2019-08-02 11:21:22 PDT
The api-mac bot encountered these failures:

> Test suite failed
> Timeout
>     TestWebKitAPI.Challenge.BasicProposedCredential
>     TestWebKitAPI.WKWebsiteDataStore.FetchPersistentCredentials
>     TestWebKitAPI.Challenge.SecIdentity

…which don’t seem related to this change.
Comment 10 WebKit Commit Bot 2019-08-02 11:50:04 PDT
Comment on attachment 375417 [details]
Patch

Clearing flags on attachment: 375417

Committed r248166: <https://trac.webkit.org/changeset/248166>
Comment 11 WebKit Commit Bot 2019-08-02 11:50:06 PDT
All reviewed patches have been landed.  Closing bug.