Bug 1357634 - Use 'beforeunload' to detect a possible page load. draft
authorHenrik Skupin <mail@hskupin.info>
Fri, 28 Apr 2017 10:27:12 +0200
changeset 571920 49c4c247633fe02b2f9b044f26758c7e60ed2601
parent 571272 48c0fd9c9ec5d68061ea7b59358874ae8da72572
child 626915 febeeedbf20f38425b906fd44a9cd418559bb1ed
push id56956
push userbmo:hskupin@gmail.com
push dateWed, 03 May 2017 14:02:25 +0000
bugs1357634
milestone55.0a1
Bug 1357634 - Use 'beforeunload' to detect a possible page load. Using only the different unload events to detect a page load is unreliable because of a possible delay in starting the navigation, which could be triggered by a slowness of the application. By using 'beforeunload', an event will be received much earlier, and the unload timer can be extended to a couple more seconds to wait for the navigation request to start. If a website has it's own 'beforeunload' listener registered, a tab modal dialog will be opened by Firefox, and Marionette returns from the currently active command immediately to allow the test to handle the dialog. MozReview-Commit-ID: 6ZUYtFJSSnz
testing/marionette/harness/marionette_harness/tests/unit/test_click.py
testing/marionette/harness/marionette_harness/www/beforeunload.html
testing/marionette/interaction.js
testing/marionette/listener.js
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_click.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_click.py
@@ -1,18 +1,17 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import urllib
 
-from marionette_driver.by import By
-from marionette_driver import errors
+from marionette_driver import By, errors
 
-from marionette_harness import MarionetteTestCase, run_if_e10s, skip
+from marionette_harness import MarionetteTestCase, run_if_e10s, skip_if_mobile
 
 
 def inline(doc):
     return "data:text/html;charset=utf-8,{}".format(urllib.quote(doc))
 
 
 # The <a> element in the following HTML is not interactable because it
 # is hidden by an overlay when scrolled into the top of the viewport.
@@ -87,32 +86,30 @@ class TestLegacyClick(MarionetteTestCase
               let button = document.querySelector("button");
               button.addEventListener("click", () => window.clicks++);
             </script>
         """))
         button = self.marionette.find_element(By.TAG_NAME, "button")
         button.click()
         self.assertEqual(1, self.marionette.execute_script("return window.clicks", sandbox=None))
 
-    @skip("Bug 1357634 - NoSuchElementException: Unable to locate element: username")
     def test_click_number_link(self):
         test_html = self.marionette.absolute_url("clicks.html")
         self.marionette.navigate(test_html)
         self.marionette.find_element(By.LINK_TEXT, "333333").click()
         self.marionette.find_element(By.ID, "username")
         self.assertEqual(self.marionette.title, "XHTML Test Page")
 
     def test_clicking_an_element_that_is_not_displayed_raises(self):
         test_html = self.marionette.absolute_url("hidden.html")
         self.marionette.navigate(test_html)
 
         with self.assertRaises(errors.ElementNotInteractableException):
             self.marionette.find_element(By.ID, "child").click()
 
-    @skip("Bug 1357696 - NoSuchElementException: Unable to locate element: username")
     def test_clicking_on_a_multiline_link(self):
         test_html = self.marionette.absolute_url("clicks.html")
         self.marionette.navigate(test_html)
         self.marionette.find_element(By.ID, "overflowLink").click()
         self.marionette.find_element(By.ID, "username")
         self.assertEqual(self.marionette.title, "XHTML Test Page")
 
     def test_scroll_into_view_near_end(self):
@@ -308,16 +305,28 @@ class TestClickNavigation(MarionetteTest
         except errors.NoSuchElementException:
             pass
 
     def test_click_link_page_load(self):
         self.marionette.find_element(By.LINK_TEXT, "333333").click()
         self.assertNotEqual(self.marionette.get_url(), self.test_page)
         self.assertEqual(self.marionette.title, "XHTML Test Page")
 
+    @skip_if_mobile("Bug 1325738 - Modal dialogs block execution of code for Fennec")
+    def test_click_link_page_load_aborted_by_beforeunload(self):
+        page = self.marionette.absolute_url("beforeunload.html")
+        self.marionette.navigate(page)
+        self.marionette.find_element(By.TAG_NAME, "a").click()
+
+        # click returns immediately when a beforeunload handler is invoked
+        alert = self.marionette.switch_to_alert()
+        alert.dismiss()
+
+        self.assertEqual(self.marionette.get_url(), page)
+
     def test_click_link_anchor(self):
         self.marionette.find_element(By.ID, "anchor").click()
         self.assertEqual(self.marionette.get_url(), "{}#".format(self.test_page))
 
     def test_click_link_install_addon(self):
         try:
             self.marionette.find_element(By.ID, "install-addon").click()
             self.assertEqual(self.marionette.get_url(), self.test_page)
new file mode 100644
--- /dev/null
+++ b/testing/marionette/harness/marionette_harness/www/beforeunload.html
@@ -0,0 +1,21 @@
+<!-- This Source Code Form is subject to the terms of the Mozilla Public
+   - License, v. 2.0. If a copy of the MPL was not distributed with this
+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
+
+<!DOCTYPE html>
+<html>
+<head>
+  <title>Marionette Test</title>
+  <script type="text/javascript">
+    function unload() {
+      window.onbeforeunload = null;
+      return "asdf";
+    }
+
+    window.onbeforeunload = unload;
+  </script>
+</head>
+<body>
+   <a href="empty.html">Click</a> to leave the page.<br/>
+</body>
+</html>
--- a/testing/marionette/interaction.js
+++ b/testing/marionette/interaction.js
@@ -297,17 +297,17 @@ interaction.flushEventLoop = function* (
       resolve();
     };
 
     if (win.closed) {
       resolve();
       return;
     }
 
-    win.addEventListener("beforeunload", handleEvent);
+    win.addEventListener("beforeunload", handleEvent, false);
     win.requestAnimationFrame(handleEvent);
   });
 };
 
 /**
  * Appends |path| to an <input type=file>'s file list.
  *
  * @param {HTMLInputElement} el
--- a/testing/marionette/listener.js
+++ b/testing/marionette/listener.js
@@ -140,32 +140,33 @@ var loadListener = {
    */
   start: function (command_id, timeout, startTime, waitForUnloaded = true) {
     this.command_id = command_id;
     this.timeout = timeout;
 
     this.seenUnload = false;
 
     this.timerPageLoad = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-    this.timerPageUnload = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+    this.timerPageUnload = null;
 
     // In case of a remoteness change, only wait the remaining time
     timeout = startTime + timeout - new Date().getTime();
 
     if (timeout <= 0) {
       this.notify(this.timerPageLoad);
       return;
     }
 
     if (waitForUnloaded) {
       addEventListener("hashchange", this, false);
       addEventListener("pagehide", this, false);
 
-      // The event can only be received if the listener gets added to the
-      // currently selected frame.
+      // The events can only be received when the event listeners are
+      // added to the currently selected frame.
+      curContainer.frame.addEventListener("beforeunload", this, false);
       curContainer.frame.addEventListener("unload", this, false);
 
       Services.obs.addObserver(this, "outer-window-destroyed");
     } else {
       addEventListener("DOMContentLoaded", loadListener, false);
       addEventListener("pageshow", loadListener, false);
     }
 
@@ -187,16 +188,17 @@ var loadListener = {
     removeEventListener("hashchange", this);
     removeEventListener("pagehide", this);
     removeEventListener("DOMContentLoaded", this);
     removeEventListener("pageshow", this);
 
     // If the original content window, where the navigation was triggered,
     // doesn't exist anymore, exceptions can be silently ignored.
     try {
+      curContainer.frame.removeEventListener("beforeunload", this);
       curContainer.frame.removeEventListener("unload", this);
     } catch (e if e.name == "TypeError") {}
 
     // In the case when the observer was added before a remoteness change,
     // it will no longer be available. Exceptions can be silently ignored.
     try {
       Services.obs.removeObserver(this, "outer-window-destroyed");
     } catch (e) {}
@@ -205,16 +207,30 @@ var loadListener = {
   /**
    * Callback for registered DOM events.
    */
   handleEvent: function (event) {
     let location = event.target.baseURI || event.target.location.href;
     logger.debug(`Received DOM event "${event.type}" for "${location}"`);
 
     switch (event.type) {
+      case "beforeunload":
+        if (this.timerPageUnload) {
+          // In the case when a document has a beforeunload handler registered,
+          // the currently active command will return immediately due to the
+          // modal dialog observer in proxy.js.
+          // Otherwise the timeout waiting for the document to start navigating
+          // is increased by 5000 ms to ensure a possible load event is not missed.
+          // In the common case such an event should occur pretty soon after
+          // beforeunload, and we optimise for this.
+          this.timerPageUnload.cancel();
+          this.timerPageUnload.initWithCallback(this, 5000, Ci.nsITimer.TYPE_ONE_SHOT)
+        }
+        break;
+
       case "unload":
         this.seenUnload = true;
         break;
 
       case "pagehide":
         if (event.target === curContainer.frame.document) {
           this.seenUnload = true;
 
@@ -347,16 +363,17 @@ var loadListener = {
     }).then(val => {
      if (!loadEventExpected) {
         sendOk(command_id);
         return;
       }
 
       // If requested setup a timer to detect a possible page load
       if (useUnloadTimer) {
+        this.timerPageUnload = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
         this.timerPageUnload.initWithCallback(this, 200, Ci.nsITimer.TYPE_ONE_SHOT);
       }
 
     }).catch(err => {
       if (loadEventExpected) {
         this.stop();
       }