1363418 - LoadListener doesn't take redirects ("http-equiv=refresh" and "301" response) into account (affects all navigation requests)

Status: NEW

(Remote Protocol :: Marionette, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

If a page specifies the following our page load listener aborts too early and doesn't wait until the redirect has been finished:

<meta http-equiv="refresh" content="0;url=https://wiki.mozilla.org/QA/Automation_Services/Projects/Addons/MemChaser">

Here the result of a `get` request:

1494344848747	Marionette	DEBUG	Received DOM event "DOMContentLoaded" for "https://outgoing.stage.mozaws.net/v1/37c1695bfa9577678d5f0c403363c0c09fd7a3276efc3fe34d755799179cdd5c/https%3A//wiki.mozilla.org/QA/Automation_Services/Projects/Addons/MemChaser"
1494344848764	Marionette	DEBUG	Received DOM event "pageshow" for "https://outgoing.stage.mozaws.net/v1/37c1695bfa9577678d5f0c403363c0c09fd7a3276efc3fe34d755799179cdd5c/https%3A//wiki.mozilla.org/QA/Automation_Services/Projects/Addons/MemChaser"
1494344848862	Marionette	TRACE	6 <- [1,11,null,{}]

Thanks to Dave for reporting.

Comment 1

[Andreas Tolfsen ❲:ato❳]

This also needs to be fleshed out in the specification.  It would make sense to do that concurrently.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

So what's missing here is actually step 11 from the `Go` command:

https://w3c.github.io/webdriver/webdriver-spec.html#go

> If the current top-level browsing context contains a refresh state pragma directive of time 1 second or less, wait until the refresh timeout has elapsed, a new navigate has begun, and return to the first step of this algorithm. 

It means we have to add additional logic which first checks for such a meta tag, before letting the command return.

When I add a testcase I will also add one for the HTTP status redirect, which is also missing.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I would say this is a P1. Please correct if it's not true.

Comment 4

[David Burns :automatedtester]

The historic FirefoxDriver didnt do this so this isnt a P1

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Dave mentioned it as a regression for one of the test suites, maybe the AMO one:

> whimboo: here's an interesting one.. https://addons-dev.allizom.org/en-US/firefox/addon/memchaser/ click "Add-on home page". It's a redirect, but the click is returning before the redirect is fulfilled

So what I parsed from it is that it might be a regression.

I talked with David and we will discuss about this feature in our chit chat tomorrow.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Here are the following lines from the Firefox driver which handles the click + load for such a situation:

https://github.com/SeleniumHQ/selenium/blob/master/javascript/firefox-driver/js/wrappedElement.js#L63-L92
https://github.com/SeleniumHQ/selenium/blob/master/javascript/firefox-driver/js/utils.js#L966-L1017

So nothing in the code actually handles http-equiv, which means that it might be just luck that it works. 

One option could be that the webprogress listener which is used by the old driver makes that possible. But also only because in the case of the above URL the delay for the redirect is 0 seconds. So there is no pause. If there would be, I could assume that it also wouldn't work.

A workaround for now would be to wait for an element on the target page.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This might not be possible by just listening for the page load events. As such we might need bug 1664165 first which will purely make use of the WebProgressListener.