Closed Bug 1764594 Opened 2 years ago Closed 1 year ago

Trying to return a ShadowRoot from "WebDriver:ExecuteScript" causes a "cyclic object value" error

Categories

(Remote Protocol :: Marionette, defect, P1)

defect
Points:
2

Tracking

(firefox111 fixed)

RESOLVED FIXED
111 Branch
Tracking Status
firefox111 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

References

()

Details

(Whiteboard: [webdriver:m6][webdriver:relnote])

Attachments

(1 file)

We have such a test failure covered on bug 1744928 but also users report that now:
https://github.com/mozilla/geckodriver/issues/2006

The failure message is like:

  TypeError: can't access property "documentElement", node.ownerDocument is null
     # element.isInXULDocument@chrome://remote/content/marionette/element.js:1353:1
     # from@chrome://remote/content/marionette/element.js:1497:19
     # element.getElementId@chrome://remote/content/marionette/element.js:624:28
     # evaluate.toJSON@chrome://remote/content/marionette/evaluate.js:348:20
     # evaluate.toJSON@chrome://remote/content/marionette/evaluate.js:362:27
     # receiveMessage@chrome://remote/content/marionette/actors/MarionetteCommandsChild.jsm:177:31
Severity: S4 → --
Component: geckodriver → Marionette
Priority: P5 → --
Severity: -- → S3
Priority: -- → P3
Whiteboard: [webdriver:backlog]

For reference here a marionette test that replicates this issue:

    def test_shadow_dom(self):
        page = inline("""
        <style>
            custom-checkbox-element {
                display:block; width:20px; height:20px;
            }
        </style>
        <custom-checkbox-element id="checkbox"></custom-checkbox-element>
        <script>
            customElements.define('custom-checkbox-element',
                class extends HTMLElement {
                    constructor() {
                            super();
                            this.attachShadow({mode: 'open'}).innerHTML = `
                                <div><input type="checkbox"/></div>
                            `;
                        }
                });
        </script>""")
        self.marionette.navigate(page)

        self.marionette.execute_async_script("""
            const resolve = arguments[0];
            resolve(document.querySelector('custom-checkbox-element').shadowRoot);
        """, sandbox=None)

        self.marionette.execute_script("""
            return document.querySelector('custom-checkbox-element').shadowRoot;
        """, sandbox=None)

While the call to Execute Async Script works, the call to Execute Script fails. Maybe it's related to the way how we resolve the script execution including the usage of Promise in the async case?

The patch over on bug 1775064 will actually help here and will change the failure to a stale element reference error. I think that we can update this bug's summary once landed to adapt to the new behavior.

Depends on: 1775064

Updating the bug's summary because of the changes for the failure as triggered by landing the patch on bug 1775064:

1655899154939	Marionette	DEBUG	3 -> [0,3,"WebDriver:ExecuteAsyncScript",{"script":"const resolve = arguments[0];\n            resolve(document.querySelector('cus ... .shadowRoot);","args":[],"newSandbox":true,"sandbox":null,"scriptTimeout":null,"line":138,"filename":"_a/test_minimized.py"}]
1655899154944	Marionette	TRACE	[40] MarionetteCommands actor created for window id 4294967298
1655899154948	Marionette	DEBUG	3 <- [1,3,null,{"value":{"shadow-6066-11e4-a52e-4f735466cecf":"ac9863fb-73ee-4234-b075-725fcc074bf6"}}]
1655899154949	Marionette	DEBUG	3 -> [0,4,"WebDriver:ExecuteScript",{"script":"return document.querySelector('custom-checkbox-element').shadowRoot;","args":[],"newSandbox":true,"sandbox":null,"line":143,"filename":"_a/test_minimized.py"}]
1655899154951	Marionette	DEBUG	3 <- [1,4,{"error":"stale element reference","message":"The element reference of [object HTMLDocument] {\"location\":{\"href\":\"d ... tte/evaluate.js:374:27\nreceiveMessage@chrome://remote/content/marionette/actors/MarionetteCommandsChild.jsm:179:36\n"},null]
Summary: Trying to return a ShadowRoot from "WebDriver:ExecuteScript" causes a "TypeError: can't access property "documentElement", node.ownerDocument is null" → Trying to return a ShadowRoot from "WebDriver:ExecuteScript" causes a "stale element reference" error

I had a quick look and the problem here is definitely not related to our serialization to JSON! Whenever receiveMessage in MarionetteCommandsChild.jsm calls into toJSON() the value is a valid ShadowRoot instance in both cases.

The only difference is that in case of Execute Script the ShadowRoot doesn't have a containingShadowRoot meaning it's undefined, and as such the isShadowRoot() returns false.

Most likely it's related to the Promise handling code in evaluate.js where we return the result differently:
https://searchfox.org/mozilla-central/rev/44d99b6b8d84abed2d20386cc2e62cd90da41cc3/remote/marionette/evaluate.js#176-205

With bug 1793920 fixed the example as given above will actually return a cyclic object value error:

 0:02.31 TEST_END: ERROR, expected PASS - marionette_driver.errors.JavascriptException: Cyclic object value
stacktrace:
	RemoteError@chrome://remote/content/shared/RemoteError.sys.mjs:8:8
	WebDriverError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:182:5
	JavaScriptError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:361:5
	evaluate.assertAcyclic@chrome://remote/content/marionette/evaluate.sys.mjs:53:11
	evaluate.toJSON@chrome://remote/content/marionette/evaluate.sys.mjs:366:14
	receiveMessage@chrome://remote/content/marionette/actors/MarionetteCommandsChild.sys.mjs:172:36
Depends on: 1793920
Summary: Trying to return a ShadowRoot from "WebDriver:ExecuteScript" causes a "stale element reference" error → Trying to return a ShadowRoot from "WebDriver:ExecuteScript" causes a "cyclic object value" error

Lets wait for the refactoring for element serialization on bug 1692468.

Depends on: 1692468
Points: --- → 2
Priority: P3 → P2

This bug will be fixed with my patch on bug 1808894.

Status: NEW → RESOLVED
Closed: 1 year ago
Duplicate of bug: 1808894
Resolution: --- → DUPLICATE
No longer blocks: 1744928
Points: 2 → ---
Priority: P2 → --
Whiteboard: [webdriver:backlog]

Turns out that this was a side-effect of a change that I should not have done. The underlying reason here is that we should keep using the containingShadowRoot check in the isShadowRoot() helper function but this causes problems in wdspec only tests because of sandboxed script evaluation. This can be fixed when unwaiving XRays before the actual isShadowRoot() check in json.sys.mjs:

https://searchfox.org/mozilla-central/rev/e35e7107299a46a696b8aa8a4a5c03a39458ac21/remote/marionette/json.sys.mjs#122-123,132

The question now is why async script execution is not affected and if we actually may not run the code in a sandbox there?

Blocks: 1744928
Status: RESOLVED → REOPENED
Points: --- → 2
No longer duplicate of bug: 1808894
Priority: -- → P1
Resolution: DUPLICATE → ---
Whiteboard: [webdriver:m6]
Assignee: nobody → hskupin
Status: REOPENED → ASSIGNED

Without unwaiving XRays first our checks for at least isShadowRoot
will fail because "containingShadowRoot" will not be available.

Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c4c62fee694f
[marionette] Unwaive XRays for Node objects before checking for the element or shadow root type. r=webdriver-reviewers,jdescottes
Status: ASSIGNED → RESOLVED
Closed: 1 year ago1 year ago
Resolution: --- → FIXED
Target Milestone: --- → 111 Branch
Whiteboard: [webdriver:m6] → [webdriver:m6][webdriver:relnote]
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: