1947112 - Performing actions can fail with async events enabled, throwing "JavascriptException: Cyclic object value"

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect, P2)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Originally filed as: https://github.com/mozilla/geckodriver/issues/2212

It's reproducible with the following Marionette test:

    def test(self):
        self.marionette.navigate(
            "https://support.zoom.com/login_locate_sso.do")
        elem = self.marionette.find_element(By.ID, "sso_selector_id")

        mouse_chain = self.marionette.actions.sequence("pointer", "pointer_id")
        mouse_chain \
            .pointer_move(10, 1, origin=elem)
        mouse_chain.perform()

The error in detail is:

ERROR _a/test_minimized.py TestMinimizedTestCase.test - 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:197:5
	JavaScriptError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:476:5
	cloneObject@chrome://remote/content/marionette/json.sys.mjs:46:11
	cloneJSON@chrome://remote/content/marionette/json.sys.mjs:207:12
	cloneObject@chrome://remote/content/marionette/json.sys.mjs:59:24
	cloneJSON@chrome://remote/content/marionette/json.sys.mjs:207:12
	cloneObject@chrome://remote/content/marionette/json.sys.mjs:59:24
	cloneJSON@chrome://remote/content/marionette/json.sys.mjs:207:12
	cloneObject@chrome://remote/content/marionette/json.sys.mjs:59:24
	cloneJSON@chrome://remote/content/marionette/json.sys.mjs:207:12
	json.clone@chrome://remote/content/marionette/json.sys.mjs:212:22
	receiveMessage@chrome://remote/content/marionette/actors/MarionetteCommandsChild.sys.mjs:306:19

Sadly the cycle error doesn't give more details about which object is causing the issue. When I print more details then I see:

function argumentNames(){var names=this.toString().match(/^[\s\(]*function[^(]*\(([^)]*)\)/)[1].replace(/\/\/.*?[\r\n]|\/\*(?:.|[\r\n])*?\*\//g,"").replace(/\s+/g,"").split(",");return names.length==1&&!names[0]?[]:names}

It happens when we are calling executeScript in the content process to get the element.getClientRect(). This looks like some custom method added to the DOMRect object? I need to check in more detail.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hi Donal, mind explaining why it is a wontfix for 136? As I can see we have still some weeks left for beta and an uplift should always be possible once we got it fixed. Thanks.

Comment 2

[Donal Meehan [:dmeehan]]

Hi Henrik, it's not a new issue in Fx136, without a patch, and triaged as an S3.
I've changed it to fix optional if there's a low risk patch before we are done with beta

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: testcase.html

The problem here is related to the Prototype Javascript framework the Zoom login page is making use of. It extends various elements on the page with so called extensions:

  var extensions = {
    argumentNames:       argumentNames,
    bindAsEventListener: bindAsEventListener,
    curry:               curry,
    delay:               delay,
    defer:               defer,
    wrap:                wrap,
    methodize:           methodize
  };

Those are applied to the Function prototype, but as well seem to wrap existing methods of the parent class. I tried to minimize the page and got as far as the attached testcase.html shows.

Why it's related to Perform Actions? It's happening because when we are trying to get the element rects to determine if the element is within the visible viewport we currently make use of script evaluation. This actually adds further complexity and has as well the side-effect that we are trying to serialize values including the modified Function prototype with the cyclic reference.

To prevent that it would be better to do it similarly like we already do in WebDriver BiDi and use a dedicated _getClientRects method in the Marionette commands child actor to only return the client rects array. We even already have this method but it's not used. Somehow I left in the call to executeScript.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1947112 - [remote] Include details of the value for cyclic object errors.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1947112 - [marionette] Use dedicated method to retrieve client rects for element origins of actions.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f1ad118e753
[remote] Include details of the value for cyclic object errors. r=webdriver-reviewers,jdescottes
https://hg.mozilla.org/integration/autoland/rev/b8ad667542c1
[marionette] Use dedicated method to retrieve client rects for element origins of actions. r=webdriver-reviewers,jdescottes

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/5f1ad118e753
https://hg.mozilla.org/mozilla-central/rev/b8ad667542c1

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:whimboo, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox136 to wontfix.

For more information, please visit BugBot documentation.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

There are merge conflicts for beta and given that this is most likely a rare situation and a workaround exists, let it ride the trains.