Bug 1910452: Part 3 - Fix some issues with synthesizeMockDragAndDrop …

…r=m_kato This fixes four issues: 1. The test didn't provide enough movement to generate a drag session on the source before moving to the target. This meant that, when they were in different windows, Gecko wouldn't send dragleave to the source or dragenter to the target. It also never sent dragenter to the source in the first place. This remedies that. 2. dragenter and dragleave weren't properly handled because the test was sending dragleaves instead of dragexits (the latter being what Gecko expects and the former being synthesized from that -- see e.g. nsNativeDragTarget::DragLeave). This now uses dragexits and sets the proper expectations. 3. expectProtectedDataTransferAccess was needlessly complicated and, after #1, gave the wrong answers for some events like dragenter called on the source. 4. The event handler wasn't checking for exceptions and the drop handler was intentionally causing one, which was causing it to miss the rest of its execution. Differential Revision: https://phabricator.services.mozilla.com/D219550 UltraBlame original commit: f50209dd351a1d3a5d7bcc106c1a2dca07d0db5a
marco-c · Sep 16, 2024 · 52b4bae · 52b4bae
1 parent 4ec2187
commit 52b4bae
Show file tree

Hide file tree

Showing 4 changed files with 118 additions and 84 deletions.
diff --git a/testing/mochitest/tests/SimpleTest/DragChildContextBase.sys.mjs b/testing/mochitest/tests/SimpleTest/DragChildContextBase.sys.mjs
@@ -60,11 +60,6 @@ export class DragChildContextBase {
   // dataTransfer?  Set as parameter to initialize.
   expectProtectedDataTransferAccess = false;
 
-  // Should events dragend events have access to the dataTransfer?  dragend
-  // access is also subject to expectProtectedDataTransferAccess.  Set as
-  // parameter to initialize.
-  expectProtectedDataTransferAccessDragendOnly = false;
-
   window = null;
 
   dragService = null;
@@ -232,49 +227,56 @@ export class DragChildContextBase {
       sandbox
     );
 
-    if (aEv.type == "dragstart") {
-      // Add some additional data to the DataTransfer so we can look for it
-      // as we get later events.
-      this.is(
-        getFromDataTransfer(),
-        "",
-        `[${aEv.type}]| DataTransfer didn't have kTestDataTransferType`
-      );
-      setInDataTransfer();
-      this.is(
-        getFromDataTransfer(),
-        kTestDataTransferData,
-        `[${aEv.type}]| Successfully added kTestDataTransferType to DataTransfer`
-      );
-    } else if (aEv.type == "drop") {
-      this.is(
-        getFromDataTransfer(),
-        kTestDataTransferData,
-        `[${aEv.type}]| Successfully read from DataTransfer`
-      );
-      clearDataTransfer();
-      this.is(
-        getFromDataTransfer(),
-        kTestDataTransferData,
-        `[${aEv.type}]| Properly failed to write to DataTransfer`
-      );
-    } else if (
-      aEv.type == "dragenter" ||
-      aEv.type == "dragover" ||
-      aEv.type == "dragleave" ||
-      aEv.type == "dragend"
-    ) {
-      let expectProtectedDataTransferAccess =
-        this.expectProtectedDataTransferAccess ||
-        (aEv.type == "dragend" &&
-          this.expectProtectedDataTransferAccessDragendOnly);
-      this.is(
-        getFromDataTransfer(),
-        expectProtectedDataTransferAccess ? kTestDataTransferData : "",
-        `[${aEv.type}]| ${
-          expectProtectedDataTransferAccess ? "Successfully" : "Unsuccessfully"
-        } read from DataTransfer`
-      );
+    try {
+      if (aEv.type == "dragstart") {
+        // Add some additional data to the DataTransfer so we can look for it
+        // as we get later events.
+        this.is(
+          getFromDataTransfer(),
+          "",
+          `[${aEv.type}]| DataTransfer didn't have kTestDataTransferType`
+        );
+        setInDataTransfer();
+        this.is(
+          getFromDataTransfer(),
+          kTestDataTransferData,
+          `[${aEv.type}]| Successfully added kTestDataTransferType to DataTransfer`
+        );
+      } else if (aEv.type == "drop") {
+        this.is(
+          getFromDataTransfer(),
+          kTestDataTransferData,
+          `[${aEv.type}]| Successfully read from DataTransfer`
+        );
+        try {
+          clearDataTransfer();
+          this.ok(false, "Writing to DataTransfer throws an exception");
+        } catch (ex) {
+          this.ok(true, "Got exception: " + ex);
+        }
+        this.is(
+          getFromDataTransfer(),
+          kTestDataTransferData,
+          `[${aEv.type}]| Properly failed to write to DataTransfer`
+        );
+      } else if (
+        aEv.type == "dragenter" ||
+        aEv.type == "dragover" ||
+        aEv.type == "dragleave" ||
+        aEv.type == "dragend"
+      ) {
+        this.is(
+          getFromDataTransfer(),
+          this.expectProtectedDataTransferAccess ? kTestDataTransferData : "",
+          `[${aEv.type}]| ${
+            this.expectProtectedDataTransferAccess
+              ? "Successfully"
+              : "Unsuccessfully"
+          } read from DataTransfer`
+        );
+      }
+    } catch (ex) {
+      this.ok(false, "Handler did not throw an uncaught exception: " + ex);
     }
 
     if (

diff --git a/testing/mochitest/tests/SimpleTest/EventUtils.js b/testing/mochitest/tests/SimpleTest/EventUtils.js
@@ -4163,21 +4163,18 @@ async function synthesizeMockDragAndDrop(aParams) {
 
 
 
-  let expectProtectedDataTransferAccess =
-    !SpecialPowers.getBoolPref("dom.events.dataTransfer.protected.enabled") &&
-    browsingContextsAreRelated(targetBrowsingCxt, sourceBrowsingCxt);
-
-
-
-  let expectProtectedDataTransferAccessDragendOnly = !SpecialPowers.getBoolPref(
+  let expectProtectedDataTransferAccessSource = !SpecialPowers.getBoolPref(
     "dom.events.dataTransfer.protected.enabled"
   );
+  let expectProtectedDataTransferAccessTarget =
+    expectProtectedDataTransferAccessSource &&
+    browsingContextsAreRelated(targetBrowsingCxt, sourceBrowsingCxt);
 
   info(
-    `expectProtectedDataTransferAccess: ${expectProtectedDataTransferAccess}`
+    `expectProtectedDataTransferAccessSource: ${expectProtectedDataTransferAccessSource}`
   );
   info(
-    `expectProtectedDataTransferAccessDragendOnly: ${expectProtectedDataTransferAccessDragendOnly}`
+    `expectProtectedDataTransferAccessTarget: ${expectProtectedDataTransferAccessTarget}`
   );
 
 
@@ -4225,18 +4222,20 @@ async function synthesizeMockDragAndDrop(aParams) {
       expectCancelDragStart,
       expectSrcElementDisconnected,
       expectNoDragEvents,
-      expectProtectedDataTransferAccessDragendOnly,
+      expectProtectedDataTransferAccess:
+        expectProtectedDataTransferAccessSource,
       dragElementId: srcElement,
     };
     const targetVars = {
       expectDragLeave,
       expectNoDragTargetEvents,
+      expectProtectedDataTransferAccess:
+        expectProtectedDataTransferAccessTarget,
       dragElementId: targetElement,
     };
     const bothVars = {
       contextLabel,
       throwOnExtraMessage,
-      expectProtectedDataTransferAccess,
       relevantEvents: [
         "mousedown",
         "mouseup",
@@ -4376,20 +4375,20 @@ async function synthesizeMockDragAndDrop(aParams) {
 
 
 
-    info(`Moving to target element.`);
-    let currentTargetScreenPos = [
-      Math.ceil(targetPos.screenPos[0]),
-      Math.ceil(targetPos.screenPos[1]),
+    currentSrcScreenPos = [
+      currentSrcScreenPos[0] + step[0],
+      currentSrcScreenPos[1] + step[1],
     ];
-
+    info(
+      `third mousemove at ${currentSrcScreenPos[0]}, ${currentSrcScreenPos[1]}`
+    );
     dragController.sendEvent(
       sourceBrowsingCxt,
       Ci.nsIMockDragServiceController.eMouseMove,
-      currentTargetScreenPos[0],
-      currentTargetScreenPos[1]
+      currentSrcScreenPos[0],
+      currentSrcScreenPos[1]
     );
-
-    await sourceCxt.checkExpected();
+    info(`third mousemove sent`);
 
     ok(
       _getDOMWindowUtils(sourceBrowsingCxt.ownerGlobal).dragSession,
@@ -4406,26 +4405,60 @@ async function synthesizeMockDragAndDrop(aParams) {
       return;
     }
 
-    currentTargetScreenPos = [
-      currentTargetScreenPos[0] + step[0],
-      currentTargetScreenPos[1] + step[1],
-    ];
+    await sourceCxt.checkExpected();
 
 
 
 
 
+    info("synthesizing dragover inside source");
+    sourceCxt.expect("dragenter");
+    sourceCxt.expect("dragover");
+    currentSrcScreenPos = [
+      currentSrcScreenPos[0] + step[0],
+      currentSrcScreenPos[1] + step[1],
+    ];
+    info(`dragover at ${currentSrcScreenPos[0]}, ${currentSrcScreenPos[1]}`);
+    dragController.sendEvent(
+      sourceBrowsingCxt,
+      Ci.nsIMockDragServiceController.eDragOver,
+      currentSrcScreenPos[0],
+      currentSrcScreenPos[1]
+    );
+
+    info(`dragover sent`);
+    await sourceCxt.checkExpected();
+
+    let currentTargetScreenPos = [
+      Math.ceil(targetPos.screenPos[0]),
+      Math.ceil(targetPos.screenPos[1]),
+    ];
+
 
+    if (!expectNoDragTargetEvents) {
+      sourceCxt.expect("dragleave");
+    }
+
     if (
       sourceBrowsingCxt.top.embedderElement !==
       targetBrowsingCxt.top.embedderElement
     ) {
 
-      info("synthesizing dragleave and dragenter to enter new widget");
+
+
+
+
+
+
+
+      info("synthesizing dragexit and dragenter to enter new widget");
+      if (!expectNoDragTargetEvents) {
+        info("This will generate dragleave on the source");
+      }
 
       dragController.sendEvent(
         sourceBrowsingCxt,
-        Ci.nsIMockDragServiceController.eDragLeave,
+        Ci.nsIMockDragServiceController.eDragExit,
         currentTargetScreenPos[0],
         currentTargetScreenPos[1]
       );
@@ -4437,21 +4470,20 @@ async function synthesizeMockDragAndDrop(aParams) {
         currentTargetScreenPos[1]
       );
 
+      await sourceCxt.synchronize();
+
       await sourceCxt.checkExpected();
       await targetCxt.checkExpected();
     }
 
-    info("synthesizing dragover to generate dragenter in DOM");
-
+    info(
+      "Synthesizing dragover over target.  This will first generate a dragenter."
+    );
     if (!expectNoDragTargetEvents) {
       targetCxt.expect("dragenter");
       targetCxt.expect("dragover");
     }
 
-    currentTargetScreenPos = [
-      currentTargetScreenPos[0] + step[0],
-      currentTargetScreenPos[1] + step[1],
-    ];
     dragController.sendEvent(
       targetBrowsingCxt,
       Ci.nsIMockDragServiceController.eDragOver,

diff --git a/widget/MockDragServiceController.cpp b/widget/MockDragServiceController.cpp
@@ -102,8 +102,8 @@ static EventMessage MockEventTypeToEventMessage(
       return EventMessage::eDragEnter;
     case EventType::eDragOver:
       return EventMessage::eDragOver;
-    case EventType::eDragLeave:
-      return EventMessage::eDragLeave;
+    case EventType::eDragExit:
+      return EventMessage::eDragExit;
     case EventType::eDrop:
       return EventMessage::eDrop;
     case EventType::eMouseDown:
@@ -184,7 +184,7 @@ MockDragServiceController::SendEvent(
       currentDragSession->SetDragAction(nsIDragService::DRAGDROP_ACTION_MOVE);
       widget->DispatchInputEvent(widgetEvent.get());
       break;
-    case EventType::eDragLeave: {
+    case EventType::eDragExit: {
       NS_ENSURE_TRUE(currentDragSession, NS_ERROR_UNEXPECTED);
       currentDragSession->SetDragAction(nsIDragService::DRAGDROP_ACTION_MOVE);
       widget->DispatchInputEvent(widgetEvent.get());

diff --git a/widget/nsIMockDragServiceController.idl b/widget/nsIMockDragServiceController.idl
@@ -21,7 +21,7 @@ interface nsIMockDragServiceController : nsISupports
   cenum EventType : 8 {
     eDragEnter = 0,
     eDragOver = 1,
-    eDragLeave = 2,
+    eDragExit = 2,
     eDrop = 3,
     eMouseDown = 4,
     eMouseMove = 5,