Bug 181294 – [Attachment Support] Support dragging attachment elements out as files on macOS

• Component: HTML Editing
• Status: RESOLVED FIXED

[Wenson Hsieh]

The macOS counterpart to https://bugs.webkit.org/show_bug.cgi?id=181199.

[Radar WebKit Bug Importer]

<rdar://problem/36298801>

[Wenson Hsieh]

Created attachment 331172 [details]
Patch

[Wenson Hsieh]

Created attachment 331175 [details]
Blocked on webkit.org/b/181574

[Wenson Hsieh]

Created attachment 347481 [details]
Depends on 188496

[Wenson Hsieh]

Created attachment 347760 [details]
Rebase on trunk

[Wenson Hsieh]

Created attachment 347764 [details]
Fix 32-bit macOS build

[EWS Watchlist]

Comment on attachment 347764 [details]
Fix 32-bit macOS build

Attachment 347764 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8944461

New failing tests:
http/tests/security/contentSecurityPolicy/userAgentShadowDOM/allow-video.html

[EWS Watchlist]

Created attachment 347800 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Wenson Hsieh]

Created attachment 347802 [details]
Fix 32-bit macOS build (2)

[Wenson Hsieh]

Created attachment 347811 [details]
Fix OpenSource macOS builds

[Tim Horton]

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?

[Wenson Hsieh]

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.

[Wenson Hsieh]

Created attachment 347835 [details]
Patch for EWS

[Wenson Hsieh]

Created attachment 347836 [details]
Patch for EWS

[WebKit Commit Bot]

Comment on attachment 347836 [details]
Patch for EWS

Clearing flags on attachment: 347836

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