REGRESSION(r210287) On drop, event.dataTransfer.getData("text") returns an empty...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Aug 2017 05:09:23 +0000 (05:09 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Aug 2017 05:09:23 +0000 (05:09 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170637
<rdar://problem/31347248>

Reviewed by Ryosuke Niwa.

Source/WebCore:

In r210287, the behavior of DragData::containsFiles was changed to return true if NSFilesPromisePboardType is
present in the pasteboard. This means that we will consider images dragged from web content, for which we add
the NSFilesPromisePboardType UTI, as containing files on the pasteboard; this, in turn, means we'll initialize
the DataTransfer upon drop with m_forFileDrag set to true. Due to early returns in getData() and setData() to
deny data access when dropping a dragged file, this means the page won't ever get access to the URL in the
pasteboard due to the presence of the NSFilesPromisePboardType UTI.

To fix this, we replace the early m_forFileDrag returns in getData and setData, instead early returning the null
string if there are any file URLs present on the pasteboard (determined via readFilenames() retrieving a non-
empty result).

Test: editing/pasteboard/drag-drop-href-as-text-data.html

* dom/DataTransfer.cpp:
(WebCore::DataTransfer::DataTransfer):
(WebCore::DataTransfer::getData const):
(WebCore::DataTransfer::setData):

Rather than bail upon forFileDrag() (formerly, m_forFileDrag) being true, bail if there are any file URLs
present on the pasteboard. It seems like this was the intention of the early return in the first place, to
prevent the page from being able to ask for a real file URL when dragging a file.

(WebCore::DataTransfer::files const):
(WebCore::DataTransfer::setDragImage):
(WebCore::DataTransfer::setDropEffect):
(WebCore::DataTransfer::setEffectAllowed):

Swap m_forDrag and m_forFileDrag with forDrag() and forFileDrag(), respectively.

* dom/DataTransfer.h:
(WebCore::DataTransfer::forDrag const):
(WebCore::DataTransfer::forFileDrag const):

Instead of caching two bools to represent state (m_forDrag and m_forFileDrag), just remember the DataTransfer's
m_type and turn the flags into const helpers that check for the value of m_type.

LayoutTests:

Adds a new test to verify that upon dropping an image enclosed within an anchor, DataTransfer.getData() can be
used to grab the href of the enclosing anchor.

* TestExpectations:
* editing/pasteboard/drag-drop-href-as-text-data-expected.txt: Added.
* editing/pasteboard/drag-drop-href-as-text-data.html: Added.
* platform/mac-wk1/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/editing/pasteboard/drag-drop-href-as-text-data-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/drag-drop-href-as-text-data.html [new file with mode: 0644]
LayoutTests/platform/mac-wk1/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/dom/DataTransfer.cpp
Source/WebCore/dom/DataTransfer.h

index 24bd16f170bda13727479a55d7010b951b66e224..6231952112d1c5a1cbb043aed739d75cd0d3fb04 100644 (file)
@@ -1,3 +1,19 @@
+2017-08-29  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        REGRESSION(r210287) On drop, event.dataTransfer.getData("text") returns an empty string when dragging an image
+        https://bugs.webkit.org/show_bug.cgi?id=170637
+        <rdar://problem/31347248>
+
+        Reviewed by Ryosuke Niwa.
+
+        Adds a new test to verify that upon dropping an image enclosed within an anchor, DataTransfer.getData() can be
+        used to grab the href of the enclosing anchor.
+
+        * TestExpectations:
+        * editing/pasteboard/drag-drop-href-as-text-data-expected.txt: Added.
+        * editing/pasteboard/drag-drop-href-as-text-data.html: Added.
+        * platform/mac-wk1/TestExpectations:
+
 2017-08-29  Devin Rousso  <webkit@devinrousso.com>
 
         CallTracingCallback should ignore `readonly attribute`
index e43696659eea4a04114711834e2506f0800ef518..173b3ec9cb013a2034be76caf4e061da3857cdd5 100644 (file)
@@ -64,6 +64,9 @@ fast/events/mouse-force-up.html [ Skip ]
 fast/events/autoscroll-when-zoomed.html [ Skip ]
 fast/events/autoscroll-main-document.html [ Skip ]
 
+# Drag and drop via EventSender is only supported on Mac WK1
+editing/pasteboard/drag-drop-href-as-text-data.html [ Skip ]
+
 # Only iOS supports QuickLook
 quicklook [ Skip ]
 http/tests/quicklook [ Skip ]
diff --git a/LayoutTests/editing/pasteboard/drag-drop-href-as-text-data-expected.txt b/LayoutTests/editing/pasteboard/drag-drop-href-as-text-data-expected.txt
new file mode 100644 (file)
index 0000000..f0b68e4
--- /dev/null
@@ -0,0 +1,5 @@
+
+text: "https://www.webkit.org/"
+url: "https://www.webkit.org/"
+text/plain: "https://www.webkit.org/"
+text/uri-list: "https://www.webkit.org/"
diff --git a/LayoutTests/editing/pasteboard/drag-drop-href-as-text-data.html b/LayoutTests/editing/pasteboard/drag-drop-href-as-text-data.html
new file mode 100644 (file)
index 0000000..cee73e5
--- /dev/null
@@ -0,0 +1,52 @@
+<!DOCTYPE html>
+<html>
+<style>
+body {
+    margin: 0;
+}
+#source {
+    width: 300px;
+    height: 300px;
+}
+#target {
+    border: 1px blue dashed;
+    font-family: monospace;
+    overflow: scroll;
+    white-space: nowrap;
+    width: 100%;
+    height: 300px;
+}
+</style>
+
+<a id="link" href="https://www.webkit.org/"><img id="source" src="../resources/abe.png"></img></a>
+<div id="target"></div>
+
+<script>
+function append(text) {
+    let div = document.createElement("div");
+    div.textContent = text;
+    target.appendChild(div);
+}
+
+if (window.testRunner)
+    testRunner.dumpAsText();
+
+target.addEventListener("dragenter", event => event.preventDefault());
+target.addEventListener("dragover", event => event.preventDefault());
+target.addEventListener("drop", event => {
+    for (let type of ["text", "url", "text/plain", "text/uri-list"])
+        append(`${type}: "${event.dataTransfer.getData(type)}"`);
+    event.preventDefault();
+});
+
+if (window.eventSender) {
+    let x = source.offsetLeft + source.offsetWidth / 2;
+    let y = source.offsetTop + source.offsetHeight / 2;
+    eventSender.mouseMoveTo(x, y);
+    eventSender.mouseDown();
+    eventSender.leapForward(500);
+    eventSender.mouseMoveTo(x, y + 300);
+    eventSender.mouseUp();
+}
+</script>
+</html>
\ No newline at end of file
index 345127000764ba7417986af5b45815e0d742fd92..5ff8acbc974d5691ac3a610e583d2073e359848c 100644 (file)
@@ -6,6 +6,7 @@
 #//////////////////////////////////////////////////////////////////////////////////////////
 
 fast/forms/attributed-strings.html [ Pass ]
+editing/pasteboard/drag-drop-href-as-text-data.html [ Pass ]
 
 #//////////////////////////////////////////////////////////////////////////////////////////
 # End platform-specific directories.
index 8c29f838efc62c0935a43411f5b0b9805f4bcd8f..1ff256e2c8ef85dd74ccde0a631f0e7bf04dd718 100644 (file)
@@ -1,3 +1,47 @@
+2017-08-29  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        REGRESSION(r210287) On drop, event.dataTransfer.getData("text") returns an empty string when dragging an image
+        https://bugs.webkit.org/show_bug.cgi?id=170637
+        <rdar://problem/31347248>
+
+        Reviewed by Ryosuke Niwa.
+
+        In r210287, the behavior of DragData::containsFiles was changed to return true if NSFilesPromisePboardType is
+        present in the pasteboard. This means that we will consider images dragged from web content, for which we add
+        the NSFilesPromisePboardType UTI, as containing files on the pasteboard; this, in turn, means we'll initialize
+        the DataTransfer upon drop with m_forFileDrag set to true. Due to early returns in getData() and setData() to
+        deny data access when dropping a dragged file, this means the page won't ever get access to the URL in the
+        pasteboard due to the presence of the NSFilesPromisePboardType UTI.
+
+        To fix this, we replace the early m_forFileDrag returns in getData and setData, instead early returning the null
+        string if there are any file URLs present on the pasteboard (determined via readFilenames() retrieving a non-
+        empty result).
+
+        Test: editing/pasteboard/drag-drop-href-as-text-data.html
+
+        * dom/DataTransfer.cpp:
+        (WebCore::DataTransfer::DataTransfer):
+        (WebCore::DataTransfer::getData const):
+        (WebCore::DataTransfer::setData):
+
+        Rather than bail upon forFileDrag() (formerly, m_forFileDrag) being true, bail if there are any file URLs
+        present on the pasteboard. It seems like this was the intention of the early return in the first place, to
+        prevent the page from being able to ask for a real file URL when dragging a file.
+
+        (WebCore::DataTransfer::files const):
+        (WebCore::DataTransfer::setDragImage):
+        (WebCore::DataTransfer::setDropEffect):
+        (WebCore::DataTransfer::setEffectAllowed):
+
+        Swap m_forDrag and m_forFileDrag with forDrag() and forFileDrag(), respectively.
+
+        * dom/DataTransfer.h:
+        (WebCore::DataTransfer::forDrag const):
+        (WebCore::DataTransfer::forFileDrag const):
+
+        Instead of caching two bools to represent state (m_forDrag and m_forFileDrag), just remember the DataTransfer's
+        m_type and turn the flags into const helpers that check for the value of m_type.
+
 2017-08-29  Youenn Fablet  <youenn@apple.com>
 
         [Fetch API] Request should throw when keep alive is true and body is a ReadableStream
index 77939be2529b9134bd22efcd2be928a410a51c48..e238045628ce0a182b4c5b55d57fc65b21f4ff5b 100644 (file)
@@ -61,8 +61,7 @@ DataTransfer::DataTransfer(StoreMode mode, std::unique_ptr<Pasteboard> pasteboar
     : m_storeMode(mode)
     , m_pasteboard(WTFMove(pasteboard))
 #if ENABLE(DRAG_SUPPORT)
-    , m_forDrag(type == Type::DragAndDropData || type == Type::DragAndDropFiles)
-    , m_forFileDrag(type == Type::DragAndDropFiles)
+    , m_type(type)
     , m_dropEffect(ASCIILiteral("uninitialized"))
     , m_effectAllowed(ASCIILiteral("uninitialized"))
     , m_shouldUpdateDragImage(false)
@@ -137,8 +136,8 @@ String DataTransfer::getData(const String& type) const
         return String();
 
 #if ENABLE(DRAG_SUPPORT)
-    if (m_forFileDrag)
-        return String();
+    if (forFileDrag() && m_pasteboard->readFilenames().size())
+        return { };
 #endif
 
     return m_pasteboard->readString(normalizeType(type));
@@ -150,7 +149,7 @@ void DataTransfer::setData(const String& type, const String& data)
         return;
 
 #if ENABLE(DRAG_SUPPORT)
-    if (m_forFileDrag)
+    if (forFileDrag() && m_pasteboard->readFilenames().size())
         return;
 #endif
 
@@ -187,7 +186,7 @@ FileList& DataTransfer::files() const
     }
 
 #if ENABLE(DRAG_SUPPORT)
-    if (m_forDrag && !m_forFileDrag) {
+    if (forDrag() && !forFileDrag()) {
         ASSERT(m_fileList->isEmpty());
         return *m_fileList;
     }
@@ -266,7 +265,7 @@ Ref<DataTransfer> DataTransfer::createForDrop(StoreMode accessMode, const DragDa
 
 void DataTransfer::setDragImage(Element* element, int x, int y)
 {
-    if (!m_forDrag || !canWriteData())
+    if (!forDrag() || !canWriteData())
         return;
 
     CachedImage* image = nullptr;
@@ -422,7 +421,7 @@ String DataTransfer::dropEffect() const
 
 void DataTransfer::setDropEffect(const String& effect)
 {
-    if (!m_forDrag)
+    if (!forDrag())
         return;
 
     if (effect != "none" && effect != "copy" && effect != "link" && effect != "move")
@@ -443,7 +442,7 @@ String DataTransfer::effectAllowed() const
 
 void DataTransfer::setEffectAllowed(const String& effect)
 {
-    if (!m_forDrag)
+    if (!forDrag())
         return;
 
     // Ignore any attempts to set it to an unknown value.
index 4c6b4751312f0bc7a37abc584527da23417cf431..be23127932078907ea1e8261a4a01370fb89e5c8 100644 (file)
@@ -98,6 +98,11 @@ private:
     enum class Type { CopyAndPaste, DragAndDropData, DragAndDropFiles, InputEvent };
     DataTransfer(StoreMode, std::unique_ptr<Pasteboard>, Type = Type::CopyAndPaste);
 
+#if ENABLE(DRAG_SUPPORT)
+    bool forDrag() const { return m_type == Type::DragAndDropData || m_type == Type::DragAndDropFiles; }
+    bool forFileDrag() const { return m_type == Type::DragAndDropFiles; }
+#endif
+
     StoreMode m_storeMode;
     std::unique_ptr<Pasteboard> m_pasteboard;
     std::unique_ptr<DataTransferItemList> m_itemList;
@@ -105,8 +110,7 @@ private:
     mutable RefPtr<FileList> m_fileList;
 
 #if ENABLE(DRAG_SUPPORT)
-    bool m_forDrag;
-    bool m_forFileDrag;
+    Type m_type;
     String m_dropEffect;
     String m_effectAllowed;
     bool m_shouldUpdateDragImage;