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
Comment 7•2 years ago
|
||
bugherder |
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•2 years ago
|
||
So far we didn't receive any feedback or regression reports. So I assume that we are fine here.
Updated•2 years ago
|
Description
•