Bug 1414329 - Make WebDriver:ClickElement wait for click events r?jgraham,whimboo draft
authorAndreas Tolfsen <ato@sny.no>
Fri, 03 Nov 2017 19:20:46 +0000
changeset 697820 cf8f9427a85c9189ebc81ce2d5a02a408494b4f3
parent 697817 934fe0bd60626cd4d8693b2a6788205d7605fa34
child 740216 c0b01b02d0c51b7f664255199a16a3afd1b45397
push id89106
push userbmo:ato@sny.no
push dateTue, 14 Nov 2017 19:07:35 +0000
reviewersjgraham, whimboo
bugs1414329
milestone59.0a1
Bug 1414329 - Make WebDriver:ClickElement wait for click events r?jgraham,whimboo This changes interaction.flushEventLoop from relying on the beforeunload DOM event to determine when to bail in case the document navigates. Instead, it now relies on the unload event which should not affect bfcache. It also changes flushEventLoop to append a click event listener to the targetted element's container element. Because this event will be added last, the intention is that all preceding click evens will have had time to fire and propagate before our listener spins the event loop through setTimeout once. Our listeners are added using the privileged mozSystemGroup option so that a potential invocation of EventTarget.stopImmediatePropagation does not prevent our listener from firing. MozReview-Commit-ID: 2Ehxwg2p6Fo
testing/marionette/action.js
testing/marionette/harness/marionette_harness/tests/unit/test_click.py
testing/marionette/interaction.js
--- a/testing/marionette/action.js
+++ b/testing/marionette/action.js
@@ -12,17 +12,16 @@ Cu.import("chrome://marionette/content/a
 const {element} = Cu.import("chrome://marionette/content/element.js", {});
 const {
   InvalidArgumentError,
   MoveTargetOutOfBoundsError,
   UnsupportedOperationError,
 } = Cu.import("chrome://marionette/content/error.js", {});
 Cu.import("chrome://marionette/content/event.js");
 const {pprint} = Cu.import("chrome://marionette/content/format.js", {});
-Cu.import("chrome://marionette/content/interaction.js");
 
 this.EXPORTED_SYMBOLS = ["action"];
 
 // TODO? With ES 2016 and Symbol you can make a safer approximation
 // to an enum e.g. https://gist.github.com/xmlking/e86e4f15ec32b12c4689
 /**
  * Implements WebDriver Actions API: a low-level interface for providing
  * virtualised device input to the web browser.
@@ -1007,18 +1006,17 @@ action.dispatch = function(chain, window
  *     Current window global.
  *
  * @return {Promise}
  *     Promise for dispatching all tick-actions and pending DOM events.
  */
 action.dispatchTickActions = function(
     tickActions, tickDuration, window) {
   let pendingEvents = tickActions.map(toEvents(tickDuration, window));
-  return Promise.all(pendingEvents).then(
-      () => interaction.flushEventLoop(window));
+  return Promise.all(pendingEvents);
 };
 
 /**
  * Compute tick duration in milliseconds for a collection of actions.
  *
  * @param {Array.<action.Action>} tickActions
  *     List of actions for one tick.
  *
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_click.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_click.py
@@ -313,16 +313,51 @@ class TestClick(TestLegacyClick):
         button = self.marionette.find_element(By.TAG_NAME, "button")
         self.assertEqual("none", button.value_of_css_property("pointer-events"))
 
         with self.assertRaisesRegexp(errors.ElementClickInterceptedException,
                                      "does not have pointer events enabled"):
             button.click()
         self.assertFalse(self.marionette.execute_script("return window.clicked", sandbox=None))
 
+    def test_preventDefault(self):
+        self.marionette.navigate(inline("""
+            <button>click me</button>
+            <script>
+              let button = document.querySelector("button");
+              button.addEventListener("click", event => event.preventDefault());
+            </script>
+        """))
+        button = self.marionette.find_element(By.TAG_NAME, "button")
+        # should not time out
+        button.click()
+
+    def test_stopPropagation(self):
+        self.marionette.navigate(inline("""
+            <button>click me</button>
+            <script>
+              let button = document.querySelector("button");
+              button.addEventListener("click", event => event.stopPropagation());
+            </script>
+        """))
+        button = self.marionette.find_element(By.TAG_NAME, "button")
+        # should not time out
+        button.click()
+
+    def test_stopImmediatePropagation(self):
+        self.marionette.navigate(inline("""
+            <button>click me</button>
+            <script>
+              let button = document.querySelector("button");
+              button.addEventListener("click", event => event.stopImmediatePropagation());
+            </script>
+        """))
+        button = self.marionette.find_element(By.TAG_NAME, "button")
+        # should not time out
+        button.click()
 
 
 class TestClickNavigation(MarionetteTestCase):
 
     def setUp(self):
         super(TestClickNavigation, self).setUp()
 
         self.test_page = self.marionette.absolute_url("clicks.html")
--- a/testing/marionette/interaction.js
+++ b/testing/marionette/interaction.js
@@ -172,22 +172,22 @@ async function webdriverClickElement(el,
   a11y.assertVisible(acc, el, true);
   a11y.assertEnabled(acc, el, true);
   a11y.assertActionable(acc, el);
 
   // step 8
   if (el.localName == "option") {
     interaction.selectOption(el);
   } else {
+    // step 9
+    let clicked = interaction.flushEventLoop(containerEl);
     event.synthesizeMouseAtPoint(clickPoint.x, clickPoint.y, {}, win);
+    await clicked;
   }
 
-  // step 9
-  await interaction.flushEventLoop(win);
-
   // step 10
   // if the click causes navigation, the post-navigation checks are
   // handled by the load listener in listener.js
 }
 
 async function chromeClick(el, a11y) {
   if (!atom.isElementEnabled(el)) {
     throw new InvalidElementStateError("Element is not enabled");
@@ -282,46 +282,47 @@ interaction.selectOption = function(el) 
   }
 
   event.change(containerEl);
   event.mouseup(containerEl);
   event.click(containerEl);
 };
 
 /**
- * Flushes the event loop by requesting an animation frame.
- *
- * This will wait for the browser to repaint before returning, typically
- * flushing any queued events.
+ * Waits until the event loop has spun enough times to process the
+ * DOM events generated by clicking an element, or until the document
+ * is unloaded.
  *
- * If the document is unloaded during this request, the promise is
- * rejected.
- *
- * @param {Window} win
- *     Associated window.
+ * @param {Element} el
+ *     Element that is expected to receive the click.
  *
  * @return {Promise}
- *     Promise is accepted once event queue is flushed, or rejected if
- *     <var>win</var> has closed or been unloaded before the queue can
- *     be flushed.
+ *     Promise is resolved once <var>el</var> has been clicked
+ *     (its <code>click</code> event fires) or the document is unloaded.
  */
-interaction.flushEventLoop = async function(win) {
+interaction.flushEventLoop = async function(el) {
+  const win = el.ownerGlobal;
+  let unloadEv, clickEv;
+
   return new Promise(resolve => {
-    let handleEvent = () => {
-      win.removeEventListener("beforeunload", this);
-      resolve();
+    unloadEv = resolve;
+    clickEv = () => {
+      if (win.closed) {
+        resolve();
+      } else {
+        win.setTimeout(resolve, 0);
+      }
     };
 
-    if (win.closed) {
-      resolve();
-      return;
-    }
-
-    win.addEventListener("beforeunload", handleEvent);
-    win.requestAnimationFrame(handleEvent);
+    win.addEventListener("unload", unloadEv, {mozSystemGroup: true});
+    el.addEventListener("click", clickEv, {mozSystemGroup: true});
+  }).then(() => {
+    // only one event fires
+    win.removeEventListener("unload", unloadEv);
+    el.removeEventListener("click", clickEv);
   });
 };
 
 /**
  * Appends <var>path</var> to an <tt>&lt;input type=file&gt;</tt>'s
  * file list.
  *
  * @param {HTMLInputElement} el