[Cocoa] Exception (fileType 'dyn.agq8u' is not a valid UTI) raised when dragging...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Aug 2018 20:14:21 +0000 (20:14 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 27 Aug 2018 20:14:21 +0000 (20:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188903
<rdar://problem/43702993>

Reviewed by Tim Horton.

Source/WebCore:

Fixes the exception for attachments that are created when dropping files with extensions that don't map to any
known UTIs, and when dropping folders. See below for more detail.

Tests:  WKAttachmentTests.InsertFolderAndFileWithUnknownExtension
        WKAttachmentTests.DropFolderAsAttachmentAndMoveByDragging
        WKAttachmentTests.ChangeAttachmentDataAndFileInformation

* editing/Editor.cpp:
(WebCore::Editor::insertAttachment):
* editing/Editor.h:
* editing/cocoa/WebContentReaderCocoa.mm:
(WebCore::WebContentReader::readFilePaths):

When creating an attachment by dropping or pasting a file backed by a file path, handle the cases where…
(1)     the dropped path is a directory, by setting the UTI to "public.directory". This allows us to show a
        folder icon for the dropped attachment element on macOS.
(2)     the dropped path is a file whose UTI is unknown, by defaulting to "public.data".

By ensuring that the UTI of a dropped file-backed attachment is set to a concrete type in any case, we avoid an
exception when dragging the attachment on macOS, and on iOS, avoid silently failing to drag an attachment.

* html/HTMLAttachmentElement.cpp:
(WebCore::HTMLAttachmentElement::updateAttributes):

Change this method to take an optional file size (the subtitle attribute will only be set if the file size is
not `std::nullopt`). Furthermore, allow callers of this method to clear attributes on the attachment element by
passing in `std::nullopt` for any of the three arguments. This allows us to handle the case where an
attachment's file wrapper is changed from a regular file to a folder whose total size is currently unknown.
Instead of showing "0 bytes", we'll simply refrain from showing a subtitle at all (in the future, this should
be improved by implementing a way to estimate the size of the files in the folder, or perhaps show the number of
items in the folder as the subtitle).

* html/HTMLAttachmentElement.h:

Source/WebKit:

Fixes the bug by supporting NSFileWrappers of type directory, as well as NSFileWrappers with file that do not
map to concrete type identifiers. Among other things, this patch ensures that:
-       Inserting a directory file wrapper (or using -setFileWrapper:…: to change an existing _WKAttachment's
        file wrapper to a directory) does not cause the attachment element to show "0 bytes" as the subtitle.
-       In the above scenario, we also won't end up with a missing "type" attribute for the attachment element,
        as well as a corresponding API::Attachment::contentType() that's an empty string.
-       Dropping or pasting attachments backed by paths on disk also doesn't trigger these problems, if the path
        is a directory or of unknown file type.

Changes are verified by 2 new API tests.

* UIProcess/API/APIAttachment.cpp:
(API::Attachment::updateAttributes):
(API::Attachment::fileSizeForDisplay const):
* UIProcess/API/APIAttachment.h:
* UIProcess/API/Cocoa/APIAttachmentCocoa.mm:
(API::Attachment::setFileWrapperAndUpdateContentType):

Add a helper that sets the file wrapper to the given NSFileWrapper, and either sets the content type to the
given content type if it's specified, or infers it from the file extension of the new NSFileWrapper. Invoked
whenever an NSFileWrapper and content type combination is set on an API attachment via WebKit SPI.

(API::Attachment::fileSizeForDisplay const):

Returns a file size to be displayed in the attachment element's subtitle. This returns an optional file size,
where `std::nullopt` indicates that there should not be a file size shown. For now, this returns `std::nullopt`
for directory file wrappers, though in the future, this should be done only in cases where we don't immediately
have a size estimate for the file wrapper.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _insertAttachmentWithFileWrapper:contentType:options:completion:]):

Use API::Attachment::setFileWrapperAndUpdateContentType() instead of trying to come up with a fallback UTI.

* UIProcess/API/Cocoa/_WKAttachment.mm:
(-[_WKAttachment setFileWrapper:contentType:completion:]):

Use API::Attachment::setFileWrapperAndUpdateContentType() instead of trying to come up with a fallback UTI.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::insertAttachment):

Remove the separate arguments for file size, content type, and file name, and instead get them from the given
API Attachment object.

(WebKit::WebPageProxy::updateAttachmentAttributes):

Remove separate arguments for file size, content type and file name and just take an API::Attachment instead.
These separate pieces of information can simply be asked from the Attachment itself.

* UIProcess/WebPageProxy.h:
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::insertAttachment):
(WebKit::WebPage::updateAttachmentAttributes):

Adjust some interfaces here to allow the displayed file size to be optional.

* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:

Tools:

Add two API tests and adjust existing WKAttachment API tests. The new tests exercise the following scenarios, in
both iOS and macOS:
•       Dropping a folder as an attachment element, and then moving that attachment element in the document by
        dragging and dropping.
•       Using WKWebView SPI to insert a folder and a file with an unknown extension, and then using more
        _WKAttachment SPI to swap the attachments' backing file wrappers.

* TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
(runTestWithTemporaryFolder):

Add a helper function to run a test with a new folder path, created in the temporary directory and populated
with some sample content. This folder is deleted after running the test.

(simulateFolderDragWithURL):

Add a helper function to prepare a given DragAndDropSimulator for simulating a dragged folder from a source
external to the web view.

(platformCopyRichTextWithMultipleAttachments):
(platformCopyRichTextWithImage):
(platformCopyPNG):
(TestWebKitAPI::TEST):

Add new API tests, and adjust existing tests to reflect new -setFileWrapper:…: behavior. Specifically,
ChangeAttachmentDataAndFileInformation previously required that changing a _WKAttachment's NSFileWrapper would
preserve the previous NSFileWrapper's preferred name if the new file wrapper does not have a preferred name, but
this quirk is no longer supported.

Also add a few bridging casts for the eventual transition of TestWebKitAPI to ARC.

* TestWebKitAPI/cocoa/DragAndDropSimulator.h:

Add a new hook to clear any external drag information on an existing DragAndDropSimulator. This is convenient
when using the same DragAndDropSimulator to perform multiple drags in a single test.

* TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:
(-[DragAndDropSimulator clearExternalDragInformation]):
* TestWebKitAPI/mac/DragAndDropSimulatorMac.mm:
(-[DragAndDropSimulator clearExternalDragInformation]):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@235392 268f45cc-cd09-0410-ab3c-d52691b4dbfc

22 files changed:
Source/WebCore/ChangeLog
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/Editor.h
Source/WebCore/editing/cocoa/WebContentReaderCocoa.mm
Source/WebCore/html/HTMLAttachmentElement.cpp
Source/WebCore/html/HTMLAttachmentElement.h
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/APIAttachment.cpp
Source/WebKit/UIProcess/API/APIAttachment.h
Source/WebKit/UIProcess/API/Cocoa/APIAttachmentCocoa.mm
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/API/Cocoa/_WKAttachment.mm
Source/WebKit/UIProcess/WebPageProxy.cpp
Source/WebKit/UIProcess/WebPageProxy.h
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm
Tools/TestWebKitAPI/cocoa/DragAndDropSimulator.h
Tools/TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm
Tools/TestWebKitAPI/mac/DragAndDropSimulatorMac.mm

index 871fe59..280f471 100644 (file)
@@ -1,3 +1,45 @@
+2018-08-27  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Cocoa] Exception (fileType 'dyn.agq8u' is not a valid UTI) raised when dragging an attachment whose file wrapper is a directory
+        https://bugs.webkit.org/show_bug.cgi?id=188903
+        <rdar://problem/43702993>
+
+        Reviewed by Tim Horton.
+
+        Fixes the exception for attachments that are created when dropping files with extensions that don't map to any
+        known UTIs, and when dropping folders. See below for more detail.
+
+        Tests:  WKAttachmentTests.InsertFolderAndFileWithUnknownExtension
+                WKAttachmentTests.DropFolderAsAttachmentAndMoveByDragging
+                WKAttachmentTests.ChangeAttachmentDataAndFileInformation
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::insertAttachment):
+        * editing/Editor.h:
+        * editing/cocoa/WebContentReaderCocoa.mm:
+        (WebCore::WebContentReader::readFilePaths):
+
+        When creating an attachment by dropping or pasting a file backed by a file path, handle the cases where…
+        (1)     the dropped path is a directory, by setting the UTI to "public.directory". This allows us to show a
+                folder icon for the dropped attachment element on macOS.
+        (2)     the dropped path is a file whose UTI is unknown, by defaulting to "public.data".
+
+        By ensuring that the UTI of a dropped file-backed attachment is set to a concrete type in any case, we avoid an
+        exception when dragging the attachment on macOS, and on iOS, avoid silently failing to drag an attachment.
+
+        * html/HTMLAttachmentElement.cpp:
+        (WebCore::HTMLAttachmentElement::updateAttributes):
+
+        Change this method to take an optional file size (the subtitle attribute will only be set if the file size is
+        not `std::nullopt`). Furthermore, allow callers of this method to clear attributes on the attachment element by
+        passing in `std::nullopt` for any of the three arguments. This allows us to handle the case where an
+        attachment's file wrapper is changed from a regular file to a folder whose total size is currently unknown.
+        Instead of showing "0 bytes", we'll simply refrain from showing a subtitle at all (in the future, this should
+        be improved by implementing a way to estimate the size of the files in the folder, or perhaps show the number of
+        items in the folder as the subtitle).
+
+        * html/HTMLAttachmentElement.h:
+
 2018-08-27  Devin Rousso  <drousso@apple.com>
 
         Web Inspector: provide autocompletion for event breakpoints
index 176a4a0..4eca915 100644 (file)
@@ -3876,11 +3876,11 @@ void Editor::notifyClientOfAttachmentUpdates()
     }
 }
 
-void Editor::insertAttachment(const String& identifier, const AttachmentDisplayOptions&, uint64_t fileSize, const String& fileName, std::optional<String>&& explicitContentType)
+void Editor::insertAttachment(const String& identifier, const AttachmentDisplayOptions&, std::optional<uint64_t>&& fileSize, const String& fileName, const String& contentType)
 {
     auto attachment = HTMLAttachmentElement::create(HTMLNames::attachmentTag, document());
     attachment->setUniqueIdentifier(identifier);
-    attachment->updateAttributes(fileSize, WTFMove(explicitContentType), fileName);
+    attachment->updateAttributes(WTFMove(fileSize), contentType, fileName);
 
     auto fragmentToInsert = document().createDocumentFragment();
     fragmentToInsert->appendChild(attachment.get());
index 1ec120b..6f5abba 100644 (file)
@@ -503,7 +503,7 @@ public:
     bool isGettingDictionaryPopupInfo() const { return m_isGettingDictionaryPopupInfo; }
 
 #if ENABLE(ATTACHMENT_ELEMENT)
-    WEBCORE_EXPORT void insertAttachment(const String& identifier, const AttachmentDisplayOptions&, uint64_t fileSize, const String& fileName, std::optional<String>&& explicitContentType = std::nullopt);
+    WEBCORE_EXPORT void insertAttachment(const String& identifier, const AttachmentDisplayOptions&, std::optional<uint64_t>&& fileSize, const String& fileName, const String& contentType);
     void registerAttachmentIdentifier(const String&, const String& /* contentType */, const String& /* preferredFileName */, Ref<SharedBuffer>&&);
     void registerAttachmentIdentifier(const String&, const String& /* contentType */, const String& /* filePath */);
     void cloneAttachmentData(const String& fromIdentifier, const String& toIdentifier);
index 6109c38..e885084 100644 (file)
@@ -703,9 +703,18 @@ bool WebContentReader::readFilePaths(const Vector<String>& paths)
         for (auto& path : paths) {
             auto attachment = HTMLAttachmentElement::create(HTMLNames::attachmentTag, document);
             if (supportsClientSideAttachmentData(frame)) {
-                long long fileSize { 0 };
-                FileSystem::getFileSize(path, fileSize);
-                auto contentType = File::contentTypeForFile(path);
+                String contentType;
+                std::optional<uint64_t> fileSizeForDisplay;
+                if (FileSystem::fileIsDirectory(path, FileSystem::ShouldFollowSymbolicLinks::Yes))
+                    contentType = kUTTypeDirectory;
+                else {
+                    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);
                 if (contentTypeIsSuitableForInlineImageRepresentation(contentType)) {
                     auto image = HTMLImageElement::create(document);
@@ -713,7 +722,7 @@ bool WebContentReader::readFilePaths(const Vector<String>& paths)
                     image->setAttachmentElement(WTFMove(attachment));
                     fragment->appendChild(image);
                 } else {
-                    attachment->updateAttributes(fileSize, contentType, FileSystem::pathGetFileName(path));
+                    attachment->updateAttributes(WTFMove(fileSizeForDisplay), contentType, FileSystem::pathGetFileName(path));
                     fragment->appendChild(attachment);
                 }
             } else {
index 405cb3e..3edbc51 100644 (file)
@@ -144,15 +144,23 @@ String HTMLAttachmentElement::attachmentPath() const
     return attributeWithoutSynchronization(webkitattachmentpathAttr);
 }
 
-void HTMLAttachmentElement::updateAttributes(uint64_t fileSize, std::optional<String>&& newContentType, std::optional<String>&& newFilename)
+void HTMLAttachmentElement::updateAttributes(std::optional<uint64_t>&& newFileSize, std::optional<String>&& newContentType, std::optional<String>&& newFilename)
 {
     if (newFilename)
         setAttributeWithoutSynchronization(HTMLNames::titleAttr, *newFilename);
+    else
+        removeAttribute(HTMLNames::titleAttr);
 
     if (newContentType)
         setAttributeWithoutSynchronization(HTMLNames::typeAttr, *newContentType);
+    else
+        removeAttribute(HTMLNames::typeAttr);
+
+    if (newFileSize)
+        setAttributeWithoutSynchronization(HTMLNames::subtitleAttr, fileSizeDescription(*newFileSize));
+    else
+        removeAttribute(HTMLNames::subtitleAttr);
 
-    setAttributeWithoutSynchronization(HTMLNames::subtitleAttr, fileSizeDescription(fileSize));
     if (auto* renderer = this->renderer())
         renderer->invalidate();
 }
index 5b64192..61ea15a 100644 (file)
@@ -49,7 +49,7 @@ public:
     const String& uniqueIdentifier() const { return m_uniqueIdentifier; }
     void setUniqueIdentifier(const String& uniqueIdentifier) { m_uniqueIdentifier = uniqueIdentifier; }
 
-    WEBCORE_EXPORT void updateAttributes(uint64_t fileSize, std::optional<String>&& newContentType = std::nullopt, std::optional<String>&& newFilename = std::nullopt);
+    WEBCORE_EXPORT void updateAttributes(std::optional<uint64_t>&& newFileSize = std::nullopt, std::optional<String>&& newContentType = std::nullopt, std::optional<String>&& newFilename = std::nullopt);
 
     InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
     void removedFromAncestor(RemovalType, ContainerNode&) final;
index 0f868f0..b093366 100644 (file)
@@ -1,3 +1,71 @@
+2018-08-27  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Cocoa] Exception (fileType 'dyn.agq8u' is not a valid UTI) raised when dragging an attachment whose file wrapper is a directory
+        https://bugs.webkit.org/show_bug.cgi?id=188903
+        <rdar://problem/43702993>
+
+        Reviewed by Tim Horton.
+
+        Fixes the bug by supporting NSFileWrappers of type directory, as well as NSFileWrappers with file that do not
+        map to concrete type identifiers. Among other things, this patch ensures that:
+        -       Inserting a directory file wrapper (or using -setFileWrapper:…: to change an existing _WKAttachment's
+                file wrapper to a directory) does not cause the attachment element to show "0 bytes" as the subtitle.
+        -       In the above scenario, we also won't end up with a missing "type" attribute for the attachment element,
+                as well as a corresponding API::Attachment::contentType() that's an empty string.
+        -       Dropping or pasting attachments backed by paths on disk also doesn't trigger these problems, if the path
+                is a directory or of unknown file type.
+
+        Changes are verified by 2 new API tests.
+
+        * UIProcess/API/APIAttachment.cpp:
+        (API::Attachment::updateAttributes):
+        (API::Attachment::fileSizeForDisplay const):
+        * UIProcess/API/APIAttachment.h:
+        * UIProcess/API/Cocoa/APIAttachmentCocoa.mm:
+        (API::Attachment::setFileWrapperAndUpdateContentType):
+
+        Add a helper that sets the file wrapper to the given NSFileWrapper, and either sets the content type to the
+        given content type if it's specified, or infers it from the file extension of the new NSFileWrapper. Invoked
+        whenever an NSFileWrapper and content type combination is set on an API attachment via WebKit SPI.
+
+        (API::Attachment::fileSizeForDisplay const):
+
+        Returns a file size to be displayed in the attachment element's subtitle. This returns an optional file size,
+        where `std::nullopt` indicates that there should not be a file size shown. For now, this returns `std::nullopt`
+        for directory file wrappers, though in the future, this should be done only in cases where we don't immediately
+        have a size estimate for the file wrapper.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _insertAttachmentWithFileWrapper:contentType:options:completion:]):
+
+        Use API::Attachment::setFileWrapperAndUpdateContentType() instead of trying to come up with a fallback UTI.
+
+        * UIProcess/API/Cocoa/_WKAttachment.mm:
+        (-[_WKAttachment setFileWrapper:contentType:completion:]):
+
+        Use API::Attachment::setFileWrapperAndUpdateContentType() instead of trying to come up with a fallback UTI.
+
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::insertAttachment):
+
+        Remove the separate arguments for file size, content type, and file name, and instead get them from the given
+        API Attachment object.
+
+        (WebKit::WebPageProxy::updateAttachmentAttributes):
+
+        Remove separate arguments for file size, content type and file name and just take an API::Attachment instead.
+        These separate pieces of information can simply be asked from the Attachment itself.
+
+        * UIProcess/WebPageProxy.h:
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::insertAttachment):
+        (WebKit::WebPage::updateAttachmentAttributes):
+
+        Adjust some interfaces here to allow the displayed file size to be optional.
+
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/WebPage.messages.in:
+
 2018-08-27  Alex Christensen  <achristensen@webkit.org>
 
         Fix internal builds after r235368
index b615048..073a44c 100644 (file)
@@ -57,17 +57,14 @@ void Attachment::setDisplayOptions(WebCore::AttachmentDisplayOptions options, Fu
         callback(WebKit::CallbackBase::Error::OwnerWasInvalidated);
 }
 
-void Attachment::updateAttributes(uint64_t fileSize, const WTF::String& newContentType, const WTF::String& newFilename, Function<void(WebKit::CallbackBase::Error)>&& callback)
+void Attachment::updateAttributes(Function<void(WebKit::CallbackBase::Error)>&& callback)
 {
-    setContentType(newContentType);
-    setFilePath({ });
-
-    auto optionalNewContentType = newContentType.isNull() ? std::nullopt : std::optional<WTF::String> { newContentType };
-    auto optionalNewFilename = newFilename.isNull() ? std::nullopt : std::optional<WTF::String> { newFilename };
-    if (m_webPage)
-        m_webPage->updateAttachmentAttributes(m_identifier, fileSize, WTFMove(optionalNewContentType), WTFMove(optionalNewFilename), WTFMove(callback));
-    else
+    if (!m_webPage) {
         callback(WebKit::CallbackBase::Error::OwnerWasInvalidated);
+        return;
+    }
+
+    m_webPage->updateAttachmentAttributes(*this, WTFMove(callback));
 }
 
 void Attachment::invalidate()
@@ -93,6 +90,11 @@ WTF::String Attachment::fileName() const
     return { };
 }
 
+std::optional<uint64_t> Attachment::fileSizeForDisplay() const
+{
+    return std::nullopt;
+}
+
 #endif // !PLATFORM(COCOA)
 
 }
index 71741ff..8443f43 100644 (file)
@@ -56,7 +56,7 @@ public:
 
     const WTF::String& identifier() const { return m_identifier; }
     void setDisplayOptions(WebCore::AttachmentDisplayOptions, Function<void(WebKit::CallbackBase::Error)>&&);
-    void updateAttributes(uint64_t fileSize, const WTF::String& newContentType, const WTF::String& newFilename, Function<void(WebKit::CallbackBase::Error)>&&);
+    void updateAttributes(Function<void(WebKit::CallbackBase::Error)>&&);
 
     void invalidate();
     bool isValid() const { return !!m_webPage; }
@@ -64,6 +64,7 @@ public:
 #if PLATFORM(COCOA)
     NSFileWrapper *fileWrapper() const { return m_fileWrapper.get(); }
     void setFileWrapper(NSFileWrapper *fileWrapper) { m_fileWrapper = fileWrapper; }
+    void setFileWrapperAndUpdateContentType(NSFileWrapper *, NSString *contentType);
     WTF::String utiType() const;
 #endif
     WTF::String mimeType() const;
@@ -78,6 +79,8 @@ public:
     InsertionState insertionState() const { return m_insertionState; }
     void setInsertionState(InsertionState state) { m_insertionState = state; }
 
+    std::optional<uint64_t> fileSizeForDisplay() const;
+
 private:
     explicit Attachment(const WTF::String& identifier, WebKit::WebPageProxy&);
 
index 9e8b588..51740d7 100644 (file)
@@ -74,4 +74,34 @@ WTF::String Attachment::fileName() const
     return [m_fileWrapper preferredFilename];
 }
 
+void Attachment::setFileWrapperAndUpdateContentType(NSFileWrapper *fileWrapper, NSString *contentType)
+{
+    if (!contentType.length) {
+        if (fileWrapper.directory)
+            contentType = (NSString *)kUTTypeDirectory;
+        else if (fileWrapper.regularFile) {
+            if (NSString *pathExtension = (fileWrapper.filename.length ? fileWrapper.filename : fileWrapper.preferredFilename).pathExtension)
+                contentType = WebCore::MIMETypeRegistry::getMIMETypeForExtension(pathExtension);
+            if (!contentType.length)
+                contentType = (NSString *)kUTTypeData;
+        }
+    }
+
+    setContentType(contentType);
+    setFileWrapper(fileWrapper);
+}
+
+std::optional<uint64_t> Attachment::fileSizeForDisplay() const
+{
+    if (![m_fileWrapper isRegularFile]) {
+        // FIXME: We should display a size estimate for directory-type file wrappers.
+        return std::nullopt;
+    }
+
+    if (auto fileSize = [[m_fileWrapper fileAttributes][NSFileSize] unsignedLongLongValue])
+        return fileSize;
+
+    return [m_fileWrapper regularFileContents].length;
+}
+
 } // namespace API
index 7b6ccbe..2fa783f 100644 (file)
@@ -4454,18 +4454,8 @@ WEBCORE_COMMAND(yankAndSelect)
     auto identifier = createCanonicalUUIDString();
     auto coreOptions = options ? options.coreDisplayOptions : WebCore::AttachmentDisplayOptions { };
     auto attachment = API::Attachment::create(identifier, *_page);
-    attachment->setFileWrapper(fileWrapper);
-
-    if (!contentType.length) {
-        if (NSString *pathExtension = (fileWrapper.filename.length ? fileWrapper.filename : fileWrapper.preferredFilename).pathExtension)
-            contentType = WebCore::MIMETypeRegistry::getMIMETypeForExtension(pathExtension);
-    }
-
-    auto fileSize = [[[fileWrapper fileAttributes] objectForKey:NSFileSize] unsignedLongLongValue];
-    if (!fileSize && fileWrapper.regularFile)
-        fileSize = fileWrapper.regularFileContents.length;
-
-    _page->insertAttachment(attachment.copyRef(), coreOptions, fileSize, [fileWrapper preferredFilename], contentType.length ? std::optional<String> { contentType } : std::nullopt, [capturedHandler = makeBlockPtr(completionHandler)] (WebKit::CallbackBase::Error error) {
+    attachment->setFileWrapperAndUpdateContentType(fileWrapper, contentType);
+    _page->insertAttachment(attachment.copyRef(), coreOptions, [capturedHandler = makeBlockPtr(completionHandler)] (WebKit::CallbackBase::Error error) {
         if (capturedHandler)
             capturedHandler(error == WebKit::CallbackBase::Error::None);
     });
index 7df48b2..0ca3d53 100644 (file)
@@ -157,17 +157,11 @@ static const NSInteger InvalidAttachmentErrorCode = 2;
         return;
     }
 
-    auto fileSize = [fileWrapper.fileAttributes[NSFileSize] unsignedLongLongValue];
-    if (!fileSize && fileWrapper.regularFile)
-        fileSize = fileWrapper.regularFileContents.length;
-
-    if (!contentType.length) {
-        if (NSString *pathExtension = (fileWrapper.filename.length ? fileWrapper.filename : fileWrapper.preferredFilename).pathExtension)
-            contentType = WebCore::MIMETypeRegistry::getMIMETypeForExtension(pathExtension);
-    }
-
-    _attachment->setFileWrapper(fileWrapper);
-    _attachment->updateAttributes(fileSize, contentType, fileWrapper.preferredFilename, [capturedBlock = makeBlockPtr(completionHandler)] (auto error) {
+    // This file path member is only populated when the attachment is generated upon dropping files. When data is specified via NSFileWrapper
+    // from the SPI client, the corresponding file path of the data is unknown, if it even exists at all.
+    _attachment->setFilePath({ });
+    _attachment->setFileWrapperAndUpdateContentType(fileWrapper, contentType);
+    _attachment->updateAttributes([capturedBlock = makeBlockPtr(completionHandler)] (auto error) {
         if (!capturedBlock)
             return;
 
index bf6cf0f..fa8124c 100644 (file)
@@ -7751,21 +7751,17 @@ RefPtr<API::Attachment> WebPageProxy::attachmentForIdentifier(const String& iden
     return m_attachmentIdentifierToAttachmentMap.get(identifier);
 }
 
-void WebPageProxy::insertAttachment(Ref<API::Attachment>&& attachment, const AttachmentDisplayOptions& options, uint64_t fileSize, const String& filename, std::optional<String> contentType, Function<void(CallbackBase::Error)>&& callback)
+void WebPageProxy::insertAttachment(Ref<API::Attachment>&& attachment, const AttachmentDisplayOptions& options, Function<void(CallbackBase::Error)>&& callback)
 {
     if (!isValid()) {
         callback(CallbackBase::Error::OwnerWasInvalidated);
         return;
     }
 
-    if (contentType)
-        attachment->setContentType(*contentType);
-
     auto attachmentIdentifier = attachment->identifier();
-    m_attachmentIdentifierToAttachmentMap.set(attachmentIdentifier, WTFMove(attachment));
-
     auto callbackID = m_callbacks.put(WTFMove(callback), m_process->throttler().backgroundActivityToken());
-    m_process->send(Messages::WebPage::InsertAttachment(attachmentIdentifier, options, fileSize, filename, contentType, callbackID), m_pageID);
+    m_process->send(Messages::WebPage::InsertAttachment(attachmentIdentifier, options, attachment->fileSizeForDisplay(), attachment->fileName(), attachment->contentType(), callbackID), m_pageID);
+    m_attachmentIdentifierToAttachmentMap.set(attachmentIdentifier, WTFMove(attachment));
 }
 
 void WebPageProxy::setAttachmentDisplayOptions(const String& identifier, AttachmentDisplayOptions options, Function<void(CallbackBase::Error)>&& callback)
@@ -7779,7 +7775,7 @@ void WebPageProxy::setAttachmentDisplayOptions(const String& identifier, Attachm
     m_process->send(Messages::WebPage::SetAttachmentDisplayOptions(identifier, options, callbackID), m_pageID);
 }
 
-void WebPageProxy::updateAttachmentAttributes(const String& identifier, uint64_t fileSize, std::optional<String>&& newContentType, std::optional<String>&& newFilename, Function<void(CallbackBase::Error)>&& callback)
+void WebPageProxy::updateAttachmentAttributes(const API::Attachment& attachment, Function<void(CallbackBase::Error)>&& callback)
 {
     if (!isValid()) {
         callback(CallbackBase::Error::OwnerWasInvalidated);
@@ -7787,7 +7783,9 @@ void WebPageProxy::updateAttachmentAttributes(const String& identifier, uint64_t
     }
 
     auto callbackID = m_callbacks.put(WTFMove(callback), m_process->throttler().backgroundActivityToken());
-    m_process->send(Messages::WebPage::UpdateAttachmentAttributes(identifier, fileSize, WTFMove(newContentType), WTFMove(newFilename), callbackID), m_pageID);
+    auto name = attachment.fileName();
+    auto optionalName = name.isNull() ? std::nullopt : std::optional<WTF::String> { name };
+    m_process->send(Messages::WebPage::UpdateAttachmentAttributes(attachment.identifier(), attachment.fileSizeForDisplay(), attachment.contentType(), WTFMove(optionalName), callbackID), m_pageID);
 }
 
 void WebPageProxy::registerAttachmentIdentifierFromData(const String& identifier, const String& contentType, const String& preferredFileName, const IPC::DataReference& data)
index 77bd8d7..8c55d25 100644 (file)
@@ -1319,9 +1319,9 @@ public:
 
 #if ENABLE(ATTACHMENT_ELEMENT)
     RefPtr<API::Attachment> attachmentForIdentifier(const String& identifier) const;
-    void insertAttachment(Ref<API::Attachment>&&, const WebCore::AttachmentDisplayOptions&, uint64_t fileSize, const String& fileName, std::optional<String> contentType, Function<void(CallbackBase::Error)>&&);
+    void insertAttachment(Ref<API::Attachment>&&, const WebCore::AttachmentDisplayOptions&, Function<void(CallbackBase::Error)>&&);
     void setAttachmentDisplayOptions(const String& identifier, WebCore::AttachmentDisplayOptions, Function<void(CallbackBase::Error)>&&);
-    void updateAttachmentAttributes(const String& identifier, uint64_t fileSize, std::optional<String>&& newContentType, std::optional<String>&& newFilename, Function<void(CallbackBase::Error)>&&);
+    void updateAttachmentAttributes(const API::Attachment&, Function<void(CallbackBase::Error)>&&);
 #endif
 
 #if ENABLE(APPLICATION_MANIFEST)
index b5b711a..4b5f5ab 100644 (file)
@@ -6066,10 +6066,10 @@ void WebPage::storageAccessResponse(bool wasGranted, uint64_t contextId)
 
 #if ENABLE(ATTACHMENT_ELEMENT)
 
-void WebPage::insertAttachment(const String& identifier, const AttachmentDisplayOptions& options, uint64_t fileSize, const String& fileName, std::optional<String> contentType, CallbackID callbackID)
+void WebPage::insertAttachment(const String& identifier, const AttachmentDisplayOptions& options, std::optional<uint64_t>&& fileSize, const String& fileName, const String& contentType, CallbackID callbackID)
 {
     auto& frame = m_page->focusController().focusedOrMainFrame();
-    frame.editor().insertAttachment(identifier, options, fileSize, fileName, WTFMove(contentType));
+    frame.editor().insertAttachment(identifier, options, WTFMove(fileSize), fileName, contentType);
     send(Messages::WebPageProxy::VoidCallback(callbackID));
 }
 
@@ -6078,11 +6078,11 @@ void WebPage::setAttachmentDisplayOptions(const String&, const AttachmentDisplay
     send(Messages::WebPageProxy::VoidCallback(callbackID));
 }
 
-void WebPage::updateAttachmentAttributes(const String& identifier, uint64_t fileSize, std::optional<String> newContentType, std::optional<String> newFilename, CallbackID callbackID)
+void WebPage::updateAttachmentAttributes(const String& identifier, std::optional<uint64_t>&& fileSize, const String& contentType, std::optional<String>&& newFilename, CallbackID callbackID)
 {
     if (auto attachment = attachmentElementWithIdentifier(identifier)) {
         attachment->document().updateLayout();
-        attachment->updateAttributes(fileSize, WTFMove(newContentType), WTFMove(newFilename));
+        attachment->updateAttributes(WTFMove(fileSize), contentType, WTFMove(newFilename));
     }
     send(Messages::WebPageProxy::VoidCallback(callbackID));
 }
index 6be1cbf..4d4ae85 100644 (file)
@@ -1075,9 +1075,9 @@ public:
 #endif
 
 #if ENABLE(ATTACHMENT_ELEMENT)
-    void insertAttachment(const String& identifier, const WebCore::AttachmentDisplayOptions&, uint64_t fileSize, const String& fileName, std::optional<String> contentType, CallbackID);
+    void insertAttachment(const String& identifier, const WebCore::AttachmentDisplayOptions&, std::optional<uint64_t>&& fileSize, const String& fileName, const String& contentType, CallbackID);
     void setAttachmentDisplayOptions(const String& identifier, const WebCore::AttachmentDisplayOptions&, CallbackID);
-    void updateAttachmentAttributes(const String& identifier, uint64_t fileSize, std::optional<String> newContentType, std::optional<String> newFilename, CallbackID);
+    void updateAttachmentAttributes(const String& identifier, std::optional<uint64_t>&& fileSize, const String& contentType, std::optional<String>&& newFilename, CallbackID);
 #endif
 
 #if ENABLE(APPLICATION_MANIFEST)
index 96e487e..289abda 100644 (file)
@@ -510,9 +510,9 @@ messages -> WebPage LegacyReceiver {
 #endif
 
 #if ENABLE(ATTACHMENT_ELEMENT)
-    InsertAttachment(String identifier, struct WebCore::AttachmentDisplayOptions options, uint64_t fileSize, String fileName, std::optional<String> contentType, WebKit::CallbackID callbackID)
+    InsertAttachment(String identifier, struct WebCore::AttachmentDisplayOptions options, std::optional<uint64_t> fileSize, String fileName, String contentType, WebKit::CallbackID callbackID)
     SetAttachmentDisplayOptions(String identifier, struct WebCore::AttachmentDisplayOptions options, WebKit::CallbackID callbackID)
-    UpdateAttachmentAttributes(String identifier, uint64_t fileSize, std::optional<String> newContentType, std::optional<String> newFilename, WebKit::CallbackID callbackID)
+    UpdateAttachmentAttributes(String identifier, std::optional<uint64_t> fileSize, String newContentType, std::optional<String> newFilename, WebKit::CallbackID callbackID)
 #endif
 
 #if ENABLE(APPLICATION_MANIFEST)
index 96c6468..6cb8d2b 100644 (file)
@@ -1,3 +1,51 @@
+2018-08-27  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [Cocoa] Exception (fileType 'dyn.agq8u' is not a valid UTI) raised when dragging an attachment whose file wrapper is a directory
+        https://bugs.webkit.org/show_bug.cgi?id=188903
+        <rdar://problem/43702993>
+
+        Reviewed by Tim Horton.
+
+        Add two API tests and adjust existing WKAttachment API tests. The new tests exercise the following scenarios, in
+        both iOS and macOS:
+        •       Dropping a folder as an attachment element, and then moving that attachment element in the document by
+                dragging and dropping.
+        •       Using WKWebView SPI to insert a folder and a file with an unknown extension, and then using more
+                _WKAttachment SPI to swap the attachments' backing file wrappers.
+
+        * TestWebKitAPI/Tests/WebKitCocoa/WKAttachmentTests.mm:
+        (runTestWithTemporaryFolder):
+
+        Add a helper function to run a test with a new folder path, created in the temporary directory and populated
+        with some sample content. This folder is deleted after running the test.
+
+        (simulateFolderDragWithURL):
+
+        Add a helper function to prepare a given DragAndDropSimulator for simulating a dragged folder from a source
+        external to the web view.
+
+        (platformCopyRichTextWithMultipleAttachments):
+        (platformCopyRichTextWithImage):
+        (platformCopyPNG):
+        (TestWebKitAPI::TEST):
+
+        Add new API tests, and adjust existing tests to reflect new -setFileWrapper:…: behavior. Specifically,
+        ChangeAttachmentDataAndFileInformation previously required that changing a _WKAttachment's NSFileWrapper would
+        preserve the previous NSFileWrapper's preferred name if the new file wrapper does not have a preferred name, but
+        this quirk is no longer supported.
+
+        Also add a few bridging casts for the eventual transition of TestWebKitAPI to ARC.
+
+        * TestWebKitAPI/cocoa/DragAndDropSimulator.h:
+
+        Add a new hook to clear any external drag information on an existing DragAndDropSimulator. This is convenient
+        when using the same DragAndDropSimulator to perform multiple drags in a single test.
+
+        * TestWebKitAPI/ios/DragAndDropSimulatorIOS.mm:
+        (-[DragAndDropSimulator clearExternalDragInformation]):
+        * TestWebKitAPI/mac/DragAndDropSimulatorMac.mm:
+        (-[DragAndDropSimulator clearExternalDragInformation]):
+
 2018-08-27  Alex Christensen  <achristensen@webkit.org>
 
         Translate 4 tests using WKPageLoaderClient to ObjC
index 3f1e7e6..0a0103b 100644 (file)
@@ -408,6 +408,37 @@ static NSData *testPDFData()
 
 @end
 
+static void runTestWithTemporaryFolder(void(^runTest)(NSURL *folderURL))
+{
+    NSFileManager *defaultManager = [NSFileManager defaultManager];
+    auto temporaryFolder = retainPtr([NSURL fileURLWithPath:[NSTemporaryDirectory() stringByAppendingPathComponent:[NSString stringWithFormat:@"folder-%@", NSUUID.UUID]] isDirectory:YES]);
+    [defaultManager removeItemAtURL:temporaryFolder.get() error:nil];
+    [defaultManager createDirectoryAtURL:temporaryFolder.get() withIntermediateDirectories:NO attributes:nil error:nil];
+    [testImageData() writeToURL:[temporaryFolder.get() URLByAppendingPathComponent:@"image.png" isDirectory:NO] atomically:YES];
+    [testZIPData() writeToURL:[temporaryFolder.get() URLByAppendingPathComponent:@"archive.zip" isDirectory:NO] atomically:YES];
+    @try {
+        runTest(temporaryFolder.get());
+    } @finally {
+        [[NSFileManager defaultManager] removeItemAtURL:temporaryFolder.get() error:nil];
+    }
+}
+
+static void simulateFolderDragWithURL(DragAndDropSimulator *simulator, NSURL *folderURL)
+{
+#if PLATFORM(MAC)
+    [simulator writePromisedFiles:@[ folderURL ]];
+#else
+    auto folderProvider = adoptNS([[NSItemProvider alloc] init]);
+    [folderProvider setSuggestedName:folderURL.lastPathComponent];
+    [folderProvider setPreferredPresentationStyle:UIPreferredPresentationStyleAttachment];
+    [folderProvider registerFileRepresentationForTypeIdentifier:(__bridge NSString *)kUTTypeFolder fileOptions:0 visibility:NSItemProviderRepresentationVisibilityAll loadHandler:[&] (void(^completion)(NSURL *, BOOL, NSError *)) -> NSProgress * {
+        completion(folderURL, NO, nil);
+        return nil;
+    }];
+    simulator.externalItemProviders = @[ folderProvider.get() ];
+#endif
+}
+
 #pragma mark - Platform testing helper functions
 
 #if PLATFORM(MAC)
@@ -468,9 +499,9 @@ typedef void(^ItemProviderDataLoadHandler)(NSData *, NSError *);
 
 void platformCopyRichTextWithMultipleAttachments()
 {
-    auto image = adoptNS([[NSTextAttachment alloc] initWithData:testImageData() ofType:(NSString *)kUTTypePNG]);
-    auto pdf = adoptNS([[NSTextAttachment alloc] initWithData:testPDFData() ofType:(NSString *)kUTTypePDF]);
-    auto zip = adoptNS([[NSTextAttachment alloc] initWithData:testZIPData() ofType:(NSString *)kUTTypeZipArchive]);
+    auto image = adoptNS([[NSTextAttachment alloc] initWithData:testImageData() ofType:(__bridge NSString *)kUTTypePNG]);
+    auto pdf = adoptNS([[NSTextAttachment alloc] initWithData:testPDFData() ofType:(__bridge NSString *)kUTTypePDF]);
+    auto zip = adoptNS([[NSTextAttachment alloc] initWithData:testZIPData() ofType:(__bridge NSString *)kUTTypeZipArchive]);
 
     auto richText = adoptNS([[NSMutableAttributedString alloc] init]);
     [richText appendAttributedString:[NSAttributedString attributedStringWithAttachment:image.get()]];
@@ -491,7 +522,7 @@ void platformCopyRichTextWithMultipleAttachments()
 void platformCopyRichTextWithImage()
 {
     auto richText = adoptNS([[NSMutableAttributedString alloc] init]);
-    auto image = adoptNS([[NSTextAttachment alloc] initWithData:testImageData() ofType:(NSString *)kUTTypePNG]);
+    auto image = adoptNS([[NSTextAttachment alloc] initWithData:testImageData() ofType:(__bridge NSString *)kUTTypePNG]);
 
     [richText appendAttributedString:[[[NSAttributedString alloc] initWithString:@"Lorem ipsum "] autorelease]];
     [richText appendAttributedString:[NSAttributedString attributedStringWithAttachment:image.get()]];
@@ -518,7 +549,7 @@ void platformCopyPNG()
     UIPasteboard *pasteboard = [UIPasteboard generalPasteboard];
     auto item = adoptNS([[UIItemProvider alloc] init]);
     [item setPreferredPresentationStyle:UIPreferredPresentationStyleAttachment];
-    [item registerData:testImageData() type:(NSString *)kUTTypePNG];
+    [item registerData:testImageData() type:(__bridge NSString *)kUTTypePNG];
     pasteboard.itemProviders = @[ item.get() ];
 #endif
 }
@@ -641,7 +672,7 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenChangingFontStyles)
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
         attachment = [webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData()];
-        observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
+        observer.expectAttachmentUpdates(@[ ], @[ attachment.get() ]);
     }
     [webView expectUpdatesAfterCommand:@"InsertText" withArgument:@"World" expectedRemovals:@[] expectedInsertions:@[]];
     [webView _synchronouslyExecuteEditCommand:@"SelectAll" argument:nil];
@@ -665,7 +696,7 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingLists)
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
         attachment = [webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData()];
-        observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
+        observer.expectAttachmentUpdates(@[ ], @[ attachment.get() ]);
     }
     [webView expectUpdatesAfterCommand:@"InsertOrderedList" withArgument:nil expectedRemovals:@[] expectedInsertions:@[]];
     // This edit command behaves more like a "toggle", and will actually break us out of the list we just inserted.
@@ -686,7 +717,7 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingRichMarkup)
         ObserveAttachmentUpdatesForScope observer(webView.get());
         [webView _synchronouslyExecuteEditCommand:@"InsertHTML" argument:@"<div><strong><attachment src='cid:123-4567' title='a'></attachment></strong></div>"];
         attachment = observer.observer().inserted[0];
-        observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
+        observer.expectAttachmentUpdates(@[ ], @[ attachment.get() ]);
         observer.expectSourceForIdentifier(@"cid:123-4567", [attachment uniqueIdentifier]);
     }
     EXPECT_FALSE([webView hasAttribute:@"webkitattachmentbloburl" forQuerySelector:@"attachment"]);
@@ -697,7 +728,7 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenInsertingRichMarkup)
         ObserveAttachmentUpdatesForScope observer(webView.get());
         [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment').remove()"];
         [webView waitForNextPresentationUpdate];
-        observer.expectAttachmentUpdates(@[attachment.get()], @[ ]);
+        observer.expectAttachmentUpdates(@[ attachment.get() ], @[ ]);
     }
     [attachment expectRequestedDataToBe:nil];
 }
@@ -709,7 +740,7 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenCuttingAndPasting)
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
         attachment = [webView synchronouslyInsertAttachmentWithFilename:@"foo.txt" contentType:@"text/plain" data:testHTMLData()];
-        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+        observer.expectAttachmentUpdates(@[ ], @[ attachment.get() ]);
     }
     [attachment expectRequestedDataToBe:testHTMLData()];
     EXPECT_WK_STREQ([attachment uniqueIdentifier], [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment').uniqueIdentifier"]);
@@ -717,13 +748,13 @@ TEST(WKAttachmentTests, AttachmentUpdatesWhenCuttingAndPasting)
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
         [webView _synchronouslyExecuteEditCommand:@"Cut" argument:nil];
-        observer.expectAttachmentUpdates(@[attachment.get()], @[]);
+        observer.expectAttachmentUpdates(@[ attachment.get() ], @[ ]);
     }
     [attachment expectRequestedDataToBe:testHTMLData()];
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
         [webView _synchronouslyExecuteEditCommand:@"Paste" argument:nil];
-        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+        observer.expectAttachmentUpdates(@[ ], @[ attachment.get() ]);
     }
     [attachment expectRequestedDataToBe:testHTMLData()];
     EXPECT_FALSE([webView hasAttribute:@"webkitattachmentbloburl" forQuerySelector:@"attachment"]);
@@ -738,18 +769,99 @@ TEST(WKAttachmentTests, AttachmentDataForEmptyFile)
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
         attachment = [webView synchronouslyInsertAttachmentWithFilename:@"empty.txt" contentType:@"text/plain" data:[NSData data]];
-        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+        observer.expectAttachmentUpdates(@[ ], @[ attachment.get() ]);
     }
     [attachment expectRequestedDataToBe:[NSData data]];
     EXPECT_WK_STREQ([attachment uniqueIdentifier], [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment').uniqueIdentifier"]);
     {
         ObserveAttachmentUpdatesForScope scope(webView.get());
         [webView _synchronouslyExecuteEditCommand:@"DeleteBackward" argument:nil];
-        scope.expectAttachmentUpdates(@[attachment.get()], @[]);
+        scope.expectAttachmentUpdates(@[ attachment.get() ], @[ ]);
     }
     [attachment expectRequestedDataToBe:[NSData data]];
 }
 
+TEST(WKAttachmentTests, DropFolderAsAttachmentAndMoveByDragging)
+{
+    auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
+    [configuration _setAttachmentElementEnabled:YES];
+
+    auto simulator = adoptNS([[DragAndDropSimulator alloc] initWithWebViewFrame:NSMakeRect(0, 0, 400, 400) configuration:configuration.get()]);
+    [[simulator webView] synchronouslyLoadHTMLString:attachmentEditingTestMarkup];
+
+    runTestWithTemporaryFolder([simulator] (NSURL *folderURL) {
+        simulateFolderDragWithURL(simulator.get(), folderURL);
+        [simulator runFrom:CGPointMake(0, 0) to:CGPointMake(50, 50)];
+
+        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(folderURL.lastPathComponent, [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
+
+        NSFileWrapper *image = [attachment info].fileWrapper.fileWrappers[@"image.png"];
+        NSFileWrapper *archive = [attachment info].fileWrapper.fileWrappers[@"archive.zip"];
+        EXPECT_TRUE([image.regularFileContents isEqualToData:testImageData()]);
+        EXPECT_TRUE([archive.regularFileContents isEqualToData:testZIPData()]);
+
+        [webView evaluateJavaScript:@"getSelection().collapseToEnd()" completionHandler:nil];
+        [webView _executeEditCommand:@"InsertParagraph" argument:nil completion:nil];
+        [webView _executeEditCommand:@"InsertHTML" argument:@"<em>foo</em>" completion:nil];
+        [webView _executeEditCommand:@"InsertParagraph" argument:nil completion:nil];
+
+        [webView expectElementTag:@"ATTACHMENT" toComeBefore:@"EM"];
+        [simulator clearExternalDragInformation];
+        [simulator runFrom:webView.attachmentElementMidPoint to:CGPointMake(300, 300)];
+        [webView expectElementTag:@"EM" toComeBefore:@"ATTACHMENT"];
+    });
+}
+
+TEST(WKAttachmentTests, InsertFolderAndFileWithUnknownExtension)
+{
+    auto webView = webViewForTestingAttachments();
+    auto file = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:testHTMLData()]);
+    [file setPreferredFilename:@"test.foobar"];
+    auto image = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:testImageData()]);
+    auto document = adoptNS([[NSFileWrapper alloc] initRegularFileWithContents:testPDFData()]);
+    auto folder = adoptNS([[NSFileWrapper alloc] initDirectoryWithFileWrappers:@{ @"image.png": image.get(), @"document.pdf": document.get() }]);
+    [folder setPreferredFilename:@"folder"];
+
+    RetainPtr<_WKAttachment> firstAttachment;
+    RetainPtr<_WKAttachment> secondAttachment;
+    {
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        firstAttachment = [webView synchronouslyInsertAttachmentWithFileWrapper:file.get() contentType:nil];
+        observer.expectAttachmentUpdates(@[ ], @[ firstAttachment.get() ]);
+    }
+    {
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        secondAttachment = [webView synchronouslyInsertAttachmentWithFileWrapper:folder.get() contentType:nil];
+        observer.expectAttachmentUpdates(@[ ], @[ secondAttachment.get() ]);
+    }
+
+    auto checkAttachmentConsistency = [webView, file, folder] (_WKAttachment *expectedFileAttachment, _WKAttachment *expectedFolderAttachment) {
+        [webView expectElementCount:2 tagName:@"ATTACHMENT"];
+        EXPECT_TRUE(UTTypeConformsTo((__bridge CFStringRef)[webView valueOfAttribute:@"type" forQuerySelector:@"attachment[title=folder]"], kUTTypeDirectory));
+        EXPECT_TRUE(UTTypeConformsTo((__bridge CFStringRef)[webView valueOfAttribute:@"type" forQuerySelector:@"attachment[title^=test]"], kUTTypeData));
+        EXPECT_WK_STREQ(expectedFileAttachment.uniqueIdentifier, [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment[title^=test]').uniqueIdentifier"]);
+        EXPECT_WK_STREQ(expectedFolderAttachment.uniqueIdentifier, [webView stringByEvaluatingJavaScript:@"document.querySelector('attachment[title=folder]').uniqueIdentifier"]);
+        EXPECT_TRUE([expectedFileAttachment.info.fileWrapper isEqual:file.get()]);
+        EXPECT_TRUE([expectedFolderAttachment.info.fileWrapper isEqual:folder.get()]);
+    };
+
+    checkAttachmentConsistency(firstAttachment.get(), secondAttachment.get());
+
+    {
+        // Swap the two attachments' file wrappers without creating or destroying attachment elements.
+        ObserveAttachmentUpdatesForScope observer(webView.get());
+        [firstAttachment synchronouslySetFileWrapper:folder.get() newContentType:nil error:nil];
+        [secondAttachment synchronouslySetFileWrapper:file.get() newContentType:nil error:nil];
+        observer.expectAttachmentUpdates(@[ ], @[ ]);
+    }
+
+    checkAttachmentConsistency(secondAttachment.get(), firstAttachment.get());
+}
+
 TEST(WKAttachmentTests, ChangeAttachmentDataAndFileInformation)
 {
     auto webView = webViewForTestingAttachments();
@@ -761,7 +873,7 @@ TEST(WKAttachmentTests, ChangeAttachmentDataAndFileInformation)
         [attachment expectRequestedDataToBe:pdfData.get()];
         EXPECT_WK_STREQ(@"test.pdf", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
         EXPECT_WK_STREQ(@"application/pdf", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
-        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+        observer.expectAttachmentUpdates(@[ ], @[attachment.get()]);
     }
     {
         RetainPtr<NSData> imageData = testImageData();
@@ -770,29 +882,28 @@ TEST(WKAttachmentTests, ChangeAttachmentDataAndFileInformation)
         [attachment expectRequestedDataToBe:imageData.get()];
         EXPECT_WK_STREQ(@"icon.png", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
         EXPECT_WK_STREQ(@"image/png", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
-        observer.expectAttachmentUpdates(@[], @[]);
+        observer.expectAttachmentUpdates(@[ ], @[ ]);
     }
     {
-        RetainPtr<NSData> textData = testHTMLData();
+        RetainPtr<NSData> textData = [@"Hello world" dataUsingEncoding:NSUTF8StringEncoding];
         ObserveAttachmentUpdatesForScope observer(webView.get());
         // The new content type should be inferred from the file name.
         [attachment synchronouslySetData:textData.get() newContentType:nil newFilename:@"foo.txt" error:nil];
         [attachment expectRequestedDataToBe:textData.get()];
         EXPECT_WK_STREQ(@"foo.txt", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
         EXPECT_WK_STREQ(@"text/plain", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
-        observer.expectAttachmentUpdates(@[], @[]);
+        observer.expectAttachmentUpdates(@[ ], @[ ]);
     }
     {
-        RetainPtr<NSData> secondTextData = [@"Hello world" dataUsingEncoding:NSUTF8StringEncoding];
+        RetainPtr<NSData> htmlData = testHTMLData();
         ObserveAttachmentUpdatesForScope observer(webView.get());
-        // Both the previous file name and type should be inferred.
-        [attachment synchronouslySetData:secondTextData.get() newContentType:nil newFilename:nil error:nil];
-        [attachment expectRequestedDataToBe:secondTextData.get()];
-        EXPECT_WK_STREQ(@"foo.txt", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
-        EXPECT_WK_STREQ(@"text/plain", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
-        observer.expectAttachmentUpdates(@[], @[]);
+        [attachment synchronouslySetData:htmlData.get() newContentType:@"text/html" newFilename:@"bar" error:nil];
+        [attachment expectRequestedDataToBe:htmlData.get()];
+        EXPECT_WK_STREQ(@"bar", [webView valueOfAttribute:@"title" forQuerySelector:@"attachment"]);
+        EXPECT_WK_STREQ(@"text/html", [webView valueOfAttribute:@"type" forQuerySelector:@"attachment"]);
+        observer.expectAttachmentUpdates(@[ ], @[ ]);
     }
-    [webView expectUpdatesAfterCommand:@"DeleteBackward" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[]];
+    [webView expectUpdatesAfterCommand:@"DeleteBackward" withArgument:nil expectedRemovals:@[attachment.get()] expectedInsertions:@[ ]];
 }
 
 TEST(WKAttachmentTests, RemoveNewlinesBeforePastedImage)
@@ -903,7 +1014,7 @@ TEST(WKAttachmentTests, InsertPastedAttributedStringContainingImage)
         ObserveAttachmentUpdatesForScope observer(webView.get());
         [webView _synchronouslyExecuteEditCommand:@"SelectAll" argument:nil];
         [webView _synchronouslyExecuteEditCommand:@"DeleteBackward" argument:nil];
-        observer.expectAttachmentUpdates(@[attachment.get()], @[]);
+        observer.expectAttachmentUpdates(@[ attachment.get() ], @[ ]);
     }
 }
 
@@ -989,7 +1100,7 @@ TEST(WKAttachmentTests, InsertAndRemoveDuplicateAttachment)
         ObserveAttachmentUpdatesForScope observer(webView.get());
         originalAttachment = [webView synchronouslyInsertAttachmentWithFileWrapper:fileWrapper.get() contentType:@"text/plain"];
         EXPECT_EQ(0U, observer.observer().removed.count);
-        observer.expectAttachmentUpdates(@[], @[originalAttachment.get()]);
+        observer.expectAttachmentUpdates(@[ ], @[ originalAttachment.get() ]);
     }
     [webView selectAll:nil];
     [webView _executeEditCommand:@"Copy" argument:nil completion:nil];
@@ -1005,13 +1116,13 @@ TEST(WKAttachmentTests, InsertAndRemoveDuplicateAttachment)
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
         [webView _synchronouslyExecuteEditCommand:@"DeleteBackward" argument:nil];
-        observer.expectAttachmentUpdates(@[pastedAttachment.get()], @[]);
+        observer.expectAttachmentUpdates(@[ pastedAttachment.get() ], @[ ]);
         [originalAttachment expectRequestedDataToBe:data.get()];
     }
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
         [webView _synchronouslyExecuteEditCommand:@"DeleteBackward" argument:nil];
-        observer.expectAttachmentUpdates(@[originalAttachment.get()], @[]);
+        observer.expectAttachmentUpdates(@[ originalAttachment.get() ], @[ ]);
     }
 
     EXPECT_FALSE([originalAttachment isEqual:pastedAttachment.get()]);
@@ -1030,7 +1141,7 @@ TEST(WKAttachmentTests, InsertDuplicateAttachmentAndUpdateData)
         ObserveAttachmentUpdatesForScope observer(webView.get());
         originalAttachment = [webView synchronouslyInsertAttachmentWithFileWrapper:fileWrapper.get() contentType:@"text/plain"];
         EXPECT_EQ(0U, observer.observer().removed.count);
-        observer.expectAttachmentUpdates(@[], @[originalAttachment.get()]);
+        observer.expectAttachmentUpdates(@[ ], @[ originalAttachment.get() ]);
     }
     [webView selectAll:nil];
     [webView _executeEditCommand:@"Copy" argument:nil completion:nil];
@@ -1219,7 +1330,7 @@ TEST(WKAttachmentTests, PasteWebArchiveContainingImages)
     auto mainResource = adoptNS([[WebResource alloc] initWithData:markupData URL:[NSURL URLWithString:@"foo.html"] MIMEType:@"text/html" textEncodingName:@"utf-8" frameName:nil]);
     auto pngResource = adoptNS([[WebResource alloc] initWithData:testImageData() URL:[NSURL URLWithString:@"1.png"] MIMEType:@"image/png" textEncodingName:nil frameName:nil]);
     auto gifResource = adoptNS([[WebResource alloc] initWithData:testGIFData() URL:[NSURL URLWithString:@"2.gif"] MIMEType:@"image/gif" textEncodingName:nil frameName:nil]);
-    auto archive = adoptNS([[WebArchive alloc] initWithMainResource:(__bridge WebResource *)mainResource.get() subresources:@[ (__bridge WebResource *)pngResource.get(), (__bridge WebResource *)gifResource.get() ] subframeArchives:@[ ]]);
+    auto archive = adoptNS([[WebArchive alloc] initWithMainResource:mainResource.get() subresources:@[ pngResource.get(), gifResource.get() ] subframeArchives:@[ ]]);
 
 #if PLATFORM(MAC)
     NSPasteboard *pasteboard = [NSPasteboard generalPasteboard];
@@ -1234,7 +1345,7 @@ TEST(WKAttachmentTests, PasteWebArchiveContainingImages)
     RetainPtr<_WKAttachment> pngAttachment;
     auto webView = webViewForTestingAttachments();
 
-    ObserveAttachmentUpdatesForScope observer((__bridge TestWKWebView *)webView.get());
+    ObserveAttachmentUpdatesForScope observer(webView.get());
     [webView _synchronouslyExecuteEditCommand:@"Paste" argument:nil];
     [webView expectElementCount:2 tagName:@"IMG"];
 
@@ -1249,7 +1360,7 @@ TEST(WKAttachmentTests, PasteWebArchiveContainingImages)
     EXPECT_WK_STREQ("bar", [gifAttachment info].name);
     [pngAttachment expectRequestedDataToBe:testImageData()];
     [gifAttachment expectRequestedDataToBe:testGIFData()];
-    observer.expectAttachmentUpdates(@[ ], @[ (__bridge _WKAttachment *)pngAttachment.get(), (__bridge _WKAttachment *)gifAttachment.get() ]);
+    observer.expectAttachmentUpdates(@[ ], @[ pngAttachment.get(), gifAttachment.get() ]);
 }
 
 #pragma mark - Platform-specific tests
@@ -1363,7 +1474,7 @@ TEST(WKAttachmentTestsIOS, InsertDroppedImageAsAttachment)
     auto webView = webViewForTestingAttachments();
     auto dragAndDropSimulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
     auto item = adoptNS([[NSItemProvider alloc] init]);
-    [item registerData:testImageData() type:(NSString *)kUTTypePNG];
+    [item registerData:testImageData() type:(__bridge NSString *)kUTTypePNG];
     [dragAndDropSimulator setExternalItemProviders:@[ item.get() ]];
     [dragAndDropSimulator runFrom:CGPointZero to:CGPointMake(50, 50)];
 
@@ -1377,7 +1488,7 @@ TEST(WKAttachmentTestsIOS, InsertDroppedImageAsAttachment)
         ObserveAttachmentUpdatesForScope observer(webView.get());
         [webView _synchronouslyExecuteEditCommand:@"SelectAll" argument:nil];
         [webView _synchronouslyExecuteEditCommand:@"DeleteBackward" argument:nil];
-        observer.expectAttachmentUpdates(@[attachment.get()], @[]);
+        observer.expectAttachmentUpdates(@[ attachment.get() ], @[ ]);
     }
 }
 
@@ -1385,7 +1496,7 @@ TEST(WKAttachmentTestsIOS, InsertDroppedAttributedStringContainingAttachment)
 {
     auto webView = webViewForTestingAttachments();
     auto dragAndDropSimulator = adoptNS([[DragAndDropSimulator alloc] initWithWebView:webView.get()]);
-    auto image = adoptNS([[NSTextAttachment alloc] initWithData:testImageData() ofType:(NSString *)kUTTypePNG]);
+    auto image = adoptNS([[NSTextAttachment alloc] initWithData:testImageData() ofType:(__bridge NSString *)kUTTypePNG]);
     auto item = adoptNS([[NSItemProvider alloc] init]);
     [item registerObject:[NSAttributedString attributedStringWithAttachment:image.get()] visibility:NSItemProviderRepresentationVisibilityAll];
 
@@ -1405,7 +1516,7 @@ TEST(WKAttachmentTestsIOS, InsertDroppedAttributedStringContainingAttachment)
         ObserveAttachmentUpdatesForScope observer(webView.get());
         [webView _synchronouslyExecuteEditCommand:@"SelectAll" argument:nil];
         [webView _synchronouslyExecuteEditCommand:@"DeleteBackward" argument:nil];
-        observer.expectAttachmentUpdates(@[attachment.get()], @[]);
+        observer.expectAttachmentUpdates(@[ attachment.get() ], @[ ]);
     }
 }
 
@@ -1451,7 +1562,7 @@ TEST(WKAttachmentTestsIOS, InsertDroppedZipArchiveAsAttachment)
     // presentation style (e.g. Notes) into Mail.
     auto item = adoptNS([[NSItemProvider alloc] init]);
     NSData *data = testZIPData();
-    [item registerData:data type:(NSString *)kUTTypeZipArchive];
+    [item registerData:data type:(__bridge NSString *)kUTTypeZipArchive];
     [item setSuggestedName:@"archive.zip"];
 
     auto webView = webViewForTestingAttachments();
@@ -1481,7 +1592,7 @@ TEST(WKAttachmentTestsIOS, InsertDroppedItemProvidersInOrder)
     [inlineTextItem registerObject:appleURL.get() visibility:NSItemProviderRepresentationVisibilityAll];
 
     auto secondAttachmentItem = adoptNS([[NSItemProvider alloc] init]);
-    [secondAttachmentItem registerData:testPDFData() type:(NSString *)kUTTypePDF];
+    [secondAttachmentItem registerData:testPDFData() type:(__bridge NSString *)kUTTypePDF];
     [secondAttachmentItem setSuggestedName:@"second.pdf"];
 
     auto webView = webViewForTestingAttachments();
@@ -1508,7 +1619,7 @@ TEST(WKAttachmentTestsIOS, DragAttachmentInsertedAsFile)
 {
     auto item = adoptNS([[NSItemProvider alloc] init]);
     auto data = retainPtr(testPDFData());
-    [item registerData:data.get() type:(NSString *)kUTTypePDF];
+    [item registerData:data.get() type:(__bridge NSString *)kUTTypePDF];
     [item setSuggestedName:@"document.pdf"];
 
     auto webView = webViewForTestingAttachments();
@@ -1531,7 +1642,7 @@ TEST(WKAttachmentTestsIOS, DragAttachmentInsertedAsFile)
     EXPECT_EQ(1U, [dragAndDropSimulator sourceItemProviders].count);
     NSItemProvider *itemProvider = [dragAndDropSimulator sourceItemProviders].firstObject;
     EXPECT_EQ(UIPreferredPresentationStyleAttachment, itemProvider.preferredPresentationStyle);
-    [itemProvider expectType:(NSString *)kUTTypePDF withData:data.get()];
+    [itemProvider expectType:(__bridge NSString *)kUTTypePDF withData:data.get()];
     EXPECT_WK_STREQ("document.pdf", [itemProvider suggestedName]);
     [dragAndDropSimulator endDataTransfer];
 }
@@ -1544,7 +1655,7 @@ TEST(WKAttachmentTestsIOS, DragAttachmentInsertedAsData)
     {
         ObserveAttachmentUpdatesForScope observer(webView.get());
         attachment = [webView synchronouslyInsertAttachmentWithFilename:@"document.pdf" contentType:@"application/pdf" data:data.get()];
-        observer.expectAttachmentUpdates(@[], @[attachment.get()]);
+        observer.expectAttachmentUpdates(@[ ], @[ attachment.get() ]);
     }
 
     // First, verify that the attachment was successfully inserted from raw data.
@@ -1560,7 +1671,7 @@ TEST(WKAttachmentTestsIOS, DragAttachmentInsertedAsData)
     EXPECT_EQ(1U, [dragAndDropSimulator sourceItemProviders].count);
     NSItemProvider *itemProvider = [dragAndDropSimulator sourceItemProviders].firstObject;
     EXPECT_EQ(UIPreferredPresentationStyleAttachment, itemProvider.preferredPresentationStyle);
-    [itemProvider expectType:(NSString *)kUTTypePDF withData:data.get()];
+    [itemProvider expectType:(__bridge NSString *)kUTTypePDF withData:data.get()];
     EXPECT_WK_STREQ("document.pdf", [itemProvider suggestedName]);
     [dragAndDropSimulator endDataTransfer];
 }
index 761a36f..1d7e10e 100644 (file)
@@ -80,6 +80,7 @@ typedef NSDictionary<NSNumber *, NSValue *> *ProgressToCGPointValueMap;
 // The start location, end location, and locations of additional item requests are all in window coordinates.
 - (void)runFrom:(CGPoint)startLocation to:(CGPoint)endLocation;
 - (void)endDataTransfer;
+- (void)clearExternalDragInformation;
 @property (nonatomic, readonly) NSArray<_WKAttachment *> *insertedAttachments;
 @property (nonatomic, readonly) NSArray<_WKAttachment *> *removedAttachments;
 @property (nonatomic, readonly) TestWKWebView *webView;
index 0f97a39..cebd460 100644 (file)
@@ -568,6 +568,11 @@ static NSArray *dragAndDropEventNames()
     [self _scheduleAdvanceProgress];
 }
 
+- (void)clearExternalDragInformation
+{
+    _externalItemProviders = nil;
+}
+
 - (CGPoint)_currentLocation
 {
     CGFloat distanceX = _endLocation.x - _startLocation.x;
index e528301..2efbb5c 100644 (file)
@@ -358,6 +358,13 @@ static NSImage *defaultExternalDragImage()
     return _externalPromisedFiles.get();
 }
 
+- (void)clearExternalDragInformation
+{
+    _externalPromisedFiles = nil;
+    _externalDragImage = nil;
+    _externalDragPasteboard = nil;
+}
+
 static BOOL getFilePathsAndTypeIdentifiers(NSArray<NSURL *> *fileURLs, NSArray<NSString *> **outFilePaths, NSArray<NSString *> **outTypeIdentifiers)
 {
     NSMutableArray *filePaths = [NSMutableArray arrayWithCapacity:fileURLs.count];