Bug 181294

Summary: [Attachment Support] Support dragging attachment elements out as files on macOS
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, mitz, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 181201, 181236, 181497, 181574, 181898, 188715    
Bug Blocks: 188823    
Attachments:
Description Flags
Patch
none
Blocked on webkit.org/b/181574
none
Depends on 188496
none
Rebase on trunk
none
Fix 32-bit macOS build
none
Archive of layout-test-results from ews206 for win-future
none
Fix 32-bit macOS build (2)
none
Fix OpenSource macOS builds
thorton: review+
Patch for EWS
none
Patch for EWS none

Description Wenson Hsieh 2018-01-04 09:25:52 PST
The macOS counterpart to https://bugs.webkit.org/show_bug.cgi?id=181199.
Comment 1 Radar WebKit Bug Importer 2018-01-04 09:27:21 PST
<rdar://problem/36298801>
Comment 2 Wenson Hsieh 2018-01-11 22:54:32 PST
Created attachment 331172 [details]
Patch
Comment 3 Wenson Hsieh 2018-01-11 23:06:09 PST
Created attachment 331175 [details]
Blocked on webkit.org/b/181574
Comment 4 Wenson Hsieh 2018-08-19 22:05:29 PDT
Created attachment 347481 [details]
Depends on 188496
Comment 5 Wenson Hsieh 2018-08-21 20:36:33 PDT
Created attachment 347760 [details]
Rebase on trunk
Comment 6 Wenson Hsieh 2018-08-21 21:37:18 PDT
Created attachment 347764 [details]
Fix 32-bit macOS build
Comment 7 EWS Watchlist 2018-08-22 09:04:09 PDT Comment hidden (obsolete)
Comment 8 EWS Watchlist 2018-08-22 09:04:20 PDT Comment hidden (obsolete)
Comment 9 Wenson Hsieh 2018-08-22 09:09:14 PDT
Created attachment 347802 [details]
Fix 32-bit macOS build (2)
Comment 10 Wenson Hsieh 2018-08-22 09:45:27 PDT
Created attachment 347811 [details]
Fix OpenSource macOS builds
Comment 11 Tim Horton 2018-08-22 12:47:05 PDT
Comment on attachment 347811 [details]
Fix OpenSource macOS builds

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

> Source/WebCore/editing/cocoa/EditorCocoa.mm:173
> +    // On macOS, we currently write the attachment as a web archive; we can't do the same for iOS and remove the platform guard below

s/below/above/?

> Source/WebKit/UIProcess/API/mac/WKView.mm:1096
> +- (NSString *)filePromiseProvider:(NSFilePromiseProvider*)filePromiseProvider fileNameForType:(NSString *)fileType

Stars on the wrong side

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:3925
> +    return [NSError errorWithDomain:WKErrorDomain code:WKErrorUnknown userInfo:nil];
> +#else
> +    return [NSError errorWithDomain:@"WebKitErrorDomain" code:1 userInfo:nil];

This is weird. Have we ever done this before?

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:4003
> +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200

Do we still build back to 10.11? What's up with all of this. Should/can it be a specific HAVE() or USE() instead?
Comment 12 Wenson Hsieh 2018-08-22 13:14:11 PDT
Comment on attachment 347811 [details]
Fix OpenSource macOS builds

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

>> Source/WebCore/editing/cocoa/EditorCocoa.mm:173
>> +    // On macOS, we currently write the attachment as a web archive; we can't do the same for iOS and remove the platform guard below
> 
> s/below/above/?

Fixed!

>> Source/WebKit/UIProcess/API/mac/WKView.mm:1096
>> +- (NSString *)filePromiseProvider:(NSFilePromiseProvider*)filePromiseProvider fileNameForType:(NSString *)fileType
> 
> Stars on the wrong side

Fixed!

>> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:3925
>> +    return [NSError errorWithDomain:@"WebKitErrorDomain" code:1 userInfo:nil];
> 
> This is weird. Have we ever done this before?

Good point. The ways in which we handle errors is pretty inconsistent already (some places in WKWebView use [[NSError alloc] init], and other places in attachments code use WKErrorDomain, but with error codes that are not exposed anywhere). I filed <https://bugs.webkit.org/show_bug.cgi?id=188860> to clean this up.

>> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:4003
>> +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101200
> 
> Do we still build back to 10.11? What's up with all of this. Should/can it be a specific HAVE() or USE() instead?

You're right! It looks like there are no more El Capitan bots on build.webkit.org, so I should be able to just remove all of these version checks.
Comment 13 Wenson Hsieh 2018-08-22 13:28:01 PDT Comment hidden (obsolete)
Comment 14 Wenson Hsieh 2018-08-22 13:29:59 PDT
Created attachment 347836 [details]
Patch for EWS
Comment 15 WebKit Commit Bot 2018-08-22 14:35:10 PDT
Comment on attachment 347836 [details]
Patch for EWS

Clearing flags on attachment: 347836

Committed r235202: <https://trac.webkit.org/changeset/235202>