Add basic support for the version of DataTransferItemList.add that takes a File
https://bugs.webkit.org/show_bug.cgi?id=177853
<rdar://problem/34807346>
Reviewed by Ryosuke Niwa.
Source/WebCore:
Adds very basic support for DataTransferItemList.add(File). So far, a File added in this way can only be read
back from the same DataTransfer, during dragstart or copy. This File isn't written to the platform pasteboard
yet, so even dropping or pasting in the same page will not transfer the File, but this brings us closer to
parity with other browsers. See per-method comments for details.
Tests: editing/pasteboard/data-transfer-item-list-add-file-multiple-times.html
editing/pasteboard/data-transfer-item-list-add-file-on-copy.html
editing/pasteboard/data-transfer-item-list-add-file-on-drag.html
* dom/DataTransfer.cpp:
(WebCore::DataTransfer::updateFileList):
Recompute the DataTransfer's FileList. This behaves the same way as destroying the FileList altogether and
building it from scratch, but we avoid that approach because the FileList object needs to maintain the same DOM
wrapper after a File-backed item is removed.
(WebCore::DataTransfer::itemListDidAddFile):
Add the newly appended DataTransferItem's File to the DataTransfer's FileList.
(WebCore::DataTransfer::types const):
Return only the "Files" type if there are file-backed items in the DataTransfer's item list.
(WebCore::DataTransfer::updatedFilesForFileList const):
(WebCore::DataTransfer::files const):
* dom/DataTransfer.h:
* dom/DataTransferItem.h:
(WebCore::DataTransferItem::file const):
* dom/DataTransferItemList.cpp:
(WebCore::DataTransferItemList::add):
(WebCore::DataTransferItemList::remove):
(WebCore::DataTransferItemList::clear):
When removing a File, only clear from the DataTransfer's pasteboard if the removed item is not a File (otherwise,
clearing a File that shares the same type as some other item in the pasteboard will erroneously clear that other
item as well). Additionally, call out to the DataTransfer to update the FileList.
* dom/DataTransferItemList.h:
(WebCore::DataTransferItemList::hasItems const):
(WebCore::DataTransferItemList::items const):
Add helpers for directly accessing an item list's items. items() should be used in conjunction with hasItems().
This route is taken to (1) avoid having to copy the vector of Files, and (2) to avoid generating m_items if it
doesn't already exist.
LayoutTests:
Add tests to verify that Files can be added to and removed from the DataTransferItemList, and also read back via
both the item list and the DataTransfer's FileList when copying and dragging. Additionally, adds a test that adds
and removes the same File to the DataTransferItemList multiple times.
* TestExpectations:
* editing/pasteboard/data-transfer-item-list-add-file-multiple-times-expected.txt: Added.
* editing/pasteboard/data-transfer-item-list-add-file-multiple-times.html: Added.
* editing/pasteboard/data-transfer-item-list-add-file-on-copy-expected.txt: Added.
* editing/pasteboard/data-transfer-item-list-add-file-on-copy.html: Added.
* editing/pasteboard/data-transfer-item-list-add-file-on-drag-expected.txt: Added.
* editing/pasteboard/data-transfer-item-list-add-file-on-drag.html: Added.
* platform/ios-simulator-wk1/TestExpectations:
* platform/mac-wk1/TestExpectations:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@222885 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 9904bfe..febdc79 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,57 @@
+2017-10-04 Wenson Hsieh <wenson_hsieh@apple.com>
+
+ Add basic support for the version of DataTransferItemList.add that takes a File
+ https://bugs.webkit.org/show_bug.cgi?id=177853
+ <rdar://problem/34807346>
+
+ Reviewed by Ryosuke Niwa.
+
+ Adds very basic support for DataTransferItemList.add(File). So far, a File added in this way can only be read
+ back from the same DataTransfer, during dragstart or copy. This File isn't written to the platform pasteboard
+ yet, so even dropping or pasting in the same page will not transfer the File, but this brings us closer to
+ parity with other browsers. See per-method comments for details.
+
+ Tests: editing/pasteboard/data-transfer-item-list-add-file-multiple-times.html
+ editing/pasteboard/data-transfer-item-list-add-file-on-copy.html
+ editing/pasteboard/data-transfer-item-list-add-file-on-drag.html
+
+ * dom/DataTransfer.cpp:
+ (WebCore::DataTransfer::updateFileList):
+
+ Recompute the DataTransfer's FileList. This behaves the same way as destroying the FileList altogether and
+ building it from scratch, but we avoid that approach because the FileList object needs to maintain the same DOM
+ wrapper after a File-backed item is removed.
+
+ (WebCore::DataTransfer::itemListDidAddFile):
+
+ Add the newly appended DataTransferItem's File to the DataTransfer's FileList.
+
+ (WebCore::DataTransfer::types const):
+
+ Return only the "Files" type if there are file-backed items in the DataTransfer's item list.
+
+ (WebCore::DataTransfer::updatedFilesForFileList const):
+ (WebCore::DataTransfer::files const):
+ * dom/DataTransfer.h:
+ * dom/DataTransferItem.h:
+ (WebCore::DataTransferItem::file const):
+ * dom/DataTransferItemList.cpp:
+ (WebCore::DataTransferItemList::add):
+ (WebCore::DataTransferItemList::remove):
+ (WebCore::DataTransferItemList::clear):
+
+ When removing a File, only clear from the DataTransfer's pasteboard if the removed item is not a File (otherwise,
+ clearing a File that shares the same type as some other item in the pasteboard will erroneously clear that other
+ item as well). Additionally, call out to the DataTransfer to update the FileList.
+
+ * dom/DataTransferItemList.h:
+ (WebCore::DataTransferItemList::hasItems const):
+ (WebCore::DataTransferItemList::items const):
+
+ Add helpers for directly accessing an item list's items. items() should be used in conjunction with hasItems().
+ This route is taken to (1) avoid having to copy the vector of Files, and (2) to avoid generating m_items if it
+ doesn't already exist.
+
2017-10-04 Zalan Bujtas <zalan@apple.com>
RenderMultiColumnFlow populate/evacuate should not disable layout state.
diff --git a/Source/WebCore/dom/DataTransfer.cpp b/Source/WebCore/dom/DataTransfer.cpp
index addef0f..f982d57 100644
--- a/Source/WebCore/dom/DataTransfer.cpp
+++ b/Source/WebCore/dom/DataTransfer.cpp
@@ -28,6 +28,7 @@
#include "CachedImage.h"
#include "CachedImageClient.h"
+#include "DataTransferItem.h"
#include "DataTransferItemList.h"
#include "DragData.h"
#include "Editor.h"
@@ -161,6 +162,26 @@
m_itemList->didSetStringData(normalizedType);
}
+void DataTransfer::updateFileList()
+{
+ // If we're removing an item, then the item list must exist, which implies that the file list must have been initialized already.
+ ASSERT(m_fileList);
+ ASSERT(canWriteData());
+
+ m_fileList->m_files = filesFromPasteboardAndItemList();
+}
+
+void DataTransfer::didAddFileToItemList()
+{
+ ASSERT(canWriteData());
+ if (!m_fileList)
+ return;
+
+ auto& newItem = m_itemList->items().last();
+ ASSERT(newItem->isFile());
+ m_fileList->append(*newItem->file());
+}
+
DataTransferItemList& DataTransfer::items()
{
if (!m_itemList)
@@ -181,6 +202,9 @@
return types;
}
+ if (m_itemList && m_itemList->hasItems() && m_itemList->items().findMatching([] (const auto& item) { return item->isFile(); }) != notFound)
+ return { "Files" };
+
if (m_pasteboard->containsFiles()) {
ASSERT(!m_pasteboard->typesSafeForBindings().contains("Files"));
return { "Files" };
@@ -191,6 +215,31 @@
return types;
}
+Vector<Ref<File>> DataTransfer::filesFromPasteboardAndItemList() const
+{
+ bool addedFilesFromPasteboard = false;
+ Vector<Ref<File>> files;
+ if (!forDrag() || forFileDrag()) {
+ WebCorePasteboardFileReader reader;
+ m_pasteboard->read(reader);
+ files = WTFMove(reader.files);
+ addedFilesFromPasteboard = !files.isEmpty();
+ }
+
+ bool itemListContainsItems = false;
+ if (m_itemList && m_itemList->hasItems()) {
+ for (auto& item : m_itemList->items()) {
+ if (auto file = item->file()) {
+ files.append(*file);
+ }
+ }
+ itemListContainsItems = true;
+ }
+
+ ASSERT(!itemListContainsItems || !addedFilesFromPasteboard);
+ return files;
+}
+
FileList& DataTransfer::files() const
{
if (!canReadData()) {
@@ -201,19 +250,9 @@
return *m_fileList;
}
- if (forDrag() && !forFileDrag()) {
- if (m_fileList)
- ASSERT(m_fileList->isEmpty());
- else
- m_fileList = FileList::create();
- return *m_fileList;
- }
+ if (!m_fileList)
+ m_fileList = FileList::create(filesFromPasteboardAndItemList());
- if (!m_fileList) {
- WebCorePasteboardFileReader reader;
- m_pasteboard->read(reader);
- m_fileList = FileList::create(WTFMove(reader.files));
- }
return *m_fileList;
}
diff --git a/Source/WebCore/dom/DataTransfer.h b/Source/WebCore/dom/DataTransfer.h
index 09d4dff..eda0ede 100644
--- a/Source/WebCore/dom/DataTransfer.h
+++ b/Source/WebCore/dom/DataTransfer.h
@@ -36,6 +36,7 @@
class DragImageLoader;
class Element;
class FileList;
+class File;
class Pasteboard;
class DataTransfer : public RefCounted<DataTransfer> {
@@ -99,6 +100,9 @@
bool hasDragImage() const;
#endif
+ void didAddFileToItemList();
+ void updateFileList();
+
private:
enum class Type { CopyAndPaste, DragAndDropData, DragAndDropFiles, InputEvent };
DataTransfer(StoreMode, std::unique_ptr<Pasteboard>, Type = Type::CopyAndPaste);
@@ -111,6 +115,8 @@
bool forFileDrag() const { return false; }
#endif
+ Vector<Ref<File>> filesFromPasteboardAndItemList() const;
+
StoreMode m_storeMode;
std::unique_ptr<Pasteboard> m_pasteboard;
std::unique_ptr<DataTransferItemList> m_itemList;
diff --git a/Source/WebCore/dom/DataTransferItem.h b/Source/WebCore/dom/DataTransferItem.h
index 9ac11e7..9685529 100644
--- a/Source/WebCore/dom/DataTransferItem.h
+++ b/Source/WebCore/dom/DataTransferItem.h
@@ -54,6 +54,7 @@
~DataTransferItem();
+ RefPtr<File> file() const { return m_file; }
void clearListAndPutIntoDisabledMode();
bool isFile() const { return m_file; }
diff --git a/Source/WebCore/dom/DataTransferItemList.cpp b/Source/WebCore/dom/DataTransferItemList.cpp
index 59704b4..690a2ab 100644
--- a/Source/WebCore/dom/DataTransferItemList.cpp
+++ b/Source/WebCore/dom/DataTransferItemList.cpp
@@ -84,9 +84,14 @@
return RefPtr<DataTransferItem> { m_items->last().copyRef() };
}
-RefPtr<DataTransferItem> DataTransferItemList::add(Ref<File>&&)
+RefPtr<DataTransferItem> DataTransferItemList::add(Ref<File>&& file)
{
- return nullptr;
+ if (!m_dataTransfer.canWriteData())
+ return nullptr;
+
+ ensureItems().append(DataTransferItem::create(m_weakPtrFactory.createWeakPtr(*this), file->type(), file.copyRef()));
+ m_dataTransfer.didAddFileToItemList();
+ return RefPtr<DataTransferItem> { m_items->last().ptr() };
}
ExceptionOr<void> DataTransferItemList::remove(unsigned index)
@@ -98,13 +103,16 @@
if (items.size() <= index)
return Exception { IndexSizeError }; // Matches Gecko. See https://github.com/whatwg/html/issues/2925
- // FIXME: Handle the removal of files once we added the support for writing a File.
- ASSERT(!items[index]->isFile());
-
+ // Since file-backed DataTransferItems are not actually written to the pasteboard yet, we don't need to remove any
+ // temporary files. When we support writing file-backed DataTransferItems to the platform pasteboard, we will need
+ // to clean up here.
auto& removedItem = items[index].get();
- m_dataTransfer.pasteboard().clear(removedItem.type());
+ if (!removedItem.isFile())
+ m_dataTransfer.pasteboard().clear(removedItem.type());
removedItem.clearListAndPutIntoDisabledMode();
items.remove(index);
+ if (removedItem.isFile())
+ m_dataTransfer.updateFileList();
return { };
}
@@ -112,11 +120,17 @@
void DataTransferItemList::clear()
{
m_dataTransfer.pasteboard().clear();
+ bool removedItemContainingFile = false;
if (m_items) {
- for (auto& item : *m_items)
+ for (auto& item : *m_items) {
+ removedItemContainingFile |= item->isFile();
item->clearListAndPutIntoDisabledMode();
+ }
m_items->clear();
}
+
+ if (removedItemContainingFile)
+ m_dataTransfer.updateFileList();
}
Vector<Ref<DataTransferItem>>& DataTransferItemList::ensureItems() const
diff --git a/Source/WebCore/dom/DataTransferItemList.h b/Source/WebCore/dom/DataTransferItemList.h
index fce6947..13120e54 100644
--- a/Source/WebCore/dom/DataTransferItemList.h
+++ b/Source/WebCore/dom/DataTransferItemList.h
@@ -65,6 +65,12 @@
void didClearStringData(const String& type);
void didSetStringData(const String& type);
+ bool hasItems() const { return m_items.has_value(); }
+ const Vector<Ref<DataTransferItem>>& items() const
+ {
+ ASSERT(m_items);
+ return *m_items;
+ }
private:
Vector<Ref<DataTransferItem>>& ensureItems() const;
