Attachment 370813 Details for Bug 198315 – Patch

[patch] Patch

bug-198315-20190528173242.patch (text/plain), 18.12 KB, created by Wenson Hsieh on 2019-05-28 17:32:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-05-28 17:32:42 PDT

Size: 18.12 KB

patch

obsolete

>Subversion Revision: 245779
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 7742c4f766c39c75cb766897d6e8fe86cf3b7f50..5202eda6f1b8fa1e6ef132ccdd7b0bf73cd561a3 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,39 @@
>+2019-05-28  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Respect NSItemProvider's registered types when dropping files that are loaded in-place
>+        https://bugs.webkit.org/show_bug.cgi?id=198315
>+        <rdar://problem/51183762>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Currently, logic in PasteboardIOS.mm and WebContentReaderCocoa.mm attempts to deduce the content type from the
>+        file path when dropping attachments on iOS. Instead, we should be plumbing the content type through to the
>+        reader.
>+
>+        Test: WKAttachmentTestsIOS.InsertDroppedImageWithNonImageFileExtension
>+
>+        * editing/WebContentReader.h:
>+        * editing/cocoa/WebContentReaderCocoa.mm:
>+        (WebCore::typeForAttachmentElement):
>+
>+        Add a helper method to determine which type to use in attachment elements. This makes the paste
>+        (attachmentForData) and drop (attachmentForFilePaths) behave the same way, with respect to the type attribute
>+        used to represent the attachment.
>+
>+        (WebCore::attachmentForFilePath):
>+
>+        Use the content type, if specified; otherwise, fall back to deducing it from the file path.
>+
>+        (WebCore::attachmentForData):
>+        (WebCore::WebContentReader::readFilePath):
>+        * platform/Pasteboard.h:
>+        (WebCore::PasteboardWebContentReader::readFilePath):
>+
>+        Pass the highest fidelity representation's content type to the web content reader.
>+
>+        * platform/ios/PasteboardIOS.mm:
>+        (WebCore::Pasteboard::readRespectingUTIFidelities):
>+
> 2019-05-26  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Verification] Add areEssentiallyEqual for LayoutRect
>diff --git a/Source/WebCore/editing/WebContentReader.h b/Source/WebCore/editing/WebContentReader.h
>index 9c9641029d6123445cfe58e6e570e28370962501..2b9e99999cb778591a60ac0d3d96aebbaadef1c4 100644
>--- a/Source/WebCore/editing/WebContentReader.h
>+++ b/Source/WebCore/editing/WebContentReader.h
>@@ -71,7 +71,7 @@ public:
> private:
> #if PLATFORM(COCOA)
>     bool readWebArchive(SharedBuffer&) override;
>-    bool readFilePath(const String&, Optional<FloatSize> preferredPresentationSize = { }) override;
>+    bool readFilePath(const String&, Optional<FloatSize> preferredPresentationSize = { }, const String& contentType = { }) override;
>     bool readFilePaths(const Vector<String>&) override;
>     bool readHTML(const String&) override;
>     bool readRTFD(SharedBuffer&) override;
>@@ -95,7 +95,7 @@ public:
> private:
> #if PLATFORM(COCOA)
>     bool readWebArchive(SharedBuffer&) override;
>-    bool readFilePath(const String&, Optional<FloatSize> = { }) override { return false; }
>+    bool readFilePath(const String&, Optional<FloatSize> = { }, const String& = { }) override { return false; }
>     bool readFilePaths(const Vector<String>&) override { return false; }
>     bool readHTML(const String&) override;
>     bool readRTFD(SharedBuffer&) override;
>diff --git a/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm b/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm
>index 532a84a9c45e1f24460da25758939815b08a8252..be85b0ab30a3ba7873d6aabc60ba54ab334f2068 100644
>--- a/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm
>+++ b/Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm
>@@ -695,7 +695,16 @@ bool WebContentReader::readImage(Ref<SharedBuffer>&& buffer, const String& type,
> 
> #if ENABLE(ATTACHMENT_ELEMENT)
> 
>-static Ref<HTMLElement> attachmentForFilePath(Frame& frame, const String& path, Optional<FloatSize> preferredSize)
>+static String typeForAttachmentElement(const String& contentType)
>+{
>+    if (contentType.isEmpty())
>+        return { };
>+
>+    auto mimeType = mimeTypeFromContentType(contentType);
>+    return mimeType.isEmpty() ? contentType : mimeType;
>+}
>+
>+static Ref<HTMLElement> attachmentForFilePath(Frame& frame, const String& path, Optional<FloatSize> preferredSize, const String& explicitContentType)
> {
>     auto document = makeRef(*frame.document());
>     auto attachment = HTMLAttachmentElement::create(HTMLNames::attachmentTag, document);
>@@ -704,17 +713,23 @@ static Ref<HTMLElement> attachmentForFilePath(Frame& frame, const String& path,
>         return attachment;
>     }
> 
>-    String contentType;
>+    bool isDirectory = FileSystem::fileIsDirectory(path, FileSystem::ShouldFollowSymbolicLinks::Yes);
>+    String contentType = typeForAttachmentElement(explicitContentType);
>+    if (contentType.isEmpty()) {
>+        if (isDirectory)
>+            contentType = kUTTypeDirectory;
>+        else {
>+            contentType = File::contentTypeForFile(path);
>+            if (contentType.isEmpty())
>+                contentType = kUTTypeData;
>+        }
>+    }
>+
>     Optional<uint64_t> fileSizeForDisplay;
>-    if (FileSystem::fileIsDirectory(path, FileSystem::ShouldFollowSymbolicLinks::Yes))
>-        contentType = kUTTypeDirectory;
>-    else {
>+    if (!isDirectory) {
>         long long fileSize;
>         FileSystem::getFileSize(path, fileSize);
>         fileSizeForDisplay = fileSize;
>-        contentType = File::contentTypeForFile(path);
>-        if (contentType.isEmpty())
>-            contentType = kUTTypeData;
>     }
> 
>     frame.editor().registerAttachmentIdentifier(attachment->ensureUniqueIdentifier(), contentType, path);
>@@ -738,8 +753,7 @@ static Ref<HTMLElement> attachmentForData(Frame& frame, SharedBuffer& buffer, co
> {
>     auto document = makeRef(*frame.document());
>     auto attachment = HTMLAttachmentElement::create(HTMLNames::attachmentTag, document);
>-    auto mimeType = mimeTypeFromContentType(contentType);
>-    auto typeForAttachmentElement = mimeType.isEmpty() ? contentType : mimeType;
>+    auto attachmentType = typeForAttachmentElement(contentType);
> 
>     // FIXME: We should instead ask CoreServices for a preferred name corresponding to the given content type.
>     static const char* defaultAttachmentName = "file";
>@@ -751,15 +765,15 @@ static Ref<HTMLElement> attachmentForData(Frame& frame, SharedBuffer& buffer, co
>         fileName = name;
> 
>     if (!supportsClientSideAttachmentData(frame)) {
>-        attachment->setFile(File::create(Blob::create(buffer, WTFMove(typeForAttachmentElement)), fileName));
>+        attachment->setFile(File::create(Blob::create(buffer, WTFMove(attachmentType)), fileName));
>         return attachment;
>     }
> 
>-    frame.editor().registerAttachmentIdentifier(attachment->ensureUniqueIdentifier(), typeForAttachmentElement, fileName, buffer);
>+    frame.editor().registerAttachmentIdentifier(attachment->ensureUniqueIdentifier(), attachmentType, fileName, buffer);
> 
>-    if (contentTypeIsSuitableForInlineImageRepresentation(typeForAttachmentElement)) {
>+    if (contentTypeIsSuitableForInlineImageRepresentation(attachmentType)) {
>         auto image = HTMLImageElement::create(document);
>-        image->setAttributeWithoutSynchronization(HTMLNames::srcAttr, DOMURL::createObjectURL(document, File::create(Blob::create(buffer, WTFMove(typeForAttachmentElement)), WTFMove(fileName))));
>+        image->setAttributeWithoutSynchronization(HTMLNames::srcAttr, DOMURL::createObjectURL(document, File::create(Blob::create(buffer, WTFMove(attachmentType)), WTFMove(fileName))));
>         image->setAttachmentElement(WTFMove(attachment));
>         if (preferredSize) {
>             image->setAttributeWithoutSynchronization(HTMLNames::widthAttr, AtomicString::number(preferredSize->width()));
>@@ -768,13 +782,13 @@ static Ref<HTMLElement> attachmentForData(Frame& frame, SharedBuffer& buffer, co
>         return image;
>     }
> 
>-    attachment->updateAttributes({ buffer.size() }, WTFMove(typeForAttachmentElement), WTFMove(fileName));
>+    attachment->updateAttributes({ buffer.size() }, WTFMove(attachmentType), WTFMove(fileName));
>     return attachment;
> }
> 
> #endif // ENABLE(ATTACHMENT_ELEMENT)
> 
>-bool WebContentReader::readFilePath(const String& path, Optional<FloatSize> preferredPresentationSize)
>+bool WebContentReader::readFilePath(const String& path, Optional<FloatSize> preferredPresentationSize, const String& contentType)
> {
>     if (path.isEmpty() || !frame.document())
>         return false;
>@@ -785,7 +799,7 @@ bool WebContentReader::readFilePath(const String& path, Optional<FloatSize> pref
> 
> #if ENABLE(ATTACHMENT_ELEMENT)
>     if (RuntimeEnabledFeatures::sharedFeatures().attachmentElementEnabled())
>-        fragment->appendChild(attachmentForFilePath(frame, path, preferredPresentationSize));
>+        fragment->appendChild(attachmentForFilePath(frame, path, preferredPresentationSize, contentType));
> #endif
> 
>     return true;
>diff --git a/Source/WebCore/platform/Pasteboard.h b/Source/WebCore/platform/Pasteboard.h
>index 647dc5fc4a9bb0d9b435ec181c4e5236fcc1dc10..97eeb1f510fcc3b16d2fea0e77a40fbd59a19dfa 100644
>--- a/Source/WebCore/platform/Pasteboard.h
>+++ b/Source/WebCore/platform/Pasteboard.h
>@@ -136,7 +136,7 @@ public:
> 
> #if PLATFORM(COCOA)
>     virtual bool readWebArchive(SharedBuffer&) = 0;
>-    virtual bool readFilePath(const String&, Optional<FloatSize> preferredPresentationSize = { }) = 0;
>+    virtual bool readFilePath(const String&, Optional<FloatSize> preferredPresentationSize = { }, const String& contentType = { }) = 0;
>     virtual bool readFilePaths(const Vector<String>&) = 0;
>     virtual bool readHTML(const String&) = 0;
>     virtual bool readRTFD(SharedBuffer&) = 0;
>diff --git a/Source/WebCore/platform/ios/PasteboardIOS.mm b/Source/WebCore/platform/ios/PasteboardIOS.mm
>index 416295677f7aef2d3b8dab1ed6e4777f972d3ea5..8e005cab51abc3d631cd54b3f31d0338ca8f5e4e 100644
>--- a/Source/WebCore/platform/ios/PasteboardIOS.mm
>+++ b/Source/WebCore/platform/ios/PasteboardIOS.mm
>@@ -345,9 +345,10 @@ void Pasteboard::readRespectingUTIFidelities(PasteboardWebContentReader& reader,
>         auto info = strategy.informationForItemAtIndex(index, m_pasteboardName);
>         auto attachmentFilePath = info.pathForHighestFidelityItem();
>         bool canReadAttachment = policy == WebContentReadingPolicy::AnyType && RuntimeEnabledFeatures::sharedFeatures().attachmentElementEnabled() && !attachmentFilePath.isEmpty();
>+        auto contentType = info.contentTypeForHighestFidelityItem();
>         if (canReadAttachment && prefersAttachmentRepresentation(info)) {
>-            readURLAlongsideAttachmentIfNecessary(reader, strategy, info.contentTypeForHighestFidelityItem(), m_pasteboardName, index);
>-            reader.readFilePath(WTFMove(attachmentFilePath), info.preferredPresentationSize);
>+            readURLAlongsideAttachmentIfNecessary(reader, strategy, contentType, m_pasteboardName, index);
>+            reader.readFilePath(WTFMove(attachmentFilePath), info.preferredPresentationSize, contentType);
>             continue;
>         }
> #endif
>@@ -366,7 +367,7 @@ void Pasteboard::readRespectingUTIFidelities(PasteboardWebContentReader& reader,
>         }
> #if ENABLE(ATTACHMENT_ELEMENT)
>         if (canReadAttachment && result == ReaderResult::DidNotReadType)
>-            reader.readFilePath(WTFMove(attachmentFilePath), info.preferredPresentationSize);
>+            reader.readFilePath(WTFMove(attachmentFilePath), info.preferredPresentationSize, contentType);
> #endif
>     }
> }
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index ba94a7a046ae3be4aebd35a181ccccc5880d76dd..6c1cb21151edf0df5ed2f110f3706aa28ac406f9 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-05-28  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Respect NSItemProvider's registered types when dropping files that are loaded in-place
>+        https://bugs.webkit.org/show_bug.cgi?id=198315
>+        <rdar://problem/51183762>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Adds a new API test to verify that when dropping a file that is loaded in-place with a file extension that is
>+        not a .png (but was registered to the item provider as "public.png"), the resulting attachment is contained in
>+        an image element, and the resulting attachment info indicates that the dropped attachment is a png file.
>+
>+        Additionally, rebaseline some existing tests.
>+
>+        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
>+        (runTestWithTemporaryImageFile):
>+        (TestWebKitAPI::TEST):
>+
> 2019-05-26  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] Dropping in an editable element should result in a ranged selection
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
>index 109116bc4c67001a0ea26f633dd400d403906731..fdcc077e6adbdffa9ae9eaac416b0fab0a2cd212 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
>+++ b/Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
>@@ -410,6 +410,20 @@ static void runTestWithTemporaryFolder(void(^runTest)(NSURL *folderURL))
>     }
> }
> 
>+static void runTestWithTemporaryImageFile(NSString *fileName, void(^runTest)(NSURL *fileURL))
>+{
>+    NSFileManager *defaultManager = [NSFileManager defaultManager];
>+    auto temporaryFilePath = retainPtr([NSTemporaryDirectory() stringByAppendingPathComponent:fileName]);
>+    auto temporaryFileURL = retainPtr([NSURL fileURLWithPath:temporaryFilePath.get()]);
>+    [defaultManager removeItemAtURL:temporaryFileURL.get() error:nil];
>+    [testImageData() writeToFile:temporaryFilePath.get() atomically:YES];
>+    @try {
>+        runTest(temporaryFileURL.get());
>+    } @finally {
>+        [defaultManager removeItemAtURL:temporaryFileURL.get() error:nil];
>+    }
>+}
>+
> static void simulateFolderDragWithURL(DragAndDropSimulator *simulator, NSURL *folderURL)
> {
> #if PLATFORM(MAC)
>@@ -789,7 +803,7 @@ TEST(WKAttachmentTests, DropFolderAsAttachmentAndMoveByDragging)
>         TestWKWebView *webView = [simulator webView];
>         auto attachment = retainPtr([simulator insertedAttachments].firstObject);
>         EXPECT_WK_STREQ([attachment uniqueIdentifier], [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment').uniqueIdentifier"]);
>-        EXPECT_WK_STREQ((__bridge NSString *)kUTTypeDirectory, [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
>+        EXPECT_WK_STREQ((__bridge NSString *)kUTTypeFolder, [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
>         EXPECT_WK_STREQ(folderURL.lastPathComponent, [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
> 
>         NSFileWrapper *image = [attachment info].fileWrapper.fileWrappers[@"image.png"];
>@@ -1704,9 +1718,9 @@ TEST(WKAttachmentTestsIOS, InsertDroppedRichAndPlainTextFilesAsAttachments)
> 
>     [webView expectElementCount:2 querySelector:@"ATTACHMENT"];
>     EXPECT_WK_STREQ("hello.rtf", [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[0].getAttribute('title')"]);
>-    EXPECT_WK_STREQ("text/rtf", [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[0].getAttribute('type')"]);
>+    EXPECT_WK_STREQ((__bridge NSString *)kUTTypeFlatRTFD, [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[0].getAttribute('type')"]);
>     EXPECT_WK_STREQ("world.txt", [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[1].getAttribute('title')"]);
>-    EXPECT_WK_STREQ("text/plain", [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[1].getAttribute('type')"]);
>+    EXPECT_WK_STREQ((__bridge NSString *)kUTTypeUTF8PlainText, [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[1].getAttribute('type')"]);
> }
> 
> TEST(WKAttachmentTestsIOS, InsertDroppedZipArchiveAsAttachment)
>@@ -1763,7 +1777,7 @@ TEST(WKAttachmentTestsIOS, InsertDroppedItemProvidersInOrder)
>     [webView expectElementTagsInOrder:@[ @"ATTACHMENT", @"A", @"ATTACHMENT" ]];
> 
>     EXPECT_WK_STREQ("first.txt", [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[0].getAttribute('title')"]);
>-    EXPECT_WK_STREQ("text/plain", [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[0].getAttribute('type')"]);
>+    EXPECT_WK_STREQ((__bridge NSString *)kUTTypeUTF8PlainText, [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[0].getAttribute('type')"]);
>     EXPECT_WK_STREQ([appleURL absoluteString], [webView valueOfAttribute:@"href" forQuerySelector:@"a"]);
>     EXPECT_WK_STREQ("second.pdf", [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[1].getAttribute('title')"]);
>     EXPECT_WK_STREQ("application/pdf", [webView stringByEvaluatingJavaScript:@"document.querySelectorAll('attachment')[1].getAttribute('type')"]);
>@@ -1966,6 +1980,32 @@ TEST(WKAttachmentTestsIOS, InsertPastedFilesAsAttachments)
>     EXPECT_TRUE([webView canPerformAction:@selector(paste:) withSender:nil]);
> }
> 
>+TEST(WKAttachmentTestsIOS, InsertDroppedImageWithNonImageFileExtension)
>+{
>+    runTestWithTemporaryImageFile(@"image.hello", ^(NSURL *fileURL) {
>+        auto item = adoptNS([[NSItemProvider alloc] init]);
>+        [item setSuggestedName:@"image.hello"];
>+        [item registerFileRepresentationForTypeIdentifier:(__bridge NSString *)kUTTypePNG fileOptions:NSItemProviderFileOptionOpenInPlace visibility:NSItemProviderRepresentationVisibilityAll loadHandler:^NSProgress *(void (^callback)(NSURL *, BOOL, NSError *))
>+        {
>+            callback(fileURL, YES, nil);
>+            return nil;
>+        }];
>+
>+        auto webView = webViewForTestingAttachments();
>+        auto dragAndDropSimulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
>+        [dragAndDropSimulator setExternalItemProviders:@[ item.get() ]];
>+        [dragAndDropSimulator runFrom:CGPointZero to:CGPointMake(50, 50)];
>+
>+        EXPECT_EQ(1U, [dragAndDropSimulator insertedAttachments].count);
>+        _WKAttachment *attachment = [dragAndDropSimulator insertedAttachments].firstObject;
>+        _WKAttachmentInfo *info = attachment.info;
>+        EXPECT_WK_STREQ("image/png", info.contentType);
>+        EXPECT_WK_STREQ("image.hello", info.filePath.lastPathComponent);
>+        EXPECT_WK_STREQ("image.hello", info.name);
>+        [webView expectElementCount:1 querySelector:@"IMG"];
>+    });
>+}
>+
> #if HAVE(PENCILKIT)
> static BOOL forEachViewInHierarchy(UIView *view, void(^mapFunction)(UIView *subview, BOOL *stop))
> {

Actions: View | Formatted Diff | Diff

Attachments on bug 198315: 370813 | 370814 | 370820