"WebDriver:GetCurrentURL" should not return "about:error" prefixes for erroneous navigation requests
Categories
(Remote Protocol :: Marionette, defect, P1)
Tracking
(Fission Milestone:M6c, firefox-esr68 unaffected, firefox-esr78 unaffected, firefox80 unaffected, firefox81 fixed, firefox82 fixed)
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox-esr78 | --- | unaffected |
firefox80 | --- | unaffected |
firefox81 | --- | fixed |
firefox82 | --- | fixed |
People
(Reporter: whimboo, Assigned: whimboo)
References
(Blocks 1 open bug, Regression)
Details
(Keywords: regression, Whiteboard: [marionette-fission-mvp][simple])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-release+
|
Details |
Since bug 1658928 has been fixed we return the about:error
prefix for URLs when a former navigation wasn't successful. Here an example for an invalid certificate failure:
0:10.13 pid:63104 1599568819860 Marionette DEBUG 0 -> [0,4,"WebDriver:Navigate",{"url":"https://web-platform.test:8443/webdriver/tests/support/data/test.html"}]
0:10.13 pid:63104 1599568819862 Marionette TRACE Using browsing context 22
0:10.13 pid:63104 1599568819866 Marionette TRACE [22] Received DOM event beforeunload for about:blank
0:10.15 pid:63104 1599568819883 Marionette TRACE [22] Received DOM event beforeunload for about:blank
0:10.15 pid:63104 1599568819885 Marionette TRACE [22] Received DOM event pagehide for about:blank
0:10.29 pid:63104 1599568820027 Marionette TRACE [22] Received DOM event DOMContentLoaded for about:certerror?e=nssBadCert&u=https%3A//web-platform.test%3A8443/webdriver/tests/support/data/test.html&c=UTF-8&d=%20
0:10.30 pid:63104 1599568820033 Marionette DEBUG 0 <- [1,4,{"error":"insecure certificate","message":"","stacktrace":"WebDriverError@chrome://marionette/content/error.js:175:5\nIn ... adyState@chrome://marionette/content/listener.js:295:21\nhandleEvent@chrome://marionette/content/listener.js:266:14\n"},null]
0:10.32 pid:63104 1599568820045 webdriver::server DEBUG <- 400 Bad Request {"value":{"error":"insecure certificate","message":"","stacktrace":"WebDriverError@chrome://marionette/content/error.js:175:5\nInsecureCertificateError@chrome://marionette/content/error.js:296:5\nhandleReadyState@chrome://marionette/content/listener.js:295:21\nhandleEvent@chrome://marionette/content/listener.js:266:14\n"}}
0:10.32 pid:63104 1599568820047 webdriver::server DEBUG -> GET /session/3b0cd829-c514-9440-8681-eade0e5b5686/url
0:10.32 pid:63104 1599568820052 Marionette DEBUG 0 -> [0,5,"WebDriver:GetCurrentURL",{}]
0:10.32 pid:63104 1599568820052 Marionette TRACE Using browsing context 22
0:10.32 pid:63104 1599568820052 Marionette DEBUG 0 <- [1,5,null,{"value":"about:certerror?e=nssBadCert&u=https%3A//web-platform.test%3A8443/webdriver/tests/support/data/test.html&c=UTF-8&d=%20"}]
Currently we make use of currentWindowGlobal.documentURI
that returns this URL. Given that there is no other property on this class, I wonder what's best to return the real URL. Or is that a core bug and needs a fix in Firefox?
Assignee | ||
Comment 1•4 years ago
|
||
When handling and logging the DOM events I can use event.target.location.href
, but again I don't see a way to directly get the location of the top-browsing context. Maybe we have to use the actor and get it from the child directly? At least as long as the WindowGlobalParent
doesn't support it.
Comment 2•4 years ago
|
||
location.href
is currently not reflected on WindowGlobalParent
, though it would be possible to do so. Perhaps it could be reflected as GetExposedURI
in the parent process?
Assignee | ||
Comment 3•4 years ago
|
||
Ok, so based on the conversation on Matrix we would have to use the Marionette actor for now, and retrieve the location via the actor child.
Comment hidden (obsolete) |
Comment hidden (obsolete) |
Updated•4 years ago
|
Assignee | ||
Comment 6•4 years ago
|
||
(In reply to Henrik Skupin (:whimboo) [βοΈUTC+2] from comment #3)
Ok, so based on the conversation on Matrix we would have to use the Marionette actor for now, and retrieve the location via the actor child.
This would only apply when the actors are actually in use. In case of using the framescript we would need an endpoint in listener for getting the current location.
Assignee | ||
Comment 7•4 years ago
|
||
This regressed by the patch on bug 1658928, which changed the type of
URL Marionette returns. While before we were returning the visible
URL from the location bar, the new and broken behavior includes
"about:error" prefixes for broken navigation requests.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/77a41f4b6fa0 [marionette] Don't return the current URL with "about:error" prefixes. r=marionette-reviewers,maja_zf
Comment 9•4 years ago
|
||
bugherder |
Assignee | ||
Comment 10•4 years ago
|
||
Comment on attachment 9175227 [details]
Bug 1663641 - [marionette] Don't return the current URL with "about:error" prefixes.
Beta/Release Uplift Approval Request
- User impact if declined: Since bug 1658928 landed for Firefox 81 the WebDriver command
WebDriver:GetCurrentURL
returns an URL with aabout:error
prefix for erroneous navigations like invalid certs. This majorly impacts users of geckodriver via Selenium and can cause problems in external test suites. - Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): This patch partly reverts former changes only, and makes use of the framescript again.
Here a try build with the grafted changeset:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=189607f98903f2816e8f42140025e04a6e076214
- String changes made/needed: no
Comment 11•4 years ago
|
||
Comment on attachment 9175227 [details]
Bug 1663641 - [marionette] Don't return the current URL with "about:error" prefixes.
81 is now on m-r
Comment 12•4 years ago
|
||
Comment on attachment 9175227 [details]
Bug 1663641 - [marionette] Don't return the current URL with "about:error" prefixes.
Approved for 81.0rc2.
Comment 13•4 years ago
|
||
bugherder uplift |
Updated•4 years ago
|
Updated•2 years ago
|
Description
•