Bug 188490

Summary: [macOS 10.15] Image dragged from Safari does not appear in Notes
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: HTML EditingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, bdakin, commit-queue, ews-watchlist, megan_gardner, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-watchlist: commit-queue-
Drop the image into the area below
none
Archive of layout-test-results from ews205 for win-future
none
Patch none

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 EWS Watchlist 2018-08-16 15:36:50 PDT Comment hidden (obsolete)
Comment 5 EWS Watchlist 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.