Bug 190530

Summary: [Cocoa] Attachment dropped from one web view to another is missing its file wrapper
Product: WebKit Reporter: mitz
Component: HTML EditingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: bdakin, commit-queue, mitz, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Try to fix macOS builds
none
Try to fix the Windows build
none
Try to fix the Windows build (2)
none
Try to fix the Windows build (3)
none
Try to fix the Windows build (4)
none
Try to fix the Windows build (4.1)
none
Try to fix the Windows build (5) none

mitz
Reported 2018-10-12 11:32:06 PDT
To reproduce this bug: 1. Create two editable, attachment-enabled web views 2. Drop a file into the first web view, thus creating an attachment 3. Drag the attachment from the first web view to the other one Notice that the _WKAttachment instance passed to -_webView:didInsertAttachment:withSource: has a nil info.fileWrapper.
Attachments
Patch (44.62 KB, patch)
2018-10-30 14:45 PDT, Wenson Hsieh
no flags
Try to fix macOS builds (44.62 KB, patch)
2018-10-30 15:28 PDT, Wenson Hsieh
no flags
Try to fix the Windows build (44.89 KB, patch)
2018-10-30 16:45 PDT, Wenson Hsieh
no flags
Try to fix the Windows build (2) (44.91 KB, patch)
2018-10-30 19:10 PDT, Wenson Hsieh
no flags
Try to fix the Windows build (3) (45.06 KB, patch)
2018-10-30 19:52 PDT, Wenson Hsieh
no flags
Try to fix the Windows build (4) (45.07 KB, patch)
2018-10-30 20:11 PDT, Wenson Hsieh
no flags
Try to fix the Windows build (4.1) (45.12 KB, patch)
2018-10-30 20:23 PDT, Wenson Hsieh
no flags
Try to fix the Windows build (5) (45.09 KB, patch)
2018-10-30 20:34 PDT, Wenson Hsieh
no flags
mitz
Comment 1 2018-10-12 11:38:26 PDT
mitz
Comment 2 2018-10-12 12:41:32 PDT
Seems to also happen with copy and paste!
Wenson Hsieh
Comment 3 2018-10-30 14:45:53 PDT
Wenson Hsieh
Comment 4 2018-10-30 15:28:30 PDT
Created attachment 353417 [details] Try to fix macOS builds
Wenson Hsieh
Comment 5 2018-10-30 16:45:57 PDT
Created attachment 353435 [details] Try to fix the Windows build
Tim Horton
Comment 6 2018-10-30 16:56:09 PDT
Comment on attachment 353435 [details] Try to fix the Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=353435&action=review > Source/WebCore/html/HTMLAttachmentElement.cpp:91 > + return URLParser(makeString("applewebdata://attachment/"_s, identifier)).result(); It feels weird/scary to build a URL by concatenating strings. But it seems like we don't have a mutable URL? What happens if identifier (which comes from where?) has non-path-safe characters?
Wenson Hsieh
Comment 7 2018-10-30 17:42:51 PDT
Comment on attachment 353435 [details] Try to fix the Windows build View in context: https://bugs.webkit.org/attachment.cgi?id=353435&action=review >> Source/WebCore/html/HTMLAttachmentElement.cpp:91 >> + return URLParser(makeString("applewebdata://attachment/"_s, identifier)).result(); > > It feels weird/scary to build a URL by concatenating strings. But it seems like we don't have a mutable URL? What happens if identifier (which comes from where?) has non-path-safe characters? Good point — the identifier _normally_ comes from within WebKit (createCanonicalUUIDString()) and it's only exposed as a readonly attribute to internal clients. That being said, we do have a mechanism for temporarily saving the identifier as an attribute on the attachment element (see "webkitattachmentid"). Using this, it should be possible to influence the identifier of a pasted/dropped attachment element by writing a webarchive to the pasteboard with an attachment element that contains this attribute... I've changed this to use URL::setPath() to set the path of the URL, and verified that non-path-safe characters (e.g. space) are URL-encoded.
Wenson Hsieh
Comment 8 2018-10-30 19:10:00 PDT
Created attachment 353456 [details] Try to fix the Windows build (2)
Wenson Hsieh
Comment 9 2018-10-30 19:52:26 PDT
Created attachment 353458 [details] Try to fix the Windows build (3)
Wenson Hsieh
Comment 10 2018-10-30 20:11:31 PDT
Created attachment 353459 [details] Try to fix the Windows build (4)
Wenson Hsieh
Comment 11 2018-10-30 20:23:42 PDT
Created attachment 353460 [details] Try to fix the Windows build (4.1)
Wenson Hsieh
Comment 12 2018-10-30 20:34:16 PDT
Created attachment 353461 [details] Try to fix the Windows build (5)
WebKit Commit Bot
Comment 13 2018-10-30 22:29:26 PDT
Comment on attachment 353461 [details] Try to fix the Windows build (5) Clearing flags on attachment: 353461 Committed r237624: <https://trac.webkit.org/changeset/237624>
WebKit Commit Bot
Comment 14 2018-10-30 22:29:28 PDT
All reviewed patches have been landed. Closing bug.
mitz
Comment 15 2019-04-06 19:34:18 PDT
*** Bug 188904 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.