From 19cc83e98a96f3b34399b66768c85f10d3e8f2e2 Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Sat, 10 Feb 2024 16:46:33 +0000 Subject: [PATCH] Bug 1693857 - [marionette] Implicitly accept "beforeunload" prompts in Marionette. r=webdriver-reviewers,jdescottes Differential Revision: https://phabricator.services.mozilla.com/D198681 --- remote/components/Marionette.sys.mjs | 9 +--- remote/marionette/driver.sys.mjs | 27 ++++++++-- remote/marionette/navigate.sys.mjs | 22 +++++--- .../client/marionette_driver/geckoinstance.py | 2 - .../tests/unit/test_click.py | 23 --------- .../tests/unit/test_navigation.py | 50 +------------------ .../tests/unit/test_window_close_content.py | 20 -------- testing/profiles/web-platform/user.js | 2 - .../wptrunner/browsers/firefox_android.py | 4 ++ 9 files changed, 44 insertions(+), 115 deletions(-) diff --git a/remote/components/Marionette.sys.mjs b/remote/components/Marionette.sys.mjs index d4ce620cac27..87a53679e407 100644 --- a/remote/components/Marionette.sys.mjs +++ b/remote/components/Marionette.sys.mjs @@ -40,13 +40,6 @@ const ENV_ENABLED = "MOZ_MARIONETTE"; // pref being set to 4444. const ENV_PRESERVE_PREFS = "MOZ_MARIONETTE_PREF_STATE_ACROSS_RESTARTS"; -// Map of Marionette-specific preferences that should be set via -// RecommendedPreferences. -const RECOMMENDED_PREFS = new Map([ - // Automatically unload beforeunload alerts - ["dom.disable_beforeunload", true], -]); - const isRemote = Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT; @@ -147,7 +140,7 @@ class MarionetteParentProcess { Services.obs.addObserver(this, "domwindowopened"); } - lazy.RecommendedPreferences.applyPreferences(RECOMMENDED_PREFS); + lazy.RecommendedPreferences.applyPreferences(); // Only set preferences to preserve in a new profile // when Marionette is enabled. diff --git a/remote/marionette/driver.sys.mjs b/remote/marionette/driver.sys.mjs index d15a9b09fe78..ad59e0ccf57e 100644 --- a/remote/marionette/driver.sys.mjs +++ b/remote/marionette/driver.sys.mjs @@ -114,6 +114,9 @@ export function GeckoDriver(server) { // WebDriver Session this._currentSession = null; + // Flag to indicate that the application is shutting down + this._isShuttingDown = false; + this.browsers = {}; // points to current browser @@ -211,7 +214,16 @@ GeckoDriver.prototype.handleClosedModalDialog = function () { */ GeckoDriver.prototype.handleOpenModalDialog = function (eventName, data) { this.dialog = data.prompt; - this.getActor().notifyDialogOpened(); + + if (this.dialog.promptType === "beforeunload") { + lazy.logger.trace(`Implicitly accepted "beforeunload" prompt`); + this.dialog.accept(); + return; + } + + if (!this._isShuttingDown) { + this.getActor().notifyDialogOpened(this.dialog); + } }; /** @@ -2375,7 +2387,9 @@ GeckoDriver.prototype.deleteSession = function () { // reset to the top-most frame this.mainFrame = null; - if (this.promptListener) { + if (!this._isShuttingDown && this.promptListener) { + // Do not stop the prompt listener when quitting the browser to + // allow us to also accept beforeunload prompts during shutdown. this.promptListener.stopListening(); this.promptListener = null; } @@ -2792,6 +2806,12 @@ GeckoDriver.prototype._handleUserPrompts = async function () { return; } + if (this.dialog.promptType == "beforeunload") { + // Wait until the "beforeunload" prompt has been accepted. + await this.promptListener.dialogClosed(); + return; + } + const textContent = await this.dialog.getText(); const behavior = this.currentSession.unhandledPromptBehavior; @@ -2899,16 +2919,17 @@ GeckoDriver.prototype.quit = async function (cmd) { let quitApplicationResponse; try { + this._isShuttingDown = true; quitApplicationResponse = await lazy.quit( flags, safeMode, this.currentSession.capabilities.get("moz:windowless") ); } catch (e) { + this._isShuttingDown = false; if (e instanceof TypeError) { throw new lazy.error.InvalidArgumentError(e.message); } - throw new lazy.error.UnsupportedOperationError(e.message); } finally { Services.obs.removeObserver(this, TOPIC_QUIT_APPLICATION_REQUESTED); diff --git a/remote/marionette/navigate.sys.mjs b/remote/marionette/navigate.sys.mjs index 4e46d8f7ba2b..993ca75cf800 100644 --- a/remote/marionette/navigate.sys.mjs +++ b/remote/marionette/navigate.sys.mjs @@ -255,20 +255,26 @@ navigate.waitForNavigationCompleted = async function waitForNavigationCompleted( } }; - const onPromptOpened = action => { + const onPromptOpened = (_, data) => { + if (data.prompt.promptType === "beforeunload") { + // Ignore beforeunload prompts which are handled by the driver class. + return; + } + lazy.logger.trace("Canceled page load listener because a dialog opened"); checkDone({ finished: true }); }; const onTimer = timer => { - // In the case when a document has a beforeunload handler - // registered, the currently active command will return immediately - // due to the modal dialog observer. + // For the command "Element Click" we want to detect a potential navigation + // as early as possible. The `beforeunload` event is an indication for that + // but could still cause the navigation to get aborted by the user. As such + // wait a bit longer for the `unload` event to happen, which usually will + // occur pretty soon after `beforeunload`. // - // 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. + // Note that with WebDriver BiDi enabled the `beforeunload` prompts might + // not get implicitly accepted, so lets keep the timer around until we know + // that it is really not required. if (seenBeforeUnload) { seenBeforeUnload = false; unloadTimer.initWithCallback( diff --git a/testing/marionette/client/marionette_driver/geckoinstance.py b/testing/marionette/client/marionette_driver/geckoinstance.py index 6fdbc23d5e2a..b0bec22ea08f 100644 --- a/testing/marionette/client/marionette_driver/geckoinstance.py +++ b/testing/marionette/client/marionette_driver/geckoinstance.py @@ -60,8 +60,6 @@ class GeckoInstance(object): # Do not show datareporting policy notifications which can interfere with tests "datareporting.policy.dataSubmissionEnabled": False, "datareporting.policy.dataSubmissionPolicyBypassNotification": True, - # Automatically unload beforeunload alerts - "dom.disable_beforeunload": True, # Enabling the support for File object creation in the content process. "dom.file.createInChild": True, # Disable delayed user input event handling 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 5546f114f46e..5936be1e696d 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_click.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_click.py @@ -505,29 +505,6 @@ class TestClickNavigation(WindowManagerMixin, MarionetteTestCase): self.assertNotEqual(self.marionette.get_url(), self.test_page) self.assertEqual(self.marionette.title, "Marionette Test") - def test_click_link_page_load_dismissed_beforeunload_prompt(self): - self.marionette.navigate( - inline( - """ - - Click - - """.format( - self.marionette.absolute_url("clicks.html") - ) - ) - ) - self.marionette.find_element(By.TAG_NAME, "input").send_keys("foo") - self.marionette.find_element(By.TAG_NAME, "a").click() - - # navigation auto-dismisses beforeunload prompt - with self.assertRaises(errors.NoAlertPresentException): - Alert(self.marionette).text - def test_click_link_anchor(self): self.marionette.find_element(By.ID, "anchor").click() self.assertEqual(self.marionette.get_url(), "{}#".format(self.test_page)) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py b/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py index 086374cc867d..ec1a8f1be08a 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_navigation.py @@ -385,12 +385,7 @@ class TestBackForwardNavigation(BaseNavigationTestCase): def check_page_status(page, expected_history_length): if "alert_text" in page: - if page["alert_text"] is None: - # navigation auto-dismisses beforeunload prompt - with self.assertRaises(errors.NoAlertPresentException): - Alert(self.marionette).text - else: - self.assertEqual(Alert(self.marionette).text, page["alert_text"]) + self.assertEqual(Alert(self.marionette).text, page["alert_text"]) self.assertEqual(self.marionette.get_url(), page["url"]) self.assertEqual(self.history_length, expected_history_length) @@ -442,29 +437,6 @@ class TestBackForwardNavigation(BaseNavigationTestCase): self.marionette.go_back() self.marionette.go_forward() - def test_dismissed_beforeunload_prompt(self): - url_beforeunload = inline( - """ - - - """ - ) - - def modify_page(): - self.marionette.find_element(By.TAG_NAME, "input").send_keys("foo") - - test_pages = [ - {"url": inline("

foobar

"), "alert_text": None}, - {"url": url_beforeunload, "callback": modify_page}, - {"url": inline("

foobar

"), "alert_text": None}, - ] - - self.run_bfcache_test(test_pages) - def test_data_urls(self): test_pages = [ {"url": inline("

foobar

")}, @@ -686,26 +658,6 @@ class TestRefresh(BaseNavigationTestCase): self.marionette.refresh() self.assertEqual(self.test_page_file_url, self.marionette.get_url()) - def test_dismissed_beforeunload_prompt(self): - self.marionette.navigate( - inline( - """ - - - """ - ) - ) - self.marionette.find_element(By.TAG_NAME, "input").send_keys("foo") - self.marionette.refresh() - - # navigation auto-dismisses beforeunload prompt - with self.assertRaises(errors.NoAlertPresentException): - Alert(self.marionette).text - def test_image(self): image = self.marionette.absolute_url("black.png") diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_content.py b/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_content.py index b38246f63bfc..fe883baf6bd9 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_content.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_window_close_content.py @@ -59,26 +59,6 @@ class TestCloseWindow(WindowManagerMixin, MarionetteTestCase): self.assertNotIn(new_tab, window_handles) self.assertListEqual(self.start_tabs, window_handles) - def test_close_window_with_dismissed_beforeunload_prompt(self): - new_tab = self.open_tab() - self.marionette.switch_to_window(new_tab) - - self.marionette.navigate( - inline( - """ - - - """ - ) - ) - - self.marionette.find_element(By.TAG_NAME, "input").send_keys("foo") - self.marionette.close() - def test_close_window_for_browser_window_with_single_tab(self): new_tab = self.open_window() self.marionette.switch_to_window(new_tab) diff --git a/testing/profiles/web-platform/user.js b/testing/profiles/web-platform/user.js index 5a3c36ae38a5..6370b93645c3 100644 --- a/testing/profiles/web-platform/user.js +++ b/testing/profiles/web-platform/user.js @@ -47,8 +47,6 @@ user_pref("browser.safebrowsing.downloads.enabled", false); user_pref("browser.safebrowsing.malware.enabled", false); user_pref("browser.safebrowsing.phishing.enabled", false); user_pref("browser.safebrowsing.update.enabled", false); -// Automatically unload beforeunload alerts -user_pref("dom.disable_beforeunload", true); // Disable high DPI user_pref("layout.css.devPixelsPerPx", "1.0"); // Enable the parallel styling code. diff --git a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/firefox_android.py b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/firefox_android.py index aa9cf6332329..3ce3b11d1f24 100644 --- a/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/firefox_android.py +++ b/testing/web-platform/tests/tools/wptrunner/wptrunner/browsers/firefox_android.py @@ -161,6 +161,10 @@ class ProfileCreator(FirefoxProfileCreator): "dom.send_after_paint_to_content": True, }) + if self.package_name == "org.mozilla.geckoview.test_runner": + # Bug 1879324: The TestRunner doesn't support "beforeunload" prompts yet + profile.set_preferences({"dom.disable_beforeunload": True}) + if self.test_type == "reftest": self.logger.info("Setting android reftest preferences") profile.set_preferences({