Bug 188903

Summary: [Cocoa] Exception (fileType 'dyn.agq8u' is not a valid UTI) raised when dragging an attachment whose file wrapper is a directory
Product: WebKit Reporter: mitz
Component: HTML EditingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, bdakin, commit-queue, mitz, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 188933    
Bug Blocks:    
Attachments:
Description Flags
Depends on webkit.org/b/188933
none
Rebase on trunk
none
Rebase on trunk
thorton: review+
Patch for landing none

Description mitz 2018-08-23 15:30:41 PDT
After using -[WebView _insertAttachmentWithFileWrapper:contentType:options:completion:], passing a NSFileWrapper made from a directory (and nil for the rest of the arguments), dragging the attachment out of the web view causes an exception to be raised:

Exception Name: NSInvalidArgumentException
Description: fileType 'dyn.agq8u' is not a valid UTI
User Info: (null)

0   CoreFoundation                      0x00007fff3a1f9e4c __exceptionPreprocess + 291
1   libobjc.A.dylib                     0x00007fff67ed9efb objc_exception_throw + 48
2   AppKit                              0x00007fff381a8759 -[NSFilePromiseProvider init] + 0
3   WebKit                              0x000000010139555d WebKit::WebViewImpl::startDrag(WebCore::DragItem const&, WebKit::ShareableBitmap::Handle const&)
 + 667
4   WebKit                              0x00000001012fa07a void IPC::handleMessage<Messages::WebPageProxy::StartDrag, WebKit::WebPageProxy, void (WebKit::WebPageProxy::*)(WebCore::DragItem const&, WebKit::ShareableBitmap::Handle const&)>(IPC::Decoder&, WebKit::WebPageProxy*, void (WebKit::WebPageProxy::*)(WebCore::DragItem const&, WebKit::ShareableBitmap::Handle const&))
 + 264
Comment 1 mitz 2018-08-23 15:31:09 PDT
s/WebView/WKWebView/ in the description, of course.
Comment 2 Wenson Hsieh 2018-08-23 15:41:21 PDT
This is probably a result of failing to map kUTTypeFolder to a MIME type :(
Comment 3 Wenson Hsieh 2018-08-23 16:38:09 PDT
(I learned from thorton that RenderThemeMac.mm understands MIME types "multipart/x-folder" and "application/vnd.apple.folder")
Comment 4 Radar WebKit Bug Importer 2018-08-24 17:12:44 PDT
<rdar://problem/43702993>
Comment 5 Wenson Hsieh 2018-08-24 18:55:07 PDT
Note: this is also reproducible when dropping a folder, or any file with an extension that WebKit doesn't recognize (e.g. *.crash).
Comment 6 Wenson Hsieh 2018-08-26 00:04:18 PDT
Created attachment 348089 [details]
Depends on webkit.org/b/188933
Comment 7 Wenson Hsieh 2018-08-26 19:55:36 PDT
Created attachment 348112 [details]
Rebase on trunk
Comment 8 Wenson Hsieh 2018-08-27 09:56:27 PDT
Created attachment 348155 [details]
Rebase on trunk
Comment 9 Tim Horton 2018-08-27 12:16:06 PDT
Comment on attachment 348155 [details]
Rebase on trunk

View in context: https://bugs.webkit.org/attachment.cgi?id=348155&action=review

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:385
> -    [self setFileWrapper:fileWrapper.get() contentType:newContentType completion:^(NSError *error) {
> +    [self setFileWrapper:(__bridge NSFileWrapper *)fileWrapper.get() contentType:newContentType completion:^(NSError *error) {

What is going on with all the 🌉? Other than that, r+, but please get rid of bridges for casts not between CF and NS objects.
Comment 10 Wenson Hsieh 2018-08-27 12:24:14 PDT
(In reply to Tim Horton from comment #9)
> Comment on attachment 348155 [details]
> Rebase on trunk
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=348155&action=review
> 
> > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:385
> > -    [self setFileWrapper:fileWrapper.get() contentType:newContentType completion:^(NSError *error) {
> > +    [self setFileWrapper:(__bridge NSFileWrapper *)fileWrapper.get() contentType:newContentType completion:^(NSError *error) {
> 
> What is going on with all the 🌉? Other than that, r+, but please get rid of
> bridges for casts not between CF and NS objects.

Oh...yeah, most of this bridging (with the exception of CFStringRef <=> NSString) is a mistake. I'll fix this before landing!
Comment 11 Wenson Hsieh 2018-08-27 12:36:13 PDT
Created attachment 348187 [details]
Patch for landing
Comment 12 WebKit Commit Bot 2018-08-27 13:14:29 PDT
Comment on attachment 348187 [details]
Patch for landing

Clearing flags on attachment: 348187

Committed r235392: <https://trac.webkit.org/changeset/235392>