diff --git a/testing/marionette/driver.js b/testing/marionette/driver.js index 8f28e0faa845..bb256ae1811a 100644 --- a/testing/marionette/driver.js +++ b/testing/marionette/driver.js @@ -2026,7 +2026,25 @@ GeckoDriver.prototype.clickElement = function* (cmd, resp) { // listen for it and then just send an error back. The person making the // call should be aware something isnt right and handle accordingly this.addFrameCloseListener("click"); - yield this.listener.clickElement(id); + + let click = this.listener.clickElement({id: id, pageTimeout: this.timeouts.pageLoad}); + + // If a remoteness update interrupts our page load, this will never return + // We need to re-issue this request to correctly poll for readyState and + // send errors. + this.curBrowser.pendingCommands.push(() => { + let parameters = { + // TODO(ato): Bug 1242595 + command_id: this.listener.activeMessageId, + pageTimeout: this.timeouts.pageLoad, + startTime: new Date().getTime(), + }; + this.mm.broadcastAsyncMessage( + "Marionette:waitForPageLoaded" + this.curBrowser.curFrameId, + parameters); + }); + + yield click; break; } }; diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_click.py b/testing/marionette/harness/marionette_harness/tests/unit/test_click.py index 7779ee9a3f36..3871cfed34ec 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_click.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_click.py @@ -8,7 +8,7 @@ from marionette_driver.by import By from marionette_driver import errors from marionette_driver.wait import Wait -from marionette_harness import MarionetteTestCase +from marionette_harness import MarionetteTestCase, run_if_e10s def inline(doc): @@ -97,11 +97,7 @@ class TestLegacyClick(MarionetteTestCase): test_html = self.marionette.absolute_url("clicks.html") self.marionette.navigate(test_html) self.marionette.find_element(By.LINK_TEXT, "333333").click() - # Bug 1335778 - Missing implicit wait for page being loaded - Wait(self.marionette, timeout=self.marionette.timeout.page_load, - ignored_exceptions=errors.NoSuchElementException).until( - lambda m: m.find_element(By.ID, "username"), - message="Username field hasn't been found") + 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): @@ -115,11 +111,7 @@ class TestLegacyClick(MarionetteTestCase): test_html = self.marionette.absolute_url("clicks.html") self.marionette.navigate(test_html) self.marionette.find_element(By.ID, "overflowLink").click() - # Bug 1335778 - Missing implicit wait for page being loaded - Wait(self.marionette, timeout=self.marionette.timeout.page_load, - ignored_exceptions=errors.NoSuchElementException).until( - lambda m: m.find_element(By.ID, "username"), - message="Username field hasn't been found") + self.marionette.find_element(By.ID, "username") self.assertEqual(self.marionette.title, "XHTML Test Page") def test_scroll_into_view_near_end(self): @@ -277,3 +269,46 @@ class TestClick(TestLegacyClick): "does not have pointer events enabled"): button.click() self.assertFalse(self.marionette.execute_script("return window.clicked", sandbox=None)) + + +class TestClickNavigation(MarionetteTestCase): + + def setUp(self): + super(TestClickNavigation, self).setUp() + + def test_click_link_page_load(self): + test_html = self.marionette.absolute_url("clicks.html") + self.marionette.navigate(test_html) + self.marionette.find_element(By.LINK_TEXT, "333333").click() + self.assertNotEqual(self.marionette.get_url(), test_html) + self.assertEqual(self.marionette.title, "XHTML Test Page") + + def test_click_link_anchor(self): + test_html = self.marionette.absolute_url("clicks.html") + self.marionette.navigate(test_html) + self.marionette.find_element(By.ID, "anchor").click() + self.assertEqual(self.marionette.get_url(), "{}#".format(test_html)) + + def test_click_no_link(self): + test_html = self.marionette.absolute_url("clicks.html") + self.marionette.navigate(test_html) + self.marionette.find_element(By.ID, "showbutton").click() + self.assertEqual(self.marionette.get_url(), test_html) + + @run_if_e10s("Requires e10s mode enabled") + def test_click_remoteness_change(self): + test_html = self.marionette.absolute_url("clicks.html") + + self.marionette.navigate("about:robots") + self.marionette.navigate(test_html) + self.marionette.find_element(By.ID, "anchor") + self.marionette.navigate("about:robots") + with self.assertRaises(errors.NoSuchElementException): + self.marionette.find_element(By.ID, "anchor") + + self.marionette.go_back() + self.marionette.find_element(By.ID, "anchor") + self.marionette.find_element(By.ID, "history-back").click() + with self.assertRaises(errors.NoSuchElementException): + self.marionette.find_element(By.ID, "anchor") + diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_modal_dialogs.py b/testing/marionette/harness/marionette_harness/tests/unit/test_modal_dialogs.py index 52020e00f016..f9a93ec611ae 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_modal_dialogs.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_modal_dialogs.py @@ -8,7 +8,7 @@ from marionette_driver import errors from marionette_driver.marionette import Alert from marionette_driver.wait import Wait -from marionette_harness import MarionetteTestCase, skip_if_e10s, WindowManagerMixin +from marionette_harness import MarionetteTestCase, WindowManagerMixin class BaseAlertTestCase(WindowManagerMixin, MarionetteTestCase): diff --git a/testing/marionette/harness/marionette_harness/www/clicks.html b/testing/marionette/harness/marionette_harness/www/clicks.html index e48cb1b2f7d6..bb2ffc931e21 100644 --- a/testing/marionette/harness/marionette_harness/www/clicks.html +++ b/testing/marionette/harness/marionette_harness/www/clicks.html @@ -17,6 +17,8 @@ I'm a normal link +Back in browser history +Forward in browser history I go to an anchor I open a window with javascript Click me diff --git a/testing/marionette/interaction.js b/testing/marionette/interaction.js index dbad95631dc8..a691576b496a 100644 --- a/testing/marionette/interaction.js +++ b/testing/marionette/interaction.js @@ -178,8 +178,8 @@ function* webdriverClickElement (el, a11y) { yield interaction.flushEventLoop(win); // step 10 - // TODO(ato): if the click causes navigation, - // run post-navigation checks + // if the click causes navigation, the post-navigation checks are + // handled by the load listener in listener.js } function* chromeClick (el, a11y) { @@ -291,20 +291,19 @@ interaction.selectOption = function (el) { * |win| has closed or been unloaded before the queue can be flushed. */ interaction.flushEventLoop = function* (win) { - let unloadEv; + return new Promise(resolve => { + let handleEvent = event => { + win.removeEventListener("beforeunload", this); + resolve(); + }; - return new Promise((resolve, reject) => { if (win.closed) { - reject(); + resolve(); return; } - unloadEv = reject; - win.addEventListener("unload", unloadEv, {once: true}); - - win.requestAnimationFrame(resolve); - }).then(() => { - win.removeEventListener("unload", unloadEv); + win.addEventListener("beforeunload", handleEvent); + win.requestAnimationFrame(handleEvent); }); }; diff --git a/testing/marionette/listener.js b/testing/marionette/listener.js index f85604944e48..a4e907b2edb9 100644 --- a/testing/marionette/listener.js +++ b/testing/marionette/listener.js @@ -117,8 +117,10 @@ var sandboxName = "default"; */ var loadListener = { command_id: null, + seenUnload: null, timeout: null, - timer: null, + timerPageLoad: null, + timerPageUnload: null, /** * Start listening for page unload/load events. @@ -136,48 +138,81 @@ var loadListener = { 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); + // In case of a remoteness change, only wait the remaining time timeout = startTime + timeout - new Date().getTime(); if (timeout <= 0) { - this.notify(); + this.notify(this.timerPageLoad); return; } - this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer); - this.timer.initWithCallback(this, timeout, Ci.nsITimer.TYPE_ONE_SHOT); - 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. + curContainer.frame.addEventListener("beforeunload", this, false); + + Services.obs.addObserver(this, "outer-window-destroyed"); } else { addEventListener("DOMContentLoaded", loadListener, false); addEventListener("pageshow", loadListener, false); } + + this.timerPageLoad.initWithCallback(this, timeout, Ci.nsITimer.TYPE_ONE_SHOT); }, /** * Stop listening for page unload/load events. */ stop: function () { - if (this.timer) { - this.timer.cancel(); - this.timer = null; + if (this.timerPageLoad) { + this.timerPageLoad.cancel(); + } + + if (this.timerPageUnload) { + this.timerPageUnload.cancel(); } 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); + } 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) {} }, /** * Callback for registered DOM events. */ handleEvent: function (event) { + logger.debug(`Received DOM event "${event.type}" for "${event.originalTarget.baseURI}"`); + switch (event.type) { + case "beforeunload": + this.seenUnload = true; + break; + case "pagehide": if (event.originalTarget === curContainer.frame.document) { + this.seenUnload = true; + removeEventListener("hashchange", this); removeEventListener("pagehide", this); @@ -223,9 +258,43 @@ var loadListener = { * Callback for navigation timeout timer. */ notify: function (timer) { - this.stop(); - sendError(new TimeoutError("Timeout loading page after " + this.timeout + "ms"), - this.command_id); + switch (timer) { + // If the page unload timer is raised, ensure to properly stop the load + // listener, and return from the currently active command. + case this.timerPageUnload: + if (!this.seenUnload) { + logger.debug("Canceled page load listener because no navigation " + + "has been detected"); + this.stop(); + sendOk(this.command_id); + } + break; + + case this.timerPageLoad: + this.stop(); + sendError(new TimeoutError(`Timeout loading page after ${this.timeout}ms`), + this.command_id); + break; + } + }, + + observe: function (subject, topic, data) { + const winID = subject.QueryInterface(Components.interfaces.nsISupportsPRUint64).data; + const curWinID = curContainer.frame.QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIDOMWindowUtils).outerWindowID; + + logger.debug(`Received observer notification "${topic}" for "${winID}"`); + + switch (topic) { + // In the case when the currently selected frame is about to close, + // there will be no further load events. Stop listening immediately. + case "outer-window-destroyed": + if (curWinID === winID) { + this.stop(); + sendOk(this.command_id); + } + break; + } }, /** @@ -251,23 +320,15 @@ var loadListener = { * @param {number} command_id * ID of the currently handled message between the driver and listener. * @param {number} pageTimeout - * Timeout in milliseconds the method has to wait for the page being finished loading. + * Timeout in milliseconds the method has to wait for the page finished loading. + * @param {boolean=} loadEventExpected + * Optional flag, which indicates that navigate has to wait for the page + * finished loading. * @param {string=} url * Optional URL, which is used to check if a page load is expected. */ - navigate: function (trigger, command_id, timeout, url = undefined) { - let loadEventExpected = true; - - if (typeof url == "string") { - try { - let requestedURL = new URL(url).toString(); - loadEventExpected = navigate.isLoadEventExpected(requestedURL); - } catch (e) { - sendError(new InvalidArgumentError("Malformed URL: " + e.message), command_id); - return; - } - } - + navigate: function (trigger, command_id, timeout, loadEventExpected = true, + useUnloadTimer = false) { if (loadEventExpected) { let startTime = new Date().getTime(); this.start(command_id, timeout, startTime, true); @@ -277,8 +338,14 @@ var loadListener = { yield trigger(); }).then(val => { - if (!loadEventExpected) { + if (!loadEventExpected) { sendOk(command_id); + return; + } + + // If requested setup a timer to detect a possible page load + if (useUnloadTimer) { + this.timerPageUnload.initWithCallback(this, 200, Ci.nsITimer.TYPE_ONE_SHOT); } }).catch(err => { @@ -286,7 +353,6 @@ var loadListener = { this.stop(); } - // Check why we do not raise an error if err is of type Event sendError(err, command_id); return; }); @@ -402,7 +468,6 @@ function removeMessageListenerId(messageName, handler) { var getTitleFn = dispatch(getTitle); var getPageSourceFn = dispatch(getPageSource); var getActiveElementFn = dispatch(getActiveElement); -var clickElementFn = dispatch(clickElement); var getElementAttributeFn = dispatch(getElementAttribute); var getElementPropertyFn = dispatch(getElementProperty); var getElementTextFn = dispatch(getElementText); @@ -457,7 +522,7 @@ function startListeners() { addMessageListenerId("Marionette:findElementContent", findElementContentFn); addMessageListenerId("Marionette:findElementsContent", findElementsContentFn); addMessageListenerId("Marionette:getActiveElement", getActiveElementFn); - addMessageListenerId("Marionette:clickElement", clickElementFn); + addMessageListenerId("Marionette:clickElement", clickElement); addMessageListenerId("Marionette:getElementAttribute", getElementAttributeFn); addMessageListenerId("Marionette:getElementProperty", getElementPropertyFn); addMessageListenerId("Marionette:getElementText", getElementTextFn); @@ -562,7 +627,7 @@ function deleteSession(msg) { removeMessageListenerId("Marionette:findElementContent", findElementContentFn); removeMessageListenerId("Marionette:findElementsContent", findElementsContentFn); removeMessageListenerId("Marionette:getActiveElement", getActiveElementFn); - removeMessageListenerId("Marionette:clickElement", clickElementFn); + removeMessageListenerId("Marionette:clickElement", clickElement); removeMessageListenerId("Marionette:getElementAttribute", getElementAttributeFn); removeMessageListenerId("Marionette:getElementProperty", getElementPropertyFn); removeMessageListenerId("Marionette:getElementText", getElementTextFn); @@ -1101,15 +1166,26 @@ function waitForPageLoaded(msg) { */ function get(msg) { let {command_id, pageTimeout, url} = msg.json; + let loadEventExpected = true; try { + if (typeof url == "string") { + try { + let requestedURL = new URL(url).toString(); + loadEventExpected = navigate.isLoadEventExpected(requestedURL); + } catch (e) { + sendError(new InvalidArgumentError("Malformed URL: " + e.message), command_id); + return; + } + } + // We need to move to the top frame before navigating sendSyncMessage("Marionette:switchedToFrame", {frameValue: null}); curContainer.frame = content; loadListener.navigate(() => { curContainer.frame.location = url; - }, command_id, pageTimeout, url); + }, command_id, pageTimeout, loadEventExpected); } catch (e) { sendError(e, command_id); @@ -1255,15 +1331,36 @@ function getActiveElement() { /** * Send click event to element. * + * @param {number} command_id + * ID of the currently handled message between the driver and listener. * @param {WebElement} id * Reference to the web element to click. + * @param {number} pageTimeout + * Timeout in milliseconds the method has to wait for the page being finished loading. */ -function clickElement(id) { - let el = seenEls.get(id, curContainer); - return interaction.clickElement( - el, - capabilities.get("moz:accessibilityChecks"), - capabilities.get("specificationLevel") >= 1); +function clickElement(msg) { + let {command_id, id, pageTimeout} = msg.json; + + try { + let loadEventExpected = true; + + let target = getElementAttribute(id, "target"); + + if (target === "_blank") { + loadEventExpected = false; + } + + loadListener.navigate(() => { + return interaction.clickElement( + seenEls.get(id, curContainer), + capabilities.get("moz:accessibilityChecks"), + capabilities.get("specificationLevel") >= 1 + ); + }, command_id, pageTimeout, loadEventExpected, true); + + } catch (e) { + sendError(e, command_id); + } } function getElementAttribute(id, name) {