Closed Bug 1793920 Opened 2 years ago Closed 2 years ago

Don't serialize HTMLDocument as a WebElement reference

Categories

(Remote Protocol :: Marionette, defect, P1)

Default
defect
Points:
2

Tracking

(firefox108 fixed)

RESOLVED FIXED
108 Branch
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:

https://searchfox.org/mozilla-central/rev/b0e929cfaf3994cad2dd02afdac138083ee3fc84/remote/marionette/element.sys.mjs#742-750

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.

Blocks: 1764594
Blocks: 1744928
Points: --- → 2

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.

Summary: "evaluate.toJSON()" always raises "stale element reference" error when trying to serialize a HTMLDocument → Don't serialize HTMLDocument as a WebElement reference
Attachment #9297430 - Attachment description: Bug 1793920 - [wdspec] Don't fail with "stale element reference" error when serializing HTMLDocument. → Bug 1793920 - [wdspec] Don't serialize HTMLDocument as a WebElement reference.

The severity field is not set for this bug.
:whimboo, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(hskupin)

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

Severity: -- → S3
Flags: needinfo?(hskupin)
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
Whiteboard: [webdriver:m5] → [webdriver:m5], [wptsync upstream]
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 108 Branch
Upstream PR merged by moz-wptsync-bot
Whiteboard: [webdriver:m5], [wptsync upstream] → [webdriver:m5][wptsync upstream][webdriver:relnote]

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.

So far we didn't receive any feedback or regression reports. So I assume that we are fine here.

Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: