Bug 1552814, make it possible to start dnd from dragable element even if DataTransfer is empty, r=NeilDeakin

Chrome and old Edge at least seem to have this behavior, and this way the testcase on the bug doesn't trigger click anymore since
we enter dnd mode and get dragleave etc. events.

Manually tested on linux and Windows, and annyg tested on Mac

Update test_dragstart.html's draggable=true test to follow the pattern used by other tests

Differential Revision: https://phabricator.services.mozilla.com/D48208

--HG--
extra : moz-landing-system : lando

dom/events/EventStateManager.cpp

@@ -1692,9 +1692,10 @@ void EventStateManager::StopTrackingDragGesture(bool aClearInChildProcesses) {
   mGestureDownFrameOwner = nullptr;
   mGestureDownDragStartData = nullptr;
 
-  // If a content process starts a drag but the mouse is released before the parent
-  // starts the actual drag, the content process will think a drag is still happening.
-  // Inform any child processes with active drags that the drag should be stopped.
+  // If a content process starts a drag but the mouse is released before the
+  // parent starts the actual drag, the content process will think a drag is
+  // still happening. Inform any child processes with active drags that the drag
+  // should be stopped.
   if (aClearInChildProcesses) {
     nsCOMPtr<nsIDragService> dragService =
         do_GetService("@mozilla.org/widget/dragservice;1");
@@ -1854,6 +1855,7 @@ void EventStateManager::GenerateDragGesture(nsPresContext* aPresContext,
   nsCOMPtr<nsIContent> eventContent, targetContent;
   nsCOMPtr<nsIPrincipal> principal;
   nsCOMPtr<nsIContentSecurityPolicy> csp;
+  bool allowEmptyDataTransfer = false;
   mCurrentTarget->GetContentForEvent(aEvent, getter_AddRefs(eventContent));
   if (eventContent) {
     // If the content is a text node in a password field, we shouldn't
@@ -1875,9 +1877,10 @@ void EventStateManager::GenerateDragGesture(nsPresContext* aPresContext,
       }
     }
     DetermineDragTargetAndDefaultData(
-        window, eventContent, dataTransfer, getter_AddRefs(selection),
-        getter_AddRefs(remoteDragStartData), getter_AddRefs(targetContent),
-        getter_AddRefs(principal), getter_AddRefs(csp));
+        window, eventContent, dataTransfer, &allowEmptyDataTransfer,
+        getter_AddRefs(selection), getter_AddRefs(remoteDragStartData),
+        getter_AddRefs(targetContent), getter_AddRefs(principal),
+        getter_AddRefs(csp));
   }
 
   // Stop tracking the drag gesture now. This should stop us from
@@ -1942,9 +1945,9 @@ void EventStateManager::GenerateDragGesture(nsPresContext* aPresContext,
   }
 
   if (status != nsEventStatus_eConsumeNoDefault) {
-    bool dragStarted =
-        DoDefaultDragStart(aPresContext, event, dataTransfer, targetContent,
-                           selection, remoteDragStartData, principal, csp);
+    bool dragStarted = DoDefaultDragStart(
+        aPresContext, event, dataTransfer, allowEmptyDataTransfer,
+        targetContent, selection, remoteDragStartData, principal, csp);
     if (dragStarted) {
       sActiveESM = nullptr;
       MaybeFirePointerCancel(aEvent);
@@ -1962,11 +1965,12 @@ void EventStateManager::GenerateDragGesture(nsPresContext* aPresContext,
 
 void EventStateManager::DetermineDragTargetAndDefaultData(
     nsPIDOMWindowOuter* aWindow, nsIContent* aSelectionTarget,
-    DataTransfer* aDataTransfer, Selection** aSelection,
-    RemoteDragStartData** aRemoteDragStartData, nsIContent** aTargetNode,
-    nsIPrincipal** aPrincipal, nsIContentSecurityPolicy** aCsp) {
+    DataTransfer* aDataTransfer, bool* aAllowEmptyDataTransfer,
+    Selection** aSelection, RemoteDragStartData** aRemoteDragStartData,
+    nsIContent** aTargetNode, nsIPrincipal** aPrincipal,
+    nsIContentSecurityPolicy** aCsp) {
   *aTargetNode = nullptr;
-
+  *aAllowEmptyDataTransfer = false;
   nsCOMPtr<nsIContent> dragDataNode;
 
   nsIContent* editingElement = aSelectionTarget->IsEditable()
@@ -1982,6 +1986,7 @@ void EventStateManager::DetermineDragTargetAndDefaultData(
       mGestureDownDragStartData->AddInitialDnDDataTo(aDataTransfer, aPrincipal,
                                                      aCsp);
       mGestureDownDragStartData.forget(aRemoteDragStartData);
+      *aAllowEmptyDataTransfer = true;
     }
   } else {
     mGestureDownDragStartData = nullptr;
@@ -2021,6 +2026,9 @@ void EventStateManager::DetermineDragTargetAndDefaultData(
     while (dragContent) {
       if (auto htmlElement = nsGenericHTMLElement::FromNode(dragContent)) {
         if (htmlElement->Draggable()) {
+          // We let draggable elements to trigger dnd even if there is no data
+          // in the DataTransfer.
+          *aAllowEmptyDataTransfer = true;
           break;
         }
       } else {
@@ -2057,7 +2065,8 @@ void EventStateManager::DetermineDragTargetAndDefaultData(
 
 bool EventStateManager::DoDefaultDragStart(
     nsPresContext* aPresContext, WidgetDragEvent* aDragEvent,
-    DataTransfer* aDataTransfer, nsIContent* aDragTarget, Selection* aSelection,
+    DataTransfer* aDataTransfer, bool aAllowEmptyDataTransfer,
+    nsIContent* aDragTarget, Selection* aSelection,
     RemoteDragStartData* aDragStartData, nsIPrincipal* aPrincipal,
     nsIContentSecurityPolicy* aCsp) {
   nsCOMPtr<nsIDragService> dragService =
@@ -2081,7 +2090,7 @@ bool EventStateManager::DoDefaultDragStart(
   if (aDataTransfer) {
     count = aDataTransfer->MozItemCount();
   }
-  if (!count) {
+  if (!aAllowEmptyDataTransfer && !count) {
     return false;
   }
 

dom/events/EventStateManager.h

@@ -1046,6 +1046,8 @@ class EventStateManager : public nsSupportsWeakReference, public nsIObserver {
    *
    * aSelectionTarget - target to check for selection
    * aDataTransfer - data transfer object that will contain the data to drag
+   * aAllowEmptyDataTransfer - [out] set to true, if dnd operation can be
+   *                           started even if DataTransfer is empty
    * aSelection - [out] set to the selection to be dragged
    * aTargetNode - [out] the draggable node, or null if there isn't one
    * aPrincipal - [out] set to the triggering principal of the drag, or null
@@ -1053,7 +1055,8 @@ class EventStateManager : public nsSupportsWeakReference, public nsIObserver {
    */
   void DetermineDragTargetAndDefaultData(
       nsPIDOMWindowOuter* aWindow, nsIContent* aSelectionTarget,
-      dom::DataTransfer* aDataTransfer, dom::Selection** aSelection,
+      dom::DataTransfer* aDataTransfer, bool* aAllowEmptyDataTransfer,
+      dom::Selection** aSelection,
       dom::RemoteDragStartData** aRemoteDragStartData, nsIContent** aTargetNode,
       nsIPrincipal** aPrincipal, nsIContentSecurityPolicy** aCsp);
 
@@ -1064,6 +1067,8 @@ class EventStateManager : public nsSupportsWeakReference, public nsIObserver {
    *
    * aDragEvent - the dragstart event
    * aDataTransfer - the data transfer that holds the data to be dragged
+   * aAllowEmptyDataTransfer - if true, dnd can be started even if there is no
+   *                           data to drag
    * aDragTarget - the target of the drag
    * aSelection - the selection to be dragged
    * aData - information pertaining to a drag started in a child process
@@ -1074,6 +1079,7 @@ class EventStateManager : public nsSupportsWeakReference, public nsIObserver {
   bool DoDefaultDragStart(nsPresContext* aPresContext,
                           WidgetDragEvent* aDragEvent,
                           dom::DataTransfer* aDataTransfer,
+                          bool aAllowEmptyDataTransfer,
                           nsIContent* aDragTarget, dom::Selection* aSelection,
                           dom::RemoteDragStartData* aDragStartData,
                           nsIPrincipal* aPrincipal,

dom/events/test/test_dragstart.html

@@ -533,6 +533,7 @@ function onDragStartDraggable(event)
   var dt = event.dataTransfer;
   ok(SpecialPowers.wrap(dt).mozItemCount == 0 && dt.types.length == 0 && event.originalTarget == gDragInfo.target, gDragInfo.testid);
 
+  event.preventDefault();
   gExtraDragTests++;
 }
 

widget/gtk/nsDragService.cpp

@@ -1244,6 +1244,9 @@ GtkTargetList* nsDragService::GetSourceList(void) {
       g_free(thisTarget);
     }
     g_free(targets);
+  } else {
+    // We need to create a dummy target list to be able initialize dnd.
+    targetList = gtk_target_list_new(nullptr, 0);
   }
   return targetList;
 }

widget/nsBaseDragService.cpp

@@ -46,7 +46,7 @@
 #include "mozilla/Unused.h"
 #include "nsFrameLoader.h"
 #include "BrowserParent.h"
-
+#include "nsIMutableArray.h"
 #include "gfxContext.h"
 #include "gfxPlatform.h"
 #include <algorithm>
@@ -253,12 +253,28 @@ nsBaseDragService::InvokeDragSession(
 
   uint32_t length = 0;
   mozilla::Unused << aTransferableArray->GetLength(&length);
-  for (uint32_t i = 0; i < length; ++i) {
-    nsCOMPtr<nsITransferable> trans = do_QueryElementAt(aTransferableArray, i);
-    if (trans) {
-      // Set the requestingPrincipal on the transferable.
+  if (!length) {
+    nsCOMPtr<nsIMutableArray> mutableArray =
+        do_QueryInterface(aTransferableArray);
+    if (mutableArray) {
+      // In order to be able trigger dnd, we need to have some transferable
+      // object.
+      nsCOMPtr<nsITransferable> trans =
+          do_CreateInstance("@mozilla.org/widget/transferable;1");
+      trans->Init(nullptr);
       trans->SetRequestingPrincipal(mSourceNode->NodePrincipal());
       trans->SetContentPolicyType(mContentPolicyType);
+      mutableArray->AppendElement(trans);
+    }
+  } else {
+    for (uint32_t i = 0; i < length; ++i) {
+      nsCOMPtr<nsITransferable> trans =
+          do_QueryElementAt(aTransferableArray, i);
+      if (trans) {
+        // Set the requestingPrincipal on the transferable.
+        trans->SetRequestingPrincipal(mSourceNode->NodePrincipal());
+        trans->SetContentPolicyType(mContentPolicyType);
+      }
     }
   }
 
@@ -864,9 +880,8 @@ bool nsBaseDragService::MaybeAddChildProcess(
 bool nsBaseDragService::RemoveAllChildProcesses() {
   for (uint32_t c = 0; c < mChildProcesses.Length(); c++) {
     mozilla::Unused << mChildProcesses[c]->SendEndDragSession(
-      true, false, LayoutDeviceIntPoint(), 0);
+        true, false, LayoutDeviceIntPoint(), 0);
   }
   mChildProcesses.Clear();
   return true;
 }
-