Bug 181192 - [Attachment Support] Create attachment elements when dropping files on iOS
Summary: [Attachment Support] Create attachment elements when dropping files on iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks: 181199
  Show dependency treegraph
 
Reported: 2017-12-29 18:58 PST by Wenson Hsieh
Modified: 2018-01-03 23:45 PST (History)
5 users (show)

See Also:


Attachments
First pass (68.49 KB, patch)
2017-12-31 15:58 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
v2 with slight tweaks (69.80 KB, patch)
2018-01-01 20:42 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Rebase on trunk (70.46 KB, patch)
2018-01-03 22:09 PST, Wenson Hsieh
no flags Details | Formatted Diff | Diff
Patch for landing (70.42 KB, patch)
2018-01-03 23:11 PST, 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-12-29 18:58:30 PST
This logic currently resides in WebContentReaderMac, so we currently only represent dropped file paths as attachments on Mac.

We should refactor this logic so that it resides in WebContentReaderCocoa, and teach iOS to insert attachment elements for content that either (1) explicitly prefers an attachment presentation style, as specified on the UIItemProvider, or (2) would otherwise not have a relevant DOM representation in editable areas. Of course, we should only do this if the runtime switch for attachment elements is enabled.
Comment 1 Wenson Hsieh 2017-12-31 15:58:01 PST
Created attachment 330268 [details]
First pass
Comment 2 Wenson Hsieh 2018-01-01 20:42:23 PST
Created attachment 330307 [details]
v2 with slight tweaks
Comment 3 Radar WebKit Bug Importer 2018-01-03 12:43:06 PST
<rdar://problem/36280945>
Comment 4 Tim Horton 2018-01-03 14:47:15 PST
Comment on attachment 330307 [details]
v2 with slight tweaks

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

> Source/WebCore/editing/WebContentReader.cpp:34
> +void WebContentReader::ensureFragment()

I feel like ensure functions usually return the thing they’re ensuring.

> Source/WebCore/platform/PasteboardItemInfo.h:37
> +    bool prefersAttachmentPresentation;
> +    bool prefersInlinePresentation;

Both as separate bits? Seems a bit weird, what if you set both? Should be an enum, no?

> Source/WebKit/Scripts/webkit/messages.py:379
> +        'WebCore::PasteboardItemInfo': ['<WebCore/PasteboardItemInfo.h>'],

You shouldn’t have to add this.
Comment 5 Wenson Hsieh 2018-01-03 20:30:21 PST
Comment on attachment 330307 [details]
v2 with slight tweaks

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

>> Source/WebCore/editing/WebContentReader.cpp:34
>> +void WebContentReader::ensureFragment()
> 
> I feel like ensure functions usually return the thing they’re ensuring.

Seems reasonable — made this return the DocumentFragment&.

>> Source/WebCore/platform/PasteboardItemInfo.h:37
>> +    bool prefersInlinePresentation;
> 
> Both as separate bits? Seems a bit weird, what if you set both? Should be an enum, no?

Yeah, good point. An enum is a better fit for this.

Added a new enum class, PasteboardItemPresentationStyle, to represent these styles, with three values: Unspecified, Inline, and Attachment

>> Source/WebKit/Scripts/webkit/messages.py:379
>> +        'WebCore::PasteboardItemInfo': ['<WebCore/PasteboardItemInfo.h>'],
> 
> You shouldn’t have to add this.

Removed!
Comment 6 Wenson Hsieh 2018-01-03 22:06:14 PST
Comment on attachment 330307 [details]
v2 with slight tweaks

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

> Source/WebCore/platform/ios/PlatformPasteboardIOS.mm:117
> +    NSItemProvider *itemProvider = [[m_pasteboard itemProviders] objectAtIndex:index];

(Also, I just realized that I need to guard this with PASTEBOARD_SUPPORTS_ITEM_PROVIDERS, since UIPasteboard's -itemProviders do not exist on tvOS and watchOS)
Comment 7 Wenson Hsieh 2018-01-03 22:09:03 PST
Created attachment 330446 [details]
Rebase on trunk
Comment 8 Wenson Hsieh 2018-01-03 23:11:59 PST
Created attachment 330449 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2018-01-03 23:44:59 PST
Comment on attachment 330449 [details]
Patch for landing

Clearing flags on attachment: 330449

Committed r226396: <https://trac.webkit.org/changeset/226396>
Comment 10 WebKit Commit Bot 2018-01-03 23:45:00 PST
All reviewed patches have been landed.  Closing bug.