Don't serialize HTMLDocument as a WebElement reference
Categories
(Remote Protocol :: Marionette, defect, P1)
Tracking
(firefox108 fixed)
| Tracking | Status | |
|---|---|---|
| firefox108 | --- | fixed |
People
(Reporter: whimboo, Assigned: whimboo)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [webdriver:m5][wptsync upstream][webdriver:relnote])
Attachments
(1 file)
Whenever a WebDriver command is executed which should return a reference to the HTMLDocument it is failing with a stale element reference error. The simplest example is:
self.marionette.execute_script("return document")
That happens because our element.isStale() check has a flaw:
If el is a document then el.ownerDocument is null and we consider the element as stale. Instead of comparing the documents we should check that the element's ownerGlobal is equal to the win reference if given. Then the expected el.connected state will be returned instead.
A change here might affect a couple of currently failing tests and will make them pass.
I would like to see this bug fixed first before continuing my work on bug 1692468.
|
Assignee | |
Comment 1•2 years ago
|
||
|
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Comment 2•2 years ago
|
||
As James mentioned to me a HTMLDocument should actually not be a WebElement because the WebDriver spec only allows Node's of type 1 to be handled as such a reference. Means when returning a document it should be handled as a basic object.
I checked the code and there is actually a bug in our isDOMElement() helper:
https://searchfox.org/mozilla-central/rev/80e1bfa13c954e6450a13b3cc802d82773eba2e0/remote/marionette/element.sys.mjs#1312
This code ([ELEMENT_NODE, DOCUMENT_NODE].includes(obj.nodeType)) allows both an Element and a document to be a WebElement.
Removing this we end-up in a cyclic object value and no longer raise a stale element reference when trying to return a document via Execute Script.
|
|
Assignee | |
Updated•2 years ago
|
|
||
Updated•2 years ago
|
|
||
Comment 3•2 years ago
|
||
The severity field is not set for this bug.
:whimboo, could you have a look please?
For more information, please visit auto_nag documentation.
|
|
Assignee | |
Comment 4•2 years ago
|
||
Note that there is the following WebDriver classic spec issue that we are waiting for a consensus first:
https://github.com/w3c/webdriver/issues/1687
Pushed by hskupin@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/816cfcb6b81d [wdspec] Don't serialize HTMLDocument as a WebElement reference. r=webdriver-reviewers,jgraham
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/36619 for changes under testing/web-platform/tests
|
||
Comment 7•2 years ago
|
||
| bugherder | ||
Upstream PR merged by moz-wptsync-bot
|
|
Assignee | |
Updated•2 years ago
|
|
|
Assignee | |
Comment 9•2 years ago
|
||
We should check with consumers of Webdriver if this might cause issues once the change hits beta/release. If that's the case we will have to make modifications to the spec. See the referenced GitHub issue for that.
|
|
Assignee | |
Comment 10•1 year ago
|
||
So far we didn't receive any feedback or regression reports. So I assume that we are fine here.
|
||
Updated•1 year ago
|
Description
•