Bug 181192 – [Attachment Support] Create attachment elements when dropping files on iOS

• Component: HTML Editing
• Status: RESOLVED FIXED

[Wenson Hsieh]

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.

[Wenson Hsieh]

Created attachment 330268 [details]
First pass

[Wenson Hsieh]

Created attachment 330307 [details]
v2 with slight tweaks

[Radar WebKit Bug Importer]

<rdar://problem/36280945>

[Tim Horton]

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.

[Wenson Hsieh]

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!

[Wenson Hsieh]

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)

[Wenson Hsieh]

Created attachment 330446 [details]
Rebase on trunk

[Wenson Hsieh]

Created attachment 330449 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 330449 [details]
Patch for landing

Clearing flags on attachment: 330449

Committed r226396: <https://trac.webkit.org/changeset/226396>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.