mozilla / geckodriver

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

click ignores pageLoad timeout setting when navigation takes longer than 5 seconds #1236

Open joshlandin opened 6 years ago

joshlandin commented 6 years ago

System

Testcase

Java test: https://gist.github.com/joshlandin/017faaa923f0f02063e454deaf9ea44b

Stacktrace

N/A

Trace-level log

1522188072946 webdriver::server DEBUG -> POST /session/18e21591-8e13-4f88-9b6a-b76570e7632d/timeouts {"pageLoad":50000} 1522188072948 geckodriver::marionette TRACE -> 38:[0,3,"setTimeouts",{"pageLoad":50000}] 1522188072952 Marionette TRACE 0 -> [0,3,"setTimeouts",{"pageLoad":50000}] 1522188072952 Marionette TRACE 0 <- [1,3,null,{}] 1522188072952 geckodriver::marionette TRACE <- [1,3,null,{}] 1522188072952 webdriver::server DEBUG <- 200 OK {"value": {}} . . . 1522188075054 webdriver::server DEBUG -> POST /session/18e21591-8e13-4f88-9b6a-b76570e7632d/element/26532929-bb73-4a5a-98cf-64edf84209b3/click {"id":"26532929-bb73-4a5a-98cf-64edf84209b3"} 1522188075054 geckodriver::marionette TRACE -> 68:[0,124,"clickElement",{"id":"26532929-bb73-4a5a-98cf-64edf84209b3"}] 1522188075056 Marionette TRACE 0 -> [0,124,"clickElement",{"id":"26532929-bb73-4a5a-98cf-64edf84209b3"}] 1522188075074 Marionette DEBUG Received DOM event beforeunload for http://localhost/someUrlThatSleeps?seconds=10 1522188080274 Marionette DEBUG Canceled page load listener because no navigation has been detected 1522188080276 Marionette TRACE 0 <- [1,124,null,{}] 1522188080273 geckodriver::marionette TRACE <- [1,124,null,{}] 1522188080274 webdriver::server DEBUG <- 200 OK {"value": {}}

joshlandin commented 6 years ago

Also confirmed by @lmtierney using python bindings for Selenium.

joshlandin commented 6 years ago

It's worth noting that the page load timeout is respected as long as it is under 5 seconds.

andreastt commented 6 years ago

My first thought was that this would be related to the content frame script using an outdated set of capabilities, but I fixed that in https://bugzilla.mozilla.org/show_bug.cgi?id=1379490.

Maybe @whimboo has some insight here. Are we not picking up the session page load timeout when navigating by clicking?

@joshlandin, perhaps you can check for us if this also happens if you navigate by the Navigate To command?

(A side-note is that you tested this with old versions of both geckodriver and Firefox. I don’t expect that to matter in this particular case, but it is generally advisable to report bugs against the current tip of tree.)

joshlandin commented 6 years ago

I just tested with Navigate To and the issue was not present.

this.driver.navigate().to("http://localhost/someUrlThatSleeps?seconds=10");
. . .
Took 10217 milliseconds.

@andreastt I also tested using Geckodriver 0.20.0 and FF-Nightly 61.0a1 2018-03-29 64-bit and found the same behavior.

whimboo commented 6 years ago

The problem is not that we don't respect the pageload timeout but that we cancel the load request after 5 seconds if the page has not been unloaded, which means that no unload event had been received. Checking the above test page, I can prove that.

@joshlandin, could you check via the Firefox dev tools when the unload event is being fired when you manually click that element? Also what kind of element is that? Maybe a form control?

joshlandin commented 6 years ago

@whimboo, It's just a simple <a href=""/>, nothing special. See the gist for details. As for the events, let me know if this is what you are looking for:

  <script type="text/javascript">
    $(function() {
        $("#take10").on("mousedown", function(e) {
            console.log("#take10 : mousedown.");
        });
        $("#take10").on("mouseup", function(e) {
            console.log("#take10 : mouseup.");
        });
        $("#take10").on("click", function(e) {
            console.log("#take10 : click.");
        });
        $(window).on("pageshow", function(e) {
            console.log("window : pageshow.");
        });
        $(window).on("beforeunload", function(e) {
            console.log("window : beforeunload.");
        });
        $(window).on("pagehide", function(e) {
            console.log("window : pagehide.");
        });
        $(window).on("unload", function(e) {
            console.log("window : unload.");
        });
    });
  </script>
  <a href="http://localhost/someUrlThatSleeps?seconds=10" id="take10">Takes 10 seconds</a><br/>

Console output:

16:12:01.562 Navigated to http://localhost/index.html
16:12:06.076 #take10 : mousedown.
16:12:06.153 #take10 : mouseup.
16:12:06.155 #take10 : click.
16:12:06.156 window : beforeunload.
16:12:06.234 Navigated to http://localhost/someUrlThatSleeps?seconds=10
16:12:16.258 window : pagehide.
16:12:16.260 window : unload.
whimboo commented 6 years ago
16:12:06.156 window : beforeunload.
16:12:06.234 Navigated to http://localhost/someUrlThatSleeps?seconds=10
16:12:16.258 window : pagehide.
16:12:16.260 window : unload.

Interesting. As your output from above shows the time between the click and unload event is actually 10s. I really don't understand why it takes that long for the unload event to be fired. But as it looks like it arrives when the first data comes in. I filed bug 1450876 to get this investigated.

Thank you for reporting.

andreastt commented 6 years ago

I wonder why it only happens when clicking, and not when navigating explicitly.

whimboo commented 6 years ago

No, the same also happens for navigation. So it's not click only.

yulia-dovbnia commented 5 years ago

Hello everyone! Are you going to fix this issue? Or maybe you can recommend some useful workaround? Thanks!

abasau commented 5 years ago

@yulia-dovbnia There is a workaround but it may not work in your case as it requires some particular features of automation framework. The idea is to add a beforeunload event handler that will provide indication of navigation in progress.

driver.ExecuteJavaScript("window.addEventListener('beforeunload', function () { window.__pageisreloading = true;});");

And then you check that indication when waiting for the navigation to complete.

driver.ExecuteJavaScript("return window.__pageisreloading === undefined");

Obviously your framework should allow you to run code after every navigation and you should use the same waiter during navigation. It worked perfectly in my case but it may not be feasible to implement it in your case.

whimboo commented 5 years ago

Sadly the patch for this issue is dependent on a different bug, which is kinda complicated to fix. Right now I don't have the time for that, so we don't have an ETA yet. Sorry.

Instead I would propose to use a wait().until() construct and check the document.readyState property for your expected load state (DOMContentloaded, or pageshow).

yulia-dovbnia commented 5 years ago

Hi again. Fortunately I have some time to return back to this issue. I'm really not an expert in this field. I've tried to add beforeunload before click. and added waiting for that. But this doesn't help.

I already use following waitings before click executeScript("return document.readyState").toString().equals("complete") executeScript("return jQuery.active") == 0

Bouke commented 4 years ago

Beware that polling document.readyState is not sufficient to work around this bug. When waiting for a slow server, readyState will continue to return complete while waiting for the document to be returned from the server. The readyState will only change to loading after the document has been returned and the javascript started initialising.

In C# I've implemented the following workaround, which will compare the html element. If that element has changed and the readyState is complete, then I'll consider the navigation as having completed:

internal void WaitForPageloadEvent(Action clickElement)
{
    // Normally this shouldn't be needed, but the Firefox driver has
    // an  issue that will cause it to only wait for 5 seconds.
    // See also: https://github.com/mozilla/geckodriver/issues/1236
    // Note that the suggestion of checking the readyState is not
    // sufficient, as that will ignore the wait time of the server
    // response time.
    if (driver is FirefoxDriver)
    {
        var originalDocument = driver.FindElement(By.TagName("html"));

        clickElement();

        var wait = new WebDriverWait(driver, driver.Manage().Timeouts().PageLoad);
        var javascriptExecutor = driver as IJavaScriptExecutor;
        wait.Until((_) =>
        {
            var currentDocument = driver.FindElement(By.TagName("html"));
            var isComplete = (bool)javascriptExecutor.ExecuteScript("return document.readyState == 'complete';");
            return !originalDocument.Equals(currentDocument) && isComplete;
        });
    }
    else
    {
        clickElement();
    }
}

Usage like this:

WaitForPageloadEvent(() =>
    driver.FindElement(By.CssSelector("button[type=submit]")).Click());

I've found that the IWebElement's property Id changed whenever Firefox started loading the document. This property is not available (internal), but through testing I found that .Equals() would also work. YMMV in other languages of course.