Forbid setDragImage after dragstart

https://bugs.webkit.org/show_bug.cgi?id=175751

Reviewed by Wenson Hsieh.

Removed the code to allow setting the drag mage after dragstart had happened.

The feature was apparently used in Mac WebKit1 port but using it today causes the drag image
to disapepar while the user is moving the mouse cursor and being drawn once it's stopped
and results in the contionus flickering of the drag image.

The feaure was never supported in WebKit2 and doesn't match the HTML5 specification:
https://html.spec.whatwg.org/multipage/dnd.html#concept-dnd-rw
https://html.spec.whatwg.org/multipage/dnd.html#dom-datatransfer-setdragimage
or the behaviors of other browsers such as Chrome and Firefox.

No new tests. This patch simply removes code.

* dom/DataTransfer.cpp:
(WebCore::DataTransfer::setDragImage):
(WebCore::DataTransfer::canSetDragImage const): Deleted.
* dom/DataTransfer.h:
(WebCore::DataTransfer::makeDragImageWritable): Deleted.
* page/EventHandler.cpp:
(WebCore::EventHandler::handleDrag):

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 88e3a5806820f8a3aca7751862c52a8830406486..5a9cf97901f93f3267ac6a8a9778ec0029d54ce4 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,31 @@
+2017-08-18  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Forbid setDragImage after dragstart
+        https://bugs.webkit.org/show_bug.cgi?id=175751
+
+        Reviewed by Wenson Hsieh.
+
+        Removed the code to allow setting the drag mage after dragstart had happened.
+
+        The feature was apparently used in Mac WebKit1 port but using it today causes the drag image
+        to disapepar while the user is moving the mouse cursor and being drawn once it's stopped
+        and results in the contionus flickering of the drag image.
+
+        The feaure was never supported in WebKit2 and doesn't match the HTML5 specification:
+        https://html.spec.whatwg.org/multipage/dnd.html#concept-dnd-rw
+        https://html.spec.whatwg.org/multipage/dnd.html#dom-datatransfer-setdragimage
+        or the behaviors of other browsers such as Chrome and Firefox.
+
+        No new tests. This patch simply removes code.
+
+        * dom/DataTransfer.cpp:
+        (WebCore::DataTransfer::setDragImage):
+        (WebCore::DataTransfer::canSetDragImage const): Deleted.
+        * dom/DataTransfer.h:
+        (WebCore::DataTransfer::makeDragImageWritable): Deleted.
+        * page/EventHandler.cpp:
+        (WebCore::EventHandler::handleDrag):
+
 2017-08-18  Sam Weinig  <sam@webkit.org>
 
         [WebCrypto] Get rid of CryptoKeyData class and all its subclasses

diff --git a/Source/WebCore/dom/DataTransfer.cpp b/Source/WebCore/dom/DataTransfer.cpp
index 01484def3ad921a54a9b8ec109457a0d2bf2236a..db46e541c02ce48423fd969bd054ec844aff32ab 100644 (file)
--- a/Source/WebCore/dom/DataTransfer.cpp
+++ b/Source/WebCore/dom/DataTransfer.cpp
@@ -246,18 +246,9 @@ Ref<DataTransfer> DataTransfer::createForDrop(StoreMode accessMode, const DragDa
     return adoptRef(*new DataTransfer(accessMode, Pasteboard::createForDragAndDrop(dragData), type));
 }
 
-bool DataTransfer::canSetDragImage() const
-{
-    // Note that the spec doesn't actually allow drag image modification outside the dragstart
-    // event. This capability is maintained for backwards compatiblity for ports that have
-    // supported this in the past. On many ports, attempting to set a drag image outside the
-    // dragstart operation is a no-op anyway.
-    return m_forDrag && (m_storeMode == StoreMode::DragImageWritable || m_storeMode == StoreMode::ReadWrite);
-}
-
 void DataTransfer::setDragImage(Element* element, int x, int y)
 {
-    if (!canSetDragImage())
+    if (!m_forDrag || !canWriteData())
         return;
 
     CachedImage* image = nullptr;

diff --git a/Source/WebCore/dom/DataTransfer.h b/Source/WebCore/dom/DataTransfer.h
index dd8438a5cf62819d871bbaa2d4b03e32191dc590..4c6b4751312f0bc7a37abc584527da23417cf431 100644 (file)
--- a/Source/WebCore/dom/DataTransfer.h
+++ b/Source/WebCore/dom/DataTransfer.h
@@ -41,7 +41,7 @@ class Pasteboard;
 class DataTransfer : public RefCounted<DataTransfer> {
 public:
     // https://html.spec.whatwg.org/multipage/dnd.html#drag-data-store-mode
-    enum class StoreMode { Invalid, DragImageWritable, ReadWrite, Readonly, Protected };
+    enum class StoreMode { Invalid, ReadWrite, Readonly, Protected };
 
     static Ref<DataTransfer> createForCopyAndPaste(StoreMode);
     static Ref<DataTransfer> createForInputEvent(const String& plainText, const String& htmlText);
@@ -68,11 +68,6 @@ public:
     void setDragImage(Element*, int x, int y);
 
     void makeInvalidForSecurity() { m_storeMode = StoreMode::Invalid; }
-    void makeDragImageWritable()
-    {
-        ASSERT(m_storeMode != StoreMode::Invalid);
-        m_storeMode = StoreMode::DragImageWritable;
-    }
 
     bool canReadTypes() const;
     bool canReadData() const;
@@ -103,10 +98,6 @@ private:
     enum class Type { CopyAndPaste, DragAndDropData, DragAndDropFiles, InputEvent };
     DataTransfer(StoreMode, std::unique_ptr<Pasteboard>, Type = Type::CopyAndPaste);
 
-#if ENABLE(DRAG_SUPPORT)
-    bool canSetDragImage() const;
-#endif
-
     StoreMode m_storeMode;
     std::unique_ptr<Pasteboard> m_pasteboard;
     std::unique_ptr<DataTransferItemList> m_itemList;

diff --git a/Source/WebCore/page/EventHandler.cpp b/Source/WebCore/page/EventHandler.cpp
index e021fa387cf857406f20c973288e800c9bfaac82..e538ba6c5312d9c819eb5a756640ac5aba8cd3ab 100644 (file)
--- a/Source/WebCore/page/EventHandler.cpp
+++ b/Source/WebCore/page/EventHandler.cpp
@@ -3683,9 +3683,7 @@ bool EventHandler::handleDrag(const MouseEventWithHitTestResults& event, CheckDr
         m_mouseDownMayStartDrag = dispatchDragSrcEvent(eventNames().dragstartEvent, m_mouseDown)
             && !m_frame.selection().selection().isInPasswordField();
 
-        // Invalidate dataTransfer here against anymore pasteboard writing for security. The drag
-        // image can still be changed as we drag, but not the pasteboard data.
-        dragState().dataTransfer->makeDragImageWritable();
+        dragState().dataTransfer->makeInvalidForSecurity();
 
         if (m_mouseDownMayStartDrag) {
             // Gather values from DHTML element, if it set any.