Bug 1414329 - Make WebDriver:ClickElement wait for click events r=jgraham,whimboo
authorAndreas Tolfsen <ato@sny.no>
Fri, 03 Nov 2017 19:20:46 +0000
changeset 443528 f67d247cf65228ceff08f4db458a33e2a8ef5adb
parent 443527 b90598123a5808c02651592e55602f89e8d059af
child 443529 b8a13808f775cb513b5fa8310dfcab659af2f507
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjgraham, whimboo
bugs1414329
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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