Title: [243713] trunk
Revision
243713
Author
wenson_hs...@apple.com
Date
2019-04-01 14:21:24 -0700 (Mon, 01 Apr 2019)

Log Message

Unable to copy and paste a PDF from Notes into Mail compose body
https://bugs.webkit.org/show_bug.cgi?id=196442
<rdar://problem/48573098>

Reviewed by Tim Horton.

Source/WebCore:

Refactor some logic for inserting attachment elements upon paste or drop. Currently, we only prefer inserting
content as attachment elements if the items are annotated with UIPreferredPresentationStyleAttachment. However,
many data sources around the system (both first and third party) have not adopted this API, which makes it
difficult to determine whether a given item provider should be treated as a file or not. In this bug in
particular, no preferred presentation style is set, so we fail to handle the paste command by inserting an
attachment element.

However, most apps around the system that write file or attachment-like data to the pasteboard will at least
offer a suggested name for the file, in the form of -[NSItemProvider suggestedName]. To address this, instead of
relying solely on the preferredPresentationStyle, additionally take a suggested name as an indicator that the
item is probably a file.

In fact, Pasteboard::fileContentState already has similar logic to check for either a suggested file name or
explicitly specified presentation style. We pull this out into a separate helper method on PasteboardItemInfo,
and use it for both Pasteboard::fileContentState and prefersAttachmentRepresentation.

Tests:  WKAttachmentTestsIOS.InsertPastedContactAsAttachment
        WKAttachmentTestsIOS.InsertPastedMapItemAsAttachment

* editing/cocoa/WebContentReaderCocoa.mm:
(WebCore::mimeTypeFromContentType):

Work around <rdar://problem/49478229> by using the "text/vcard" MIME type to handle "public.vcard". CoreServices
currently maps "public.vcard" to "text/directory" when using UTTypeCopyPreferredTagWithClass, despite the SPI
-[NSURLFileTypeMappings MIMETypeForExtension:] returning "text/vcard" for a ".vcf" file.

* platform/PasteboardItemInfo.h:
(WebCore::PasteboardItemInfo::canBeTreatedAsAttachmentOrFile const):

Add a helper method to determine whether the PasteboardItemInfo prefers to be represented as inline data, or an
attachment, or neither. This differs slightly from the existing value of preferredPresentationStyle in that we
consider having a suggested file name as a strong indicator that the item should be treated as an attachment,
even if the presentation style is unspecified.

* platform/cocoa/PasteboardCocoa.mm:
(WebCore::Pasteboard::fileContentState):

Use PasteboardItemInfo::canBeTreatedAsAttachmentOrFile().

* platform/ios/PasteboardIOS.mm:
(WebCore::prefersAttachmentRepresentation):

Use PasteboardItemInfo::canBeTreatedAsAttachmentOrFile().

Source/WebKit:

Relax the -canPerformAction: logic in the case of pasting an attachment. Consider an NSItemProvider to possibly
paste as an attachment if either it has a preferred presentation style of UIPreferredPresentationStyleAttachment
or has a style of UIPreferredPresentationStyleUnspecified, but has a suggested file name.

This allows for the "Paste" action to be shown in the callout menu when copying and pasting a non-text file.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView canPerformActionForWebView:withSender:]):

Tools:

Add new API tests to exercise pasting CNContact and MKMapItem-backed item providers. Additionally, adjust an
existing test that pastes a PDF file as an attachment to not require UIPreferredPresentationStyleAttachment
to be specified on the item providers.

* TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(TestWebKitAPI::mapItemForTesting):
(TestWebKitAPI::contactItemForTesting):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (243712 => 243713)


--- trunk/Source/WebCore/ChangeLog	2019-04-01 21:08:31 UTC (rev 243712)
+++ trunk/Source/WebCore/ChangeLog	2019-04-01 21:21:24 UTC (rev 243713)
@@ -1,3 +1,55 @@
+2019-04-01  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Unable to copy and paste a PDF from Notes into Mail compose body
+        https://bugs.webkit.org/show_bug.cgi?id=196442
+        <rdar://problem/48573098>
+
+        Reviewed by Tim Horton.
+
+        Refactor some logic for inserting attachment elements upon paste or drop. Currently, we only prefer inserting
+        content as attachment elements if the items are annotated with UIPreferredPresentationStyleAttachment. However,
+        many data sources around the system (both first and third party) have not adopted this API, which makes it
+        difficult to determine whether a given item provider should be treated as a file or not. In this bug in
+        particular, no preferred presentation style is set, so we fail to handle the paste command by inserting an
+        attachment element.
+
+        However, most apps around the system that write file or attachment-like data to the pasteboard will at least
+        offer a suggested name for the file, in the form of -[NSItemProvider suggestedName]. To address this, instead of
+        relying solely on the preferredPresentationStyle, additionally take a suggested name as an indicator that the
+        item is probably a file.
+
+        In fact, Pasteboard::fileContentState already has similar logic to check for either a suggested file name or
+        explicitly specified presentation style. We pull this out into a separate helper method on PasteboardItemInfo,
+        and use it for both Pasteboard::fileContentState and prefersAttachmentRepresentation.
+
+        Tests:  WKAttachmentTestsIOS.InsertPastedContactAsAttachment
+                WKAttachmentTestsIOS.InsertPastedMapItemAsAttachment
+
+        * editing/cocoa/WebContentReaderCocoa.mm:
+        (WebCore::mimeTypeFromContentType):
+
+        Work around <rdar://problem/49478229> by using the "text/vcard" MIME type to handle "public.vcard". CoreServices
+        currently maps "public.vcard" to "text/directory" when using UTTypeCopyPreferredTagWithClass, despite the SPI
+        -[NSURLFileTypeMappings MIMETypeForExtension:] returning "text/vcard" for a ".vcf" file.
+
+        * platform/PasteboardItemInfo.h:
+        (WebCore::PasteboardItemInfo::canBeTreatedAsAttachmentOrFile const):
+
+        Add a helper method to determine whether the PasteboardItemInfo prefers to be represented as inline data, or an
+        attachment, or neither. This differs slightly from the existing value of preferredPresentationStyle in that we
+        consider having a suggested file name as a strong indicator that the item should be treated as an attachment,
+        even if the presentation style is unspecified.
+
+        * platform/cocoa/PasteboardCocoa.mm:
+        (WebCore::Pasteboard::fileContentState):
+
+        Use PasteboardItemInfo::canBeTreatedAsAttachmentOrFile().
+
+        * platform/ios/PasteboardIOS.mm:
+        (WebCore::prefersAttachmentRepresentation):
+
+        Use PasteboardItemInfo::canBeTreatedAsAttachmentOrFile().
+
 2019-04-01  Tim Horton  <timothy_hor...@apple.com>
 
         Make UIWKDocumentContext rects per-character instead of per-word

Modified: trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm (243712 => 243713)


--- trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm	2019-04-01 21:08:31 UTC (rev 243712)
+++ trunk/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm	2019-04-01 21:21:24 UTC (rev 243713)
@@ -222,6 +222,12 @@
 
 static String mimeTypeFromContentType(const String& contentType)
 {
+    if (contentType == String(kUTTypeVCard)) {
+        // CoreServices erroneously reports that "public.vcard" maps to "text/directory", rather
+        // than either "text/vcard" or "text/x-vcard". Work around this by special casing the
+        // "public.vcard" UTI type. See <rdar://problem/49478229> for more detail.
+        return "text/vcard"_s;
+    }
     return isDeclaredUTI(contentType) ? MIMETypeFromUTI(contentType) : contentType;
 }
 

Modified: trunk/Source/WebCore/platform/PasteboardItemInfo.h (243712 => 243713)


--- trunk/Source/WebCore/platform/PasteboardItemInfo.h	2019-04-01 21:08:31 UTC (rev 243712)
+++ trunk/Source/WebCore/platform/PasteboardItemInfo.h	2019-04-01 21:21:24 UTC (rev 243713)
@@ -56,6 +56,24 @@
         return pathsForFileUpload[index];
     }
 
+    // The preferredPresentationStyle flag is platform API used by drag or copy sources to explicitly indicate
+    // that the data being written to the item provider should be treated as an attachment; unfortunately, not
+    // all clients attempt to set this flag, so we additionally take having a suggested filename as a strong
+    // indicator that the item should be treated as an attachment or file.
+    bool canBeTreatedAsAttachmentOrFile() const
+    {
+        switch (preferredPresentationStyle) {
+        case PasteboardItemPresentationStyle::Inline:
+            return false;
+        case PasteboardItemPresentationStyle::Attachment:
+            return true;
+        case PasteboardItemPresentationStyle::Unspecified:
+            return !suggestedFileName.isEmpty();
+        }
+        ASSERT_NOT_REACHED();
+        return false;
+    }
+
     String contentTypeForHighestFidelityItem() const
     {
         if (contentTypesForFileUpload.isEmpty())

Modified: trunk/Source/WebCore/platform/cocoa/PasteboardCocoa.mm (243712 => 243713)


--- trunk/Source/WebCore/platform/cocoa/PasteboardCocoa.mm	2019-04-01 21:08:31 UTC (rev 243712)
+++ trunk/Source/WebCore/platform/cocoa/PasteboardCocoa.mm	2019-04-01 21:21:24 UTC (rev 243713)
@@ -143,26 +143,14 @@
         // On iOS, files are not written to the pasteboard using file URLs, so we need a heuristic to determine
         // whether or not the pasteboard contains items that represent files. An example of when this gets tricky
         // is differentiating between cases where the user is dragging a plain text file, versus selected text.
-        // Some common signs that indicate a file drop as opposed to dropping inline data are:
-        //
-        //  1. Multiple items - the system generally does not give opportunities to flock multiple pieces of
-        //     selected text.
-        //  2. Preferred attachment presentation style - this means the source has explicitly marked the item
-        //     as a file-like entity, as opposed to inline data.
-        //  3. A suggested name - this means that the source has explicitly specified a potential file name for
-        //     the item when dropped.
-        //  4. The presence of any other declared non-text data in the same item indicates that the content being
-        //     dropped can take on another non-text format, which could be a file.
-        //
-        // If none of these four conditions are satisfied, it's very likely that the content being dropped is just
+        // Also, the presence of any other declared non-text data in the same item indicates that the content
+        // being dropped can take on another non-text format, which could be a file.
+        // If the item can't be treated as an attachment, it's very likely that the content being dropped is just
         // an inline piece of text, with no files in the pasteboard (and therefore, no risk of leaking file paths
         // to web content). In cases such as these, we should not suppress DataTransfer access.
         auto items = platformStrategies()->pasteboardStrategy()->allPasteboardItemInfo(m_pasteboardName);
         mayContainFilePaths = items.size() != 1 || notFound != items.findMatching([] (auto& item) {
-            if (item.preferredPresentationStyle != PasteboardItemPresentationStyle::Unspecified)
-                return item.preferredPresentationStyle == PasteboardItemPresentationStyle::Attachment;
-
-            return !item.suggestedFileName.isEmpty() || item.isNonTextType || item.containsFileURLAndFileUploadContent;
+            return item.canBeTreatedAsAttachmentOrFile() || item.isNonTextType || item.containsFileURLAndFileUploadContent;
         });
     }
 #endif

Modified: trunk/Source/WebCore/platform/ios/PasteboardIOS.mm (243712 => 243713)


--- trunk/Source/WebCore/platform/ios/PasteboardIOS.mm	2019-04-01 21:08:31 UTC (rev 243712)
+++ trunk/Source/WebCore/platform/ios/PasteboardIOS.mm	2019-04-01 21:21:24 UTC (rev 243713)
@@ -270,10 +270,10 @@
     if (contentTypeForHighestFidelityItem.isEmpty())
         return false;
 
-    if (info.preferredPresentationStyle == PasteboardItemPresentationStyle::Attachment)
-        return true;
+    if (info.preferredPresentationStyle == PasteboardItemPresentationStyle::Inline)
+        return false;
 
-    return UTTypeConformsTo(contentTypeForHighestFidelityItem.createCFString().get(), kUTTypeVCard);
+    return info.canBeTreatedAsAttachmentOrFile() || UTTypeConformsTo(contentTypeForHighestFidelityItem.createCFString().get(), kUTTypeVCard);
 }
 
 void Pasteboard::read(PasteboardWebContentReader& reader, WebContentReadingPolicy policy)

Modified: trunk/Source/WebKit/ChangeLog (243712 => 243713)


--- trunk/Source/WebKit/ChangeLog	2019-04-01 21:08:31 UTC (rev 243712)
+++ trunk/Source/WebKit/ChangeLog	2019-04-01 21:21:24 UTC (rev 243713)
@@ -1,3 +1,20 @@
+2019-04-01  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Unable to copy and paste a PDF from Notes into Mail compose body
+        https://bugs.webkit.org/show_bug.cgi?id=196442
+        <rdar://problem/48573098>
+
+        Reviewed by Tim Horton.
+
+        Relax the -canPerformAction: logic in the case of pasting an attachment. Consider an NSItemProvider to possibly
+        paste as an attachment if either it has a preferred presentation style of UIPreferredPresentationStyleAttachment
+        or has a style of UIPreferredPresentationStyleUnspecified, but has a suggested file name.
+
+        This allows for the "Paste" action to be shown in the callout menu when copying and pasting a non-text file.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView canPerformActionForWebView:withSender:]):
+
 2019-04-01  Tim Horton  <timothy_hor...@apple.com>
 
         Make UIWKDocumentContext rects per-character instead of per-word

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (243712 => 243713)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2019-04-01 21:08:31 UTC (rev 243712)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2019-04-01 21:21:24 UTC (rev 243713)
@@ -2826,7 +2826,14 @@
 #if PLATFORM(IOS)
         if (editorState.isContentRichlyEditable && _webView.configuration._attachmentElementEnabled) {
             for (NSItemProvider *itemProvider in pasteboard.itemProviders) {
-                if (itemProvider.preferredPresentationStyle == UIPreferredPresentationStyleAttachment && itemProvider.web_fileUploadContentTypes.count)
+                auto preferredPresentationStyle = itemProvider.preferredPresentationStyle;
+                if (preferredPresentationStyle == UIPreferredPresentationStyleInline)
+                    continue;
+
+                if (preferredPresentationStyle == UIPreferredPresentationStyleUnspecified && !itemProvider.suggestedName.length)
+                    continue;
+
+                if (itemProvider.web_fileUploadContentTypes.count)
                     return YES;
             }
         }

Modified: trunk/Tools/ChangeLog (243712 => 243713)


--- trunk/Tools/ChangeLog	2019-04-01 21:08:31 UTC (rev 243712)
+++ trunk/Tools/ChangeLog	2019-04-01 21:21:24 UTC (rev 243713)
@@ -1,3 +1,19 @@
+2019-04-01  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Unable to copy and paste a PDF from Notes into Mail compose body
+        https://bugs.webkit.org/show_bug.cgi?id=196442
+        <rdar://problem/48573098>
+
+        Reviewed by Tim Horton.
+
+        Add new API tests to exercise pasting CNContact and MKMapItem-backed item providers. Additionally, adjust an
+        existing test that pastes a PDF file as an attachment to not require UIPreferredPresentationStyleAttachment
+        to be specified on the item providers.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
+        (TestWebKitAPI::mapItemForTesting):
+        (TestWebKitAPI::contactItemForTesting):
+
 2019-04-01  Tim Horton  <timothy_hor...@apple.com>
 
         Make UIWKDocumentContext rects per-character instead of per-word

Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm (243712 => 243713)


--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm	2019-04-01 21:08:31 UTC (rev 243712)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm	2019-04-01 21:21:24 UTC (rev 243713)
@@ -1804,16 +1804,33 @@
     [dragAndDropSimulator endDataTransfer];
 }
 
-TEST(WKAttachmentTestsIOS, InsertDroppedMapItemAsAttachment)
+static RetainPtr<NSItemProvider> mapItemForTesting()
 {
     auto placemark = adoptNS([allocMKPlacemarkInstance() initWithCoordinate:CLLocationCoordinate2DMake(37.3327, -122.0053)]);
     auto mapItem = adoptNS([allocMKMapItemInstance() initWithPlacemark:placemark.get()]);
-    [mapItem setName:@"Apple Park"];
+    [mapItem setName:@"Apple Park.vcf"];
 
     auto itemProvider = adoptNS([[NSItemProvider alloc] init]);
     [itemProvider registerObject:mapItem.get() visibility:NSItemProviderRepresentationVisibilityAll];
     [itemProvider setSuggestedName:[mapItem name]];
+    return itemProvider;
+}
 
+static RetainPtr<NSItemProvider> contactItemForTesting()
+{
+    auto contact = adoptNS([allocCNMutableContactInstance() init]);
+    [contact setGivenName:@"Foo"];
+    [contact setFamilyName:@"Bar"];
+
+    auto itemProvider = adoptNS([[NSItemProvider alloc] init]);
+    [itemProvider registerObject:contact.get() visibility:NSItemProviderRepresentationVisibilityAll];
+    [itemProvider setSuggestedName:@"Foo Bar.vcf"];
+    return itemProvider;
+}
+
+TEST(WKAttachmentTestsIOS, InsertDroppedMapItemAsAttachment)
+{
+    auto itemProvider = mapItemForTesting();
     auto webView = webViewForTestingAttachments();
     auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
     [simulator setExternalItemProviders:@[ itemProvider.get() ]];
@@ -1832,14 +1849,7 @@
 
 TEST(WKAttachmentTestsIOS, InsertDroppedContactAsAttachment)
 {
-    auto contact = adoptNS([allocCNMutableContactInstance() init]);
-    [contact setGivenName:@"Foo"];
-    [contact setFamilyName:@"Bar"];
-
-    auto itemProvider = adoptNS([[NSItemProvider alloc] init]);
-    [itemProvider registerObject:contact.get() visibility:NSItemProviderRepresentationVisibilityAll];
-    [itemProvider setSuggestedName:@"Foo Bar"];
-
+    auto itemProvider = contactItemForTesting();
     auto webView = webViewForTestingAttachments();
     auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
     [simulator setExternalItemProviders:@[ itemProvider.get() ]];
@@ -1854,16 +1864,49 @@
     EXPECT_WK_STREQ("text/vcard", info.contentType);
 }
 
+TEST(WKAttachmentTestsIOS, InsertPastedContactAsAttachment)
+{
+    UIPasteboard.generalPasteboard.itemProviders = @[ contactItemForTesting().autorelease() ];
+    auto webView = webViewForTestingAttachments();
+    ObserveAttachmentUpdatesForScope observer(webView.get());
+    [webView paste:nil];
+
+    [webView expectElementCount:0 querySelector:@"a"];
+    EXPECT_WK_STREQ("Foo Bar.vcf", [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment').title"]);
+    EXPECT_WK_STREQ("text/vcard", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+    EXPECT_EQ(1U, observer.observer().inserted.count);
+    _WKAttachment *attachment = observer.observer().inserted.firstObject;
+    EXPECT_WK_STREQ("Foo Bar.vcf", attachment.info.name);
+    EXPECT_WK_STREQ("text/vcard", attachment.info.contentType);
+}
+
+TEST(WKAttachmentTestsIOS, InsertPastedMapItemAsAttachment)
+{
+    UIApplicationInitialize();
+    UIPasteboard.generalPasteboard.itemProviders = @[ mapItemForTesting().autorelease() ];
+    auto webView = webViewForTestingAttachments();
+    ObserveAttachmentUpdatesForScope observer(webView.get());
+    [webView paste:nil];
+
+    NSURL *pastedLinkURL = [NSURL URLWithString:[webView valueOfAttribute:@"href" forQuerySelector:@"a"]];
+    [webView expectElementTag:@"A" toComeBefore:@"ATTACHMENT"];
+    EXPECT_WK_STREQ("maps.apple.com", pastedLinkURL.host);
+    EXPECT_WK_STREQ("Apple Park.vcf", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
+    EXPECT_WK_STREQ("text/vcard", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+    EXPECT_EQ(1U, observer.observer().inserted.count);
+    _WKAttachment *attachment = observer.observer().inserted.firstObject;
+    EXPECT_WK_STREQ("Apple Park.vcf", attachment.info.name);
+    EXPECT_WK_STREQ("text/vcard", attachment.info.contentType);
+}
+
 TEST(WKAttachmentTestsIOS, InsertPastedFilesAsAttachments)
 {
     auto pdfItem = adoptNS([[NSItemProvider alloc] init]);
     [pdfItem setSuggestedName:@"doc"];
-    [pdfItem setPreferredPresentationStyle:UIPreferredPresentationStyleAttachment];
     [pdfItem registerData:testPDFData() type:(__bridge NSString *)kUTTypePDF];
 
     auto textItem = adoptNS([[NSItemProvider alloc] init]);
     [textItem setSuggestedName:@"hello"];
-    [textItem setPreferredPresentationStyle:UIPreferredPresentationStyleAttachment];
     [textItem registerData:[@"helloworld" dataUsingEncoding:NSUTF8StringEncoding] type:(__bridge NSString *)kUTTypePlainText];
 
     UIPasteboard.generalPasteboard.itemProviders = @[ pdfItem.get(), textItem.get() ];
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to