mozilla / geckodriver

WebDriver for Firefox
https://firefox-source-docs.mozilla.org/testing/geckodriver/
Mozilla Public License 2.0
7.03k stars 1.51k forks source link

Regression in confirmation dialog opened with onbeforeunload since Firefox 124.0 #2182

Closed surli closed 1 week ago

surli commented 1 week ago

The bug was originally reported there: https://github.com/SeleniumHQ/docker-selenium/issues/2215 Basically when using the docker image for Firefox 123.0 I was able to properly intercept any confirmation dialog triggered by a onbeforeunload javascript call, which was allowing us to perform some validations of our UI. Since Firefox 124.0 however, all those calls seems to not work anymore.

According to @diemol in https://github.com/SeleniumHQ/docker-selenium/issues/2215#issuecomment-2168192421 this is not a selenium issue and should be reported, hence this ticket.

System

Testcase

The issue can be reproduced with the following selenium snippet:


from selenium import webdriver
from selenium.webdriver.support.ui import WebDriverWait
from selenium.webdriver.support import expected_conditions as EC
from selenium.webdriver.common.by import By
import time

def open_url():
    driver = webdriver.Firefox()
    driver.get("https://www.selenium.dev/blog/2024/selenium-4-21-released/")
    driver.execute_script("window.onbeforeunload = function () { return false; }")
    link = driver.find_element(By.XPATH, "//a[text()='@diemol']")
    link.click()
    time.sleep(20)

    alert = WebDriverWait(driver, 3).until(EC.alert_is_present())
    alert.accept()
    driver.quit()

if __name__ == "__main__":
    open_url()

Stacktrace

Trace-level log

whimboo commented 1 week ago

Please note that in Firefox 124 we shipped support for handling the beforeunload prompt correctly. As per specification the prompt should always be automatically accepted. Up until Firefox 123 this kind of prompt was completely disabled by default. When you are saying that you were able to handle the dialog did you reset the preference dom.disable_beforeunload to false? You should not have seen this dialog at all.

surli commented 1 week ago

Thanks for the answer, though I'm not sure to understand it completely.

As per specification the prompt should always be automatically accepted.

You mean that the prompt should never be displayed even for an end user using Firefox? Or you're talking about the specification for using Firefox in an automated way only (sorry I'm not entirely familiar with your project)?

When you are saying that you were able to handle the dialog did you reset the preference dom.disable_beforeunload to false?

Yes we do set this property to false for executing our tests. I actually tried to remove this setting from our test framework, but AFAIR it didn't change the outcome.

Note that in our case it's important that we are able to test that this confirmation dialog is displayed for our end users: we want to ensure that they won't leave an edition page before saving without having to confirm it. And we're doing this test in an automated way for a long time now, so it's really an important regression for us to lose that.

whimboo commented 1 week ago

As per specification the prompt should always be automatically accepted.

You mean that the prompt should never be displayed even for an end user using Firefox? Or you're talking about the specification for using Firefox in an automated way only (sorry I'm not entirely familiar with your project)?

Sorry when it as a bit unclear. When we speak about geckodriver and testing via WebDriver classic then I reference the https://w3c.github.io/webdriver/ specification. Given the limitations of WebDriver classic it is not possible to correctly handle the beforeunload prompts. As such the specification defines that these kind of prompts are automatically accepted to not block any navigation. See the 3rd paragraph here.

This doesn't apply to normal users of Firefox for sure.

When you are saying that you were able to handle the dialog did you reset the preference dom.disable_beforeunload to false?

Yes we do set this property to false for executing our tests. I actually tried to remove this setting from our test framework, but AFAIR it didn't change the outcome.

Ok, so by changing this preference value you entered an unsupported mode, which by coincidence worked in your case. But only because our implementation for this type of prompt was not complete and also didn't conform to the specification. This has now been corrected with the Firefox 124 release.

Note that in our case it's important that we are able to test that this confirmation dialog is displayed for our end users: we want to ensure that they won't leave an edition page before saving without having to confirm it. And we're doing this test in an automated way for a long time now, so it's really an important regression for us to lose that.

Given that you are using Selenium for testing, you will need to update your tests to utilize the WebDriver BiDi upgrade path. Here are the steps you can follow to get it working again:

  1. Make sure that the webSocketUrl capability gets set to true which requests a WebDriver BiDi session.
  2. Connect to the BiDi session via a method as offered by your Selenium binding.

Now with an active BiDi session the beforeunload prompt should no longer be accepted automatically, and your current approach should work again. The only downside here is that you are required to use Firefox 129, which is currently only available as Nightly build.

Please let us know if this solution works for you or if you encounter any issues. Additionally, while I may not be aware of all the current limitations in Selenium, @diemol can potentially provide further assistance and insights.

surli commented 1 week ago

Thanks for all the details, it's very clear now. Regarding Firefox 129, it's not ideal but AFAICS it should be released in august, so it means we only have 2 or 3 months to wait: our support strategy indicates we're supposed to test on latest released version of Firefox.

I opened a ticket on our side so that we start investigating moving to use BiDi in our test framework which relies on Selenium. I hope we won't get too much regressions by doing the move: AFAIU not all features are yet supported in BiDi, I saw this dashboard https://wpt.fyi/results/webdriver/tests/bidi?label=experimental&label=master&aligned&view=subtest but I don't know if there's another doc about the current status?

whimboo commented 1 week ago

If you start with BiDi please let us know which features are missing. A good matrix of commands and events that we are going to implement or already have implemented can be found in this document. At least for Firefox and the web-platform sections I keep it up to date.

Edit: And there is the overall WebDriver BiDi feature list as well. Maybe one of the missing features that you need is/are already listed there.

surli commented 1 week ago

Perfect, thanks for the pointers. We'll try to move on the subject in a branch and to see how much tests are broken, I'll keep you posted if we see missing features, but honestly we don't use much advanced stuff I think so I hope it should be ok.

whimboo commented 1 week ago

Sounds good.

I'm going to close this issue now given that it is not actually a regression but an expected change in behavior when beforeunload prompts were enabled by the preference.