Changeset 221343 in webkit

Timestamp:

Aug 29, 2017, 10:09:23 PM (8 years ago)

Author:

Wenson Hsieh

Message:

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.

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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/editing/pasteboard/drag-drop-href-as-text-data-expected.txt (added)
• LayoutTests/editing/pasteboard/drag-drop-href-as-text-data.html (added)
• LayoutTests/platform/mac-wk1/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/DataTransfer.cpp (modified) (7 diffs)
• Source/WebCore/dom/DataTransfer.h (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/TestExpectations

@@ -64 +64 @@
 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

trunk/LayoutTests/platform/mac-wk1/TestExpectations

@@ -7 +7 @@
 
 fast/forms/attributed-strings.html [ Pass ]
+editing/pasteboard/drag-drop-href-as-text-data.html [ Pass ]
 
 #//////////////////////////////////////////////////////////////////////////////////////////

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/dom/DataTransfer.cpp

@@ -62 +62 @@
     , 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"))
@@ -138 +137 @@
 
 #if ENABLE(DRAG_SUPPORT)
-    if (m_forFileDrag)
-        return String();
+    if (forFileDrag() && m_pasteboard->readFilenames().size())
+        return { };
 #endif
 
@@ -151 +150 @@
 
 #if ENABLE(DRAG_SUPPORT)
-    if (m_forFileDrag)
+    if (forFileDrag() && m_pasteboard->readFilenames().size())
         return;
 #endif
@@ -188 +187 @@
 
 #if ENABLE(DRAG_SUPPORT)
-    if (m_forDrag && !m_forFileDrag) {
+    if (forDrag() && !forFileDrag()) {
         ASSERT(m_fileList->isEmpty());
         return *m_fileList;
@@ -267 +266 @@
 void DataTransfer::setDragImage(Element* element, int x, int y)
 {
-    if (!m_forDrag || !canWriteData())
+    if (!forDrag() || !canWriteData())
         return;
 
@@ -423 +422 @@
 void DataTransfer::setDropEffect(const String& effect)
 {
-    if (!m_forDrag)
+    if (!forDrag())
         return;
 
@@ -444 +443 @@
 void DataTransfer::setEffectAllowed(const String& effect)
 {
-    if (!m_forDrag)
+    if (!forDrag())
         return;
 

trunk/Source/WebCore/dom/DataTransfer.h

@@ -99 +99 @@
     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;
@@ -106 +111 @@
 
 #if ENABLE(DRAG_SUPPORT)
-    bool m_forDrag;
-    bool m_forFileDrag;
+    Type m_type;
     String m_dropEffect;
     String m_effectAllowed;