Bug 173511 - [iOS DnD] Support .zip archives for file uploads via drag and drop
Summary: [iOS DnD] Support .zip archives for file uploads via drag and drop
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-06-17 01:03 PDT by Wenson Hsieh
Modified: 2017-06-19 14:12 PDT (History)
7 users (show)

See Also:


Attachments
Patch (18.93 KB, patch)
2017-06-17 01:56 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch (18.93 KB, patch)
2017-06-17 01:59 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Fix Mac build (18.96 KB, patch)
2017-06-17 02:12 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews126 for ios-simulator-wk2 (914.42 KB, application/zip)
2017-06-17 03:38 PDT, Build Bot
no flags Details
Unify DragData::containsFiles on Mac/iOS (19.96 KB, patch)
2017-06-17 17:46 PDT, Wenson Hsieh
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2017-06-17 01:03:37 PDT
<rdar://problem/32224510>
Comment 1 Wenson Hsieh 2017-06-17 01:56:17 PDT
Created attachment 313192 [details]
Patch
Comment 2 Wenson Hsieh 2017-06-17 01:57:13 PDT
Actually, this is <rdar://problem/32521025>
Comment 3 Wenson Hsieh 2017-06-17 01:59:28 PDT
Created attachment 313193 [details]
Patch
Comment 4 Wenson Hsieh 2017-06-17 02:12:30 PDT
Created attachment 313194 [details]
Fix Mac build
Comment 5 Build Bot 2017-06-17 03:38:45 PDT
Comment on attachment 313194 [details]
Fix Mac build

Attachment 313194 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/3947006

New failing tests:
webrtc/video-replace-muted-track.html
Comment 6 Build Bot 2017-06-17 03:38:47 PDT
Created attachment 313196 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.5
Comment 7 Sam Weinig 2017-06-17 04:28:41 PDT
Comment on attachment 313194 [details]
Fix Mac build

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

> Source/WebCore/platform/ios/PasteboardIOS.mm:309
> +NSArray *Pasteboard::supportedFileUploadPasteboardTypes()
> +{
> +    return @[ (NSString *)kUTTypeContent, (NSString *)kUTTypeZipArchive ];
> +}

Is this really iOS specific, or are these also the types we would want to allow for uploading on macOS as well? Or are things so different that it can't be shared?
Comment 8 Wenson Hsieh 2017-06-17 15:22:17 PDT
Comment on attachment 313194 [details]
Fix Mac build

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

>> Source/WebCore/platform/ios/PasteboardIOS.mm:309
>> +}
> 
> Is this really iOS specific, or are these also the types we would want to allow for uploading on macOS as well? Or are things so different that it can't be shared?

The way file uploads work w.r.t. the pasteboard is much different on iOS. On Mac, there's no notion of "supported types" for file uploads, since the source will vend either NSFilesPromisePboardType or NSFilenamesPboardType in the pasteboard, explicitly indicating that the pasteboard contains a file. On iOS, these types don't exist (while kUTTypeFileURL exists, its use is strongly discouraged, and not supported by any system apps). Therefore, on iOS, we need to infer whether certain item providers can be files or not based on their registered UTIs.

For this reason, we already support uploading .zip files on Mac because the file will contain either NSFilesPromisePboardType or NSFilenamesPboardType, but unfortunately, the model for providing files on iOS is completely different, so we need this iOS-specific handling here :/
Comment 9 Wenson Hsieh 2017-06-17 16:24:46 PDT
(In reply to Wenson Hsieh from comment #8)
> Comment on attachment 313194 [details]
> Fix Mac build
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=313194&action=review
> 
> >> Source/WebCore/platform/ios/PasteboardIOS.mm:309
> >> +}
> > 
> > Is this really iOS specific, or are these also the types we would want to allow for uploading on macOS as well? Or are things so different that it can't be shared?
> 
> The way file uploads work w.r.t. the pasteboard is much different on iOS. On
> Mac, there's no notion of "supported types" for file uploads, since the
> source will vend either NSFilesPromisePboardType or NSFilenamesPboardType in
> the pasteboard, explicitly indicating that the pasteboard contains a file.
> On iOS, these types don't exist (while kUTTypeFileURL exists, its use is
> strongly discouraged, and not supported by any system apps). Therefore, on
> iOS, we need to infer whether certain item providers can be files or not
> based on their registered UTIs.
> 
> For this reason, we already support uploading .zip files on Mac because the
> file will contain either NSFilesPromisePboardType or NSFilenamesPboardType,
> but unfortunately, the model for providing files on iOS is completely
> different, so we need this iOS-specific handling here :/

Then again, we could just implement supportedFileUploadPasteboardTypes on Mac to return NSFilesPromisePboardType and NSFilenamesPboardType. It's a *little bit* weird, since supportedFileUploadPasteboardTypes on iOS describes "what types of content we are allowed to consider as files via type conformance", whereas on Mac, this would mean something more like "what pasteboard identifiers indicate that the source added a file". But even in the Mac case, we should be able to just use UTTypeConformsTo anyways since NSFilenamesPboardType and NSFilesPromisePboardType are effectively private UTIs, so the UTTypeConformsTo check would reduce to checking string equality, which is what we do now.

I actually like this approach, since it would allow us to remove the platform-specific guards in DragData::containsFiles.
Comment 10 Wenson Hsieh 2017-06-17 17:46:18 PDT
Created attachment 313215 [details]
Unify DragData::containsFiles on Mac/iOS
Comment 11 WebKit Commit Bot 2017-06-19 14:12:32 PDT
Comment on attachment 313215 [details]
Unify DragData::containsFiles on Mac/iOS

Clearing flags on attachment: 313215

Committed r218508: <http://trac.webkit.org/changeset/218508>
Comment 12 WebKit Commit Bot 2017-06-19 14:12:34 PDT
All reviewed patches have been landed.  Closing bug.