1270585 - Opening modal whilst a script runs causes WebDriver:ExecuteScript/ExecuteAsyncScript to return immediately with a null result

Status: RESOLVED WORKSFORME

(Remote Protocol :: Marionette, defect, P2)

Description

[James Graham [:jgraham]]

In [8]: import marionette
     
    In [9]: m = marionette.Marionette()
     
    In [10]: m.start_session()
    Out[10]:
    {u'XULappId': u'{ec8030f7-c20a-464f-9b0e-13a3a9e97384}',
     u'acceptSslCerts': False,
     u'appBuildId': u'20160504043118',
     u'browserName': u'Firefox',
     u'browserVersion': u'49.0a1',
     u'command_id': 1,
     u'device': u'desktop',
     u'platform': u'LINUX',
     u'platformName': u'Linux',
     u'platformVersion': u'3.13.0-34-generic',
     u'proxy': {},
     u'raisesAccessibilityExceptions': False,
     u'rotatable': False,
     u'specificationLevel': 0,
     u'takesElementScreenshot': True,
     u'takesScreenshot': True,
     u'version': u'49.0a1'}
     
    In [11]: m.execute_script("return 1")
    Out[11]: 1
     
    In [12]: m.execute_script("alert(1); return 1") 

This isn't clearly the right thing to do, for example it would break a test that opened a window which spawned an alert, and then navigated that window [1]

[1] https://fiddle.jshell.net/wqansk7c/7/?utm_source=website&utm_medium=embed&utm_campaign=wqansk7c

Comment 2

[Andreas Tolfsen ❲:ato❳]

This could be a conformance problem.

As Jim Evans pointed out to me in #webdriver, there is also a likely
specification change necessary here to figure out what the expected
behaviour is meant to be.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

The current WebDriver spec states:

> If at any point during the algorithm a user prompt appears, abort all subsequent substeps of this algorithm, and return success with data null.

So isn't it the behavior we currently have? The example of comment 0 only references an alert, but not a different window. Also the provided link is a 404 now.

Andreas, what do you think?

Comment 4

[Andreas Tolfsen ❲:ato❳]

I just tried this with the Python client and it appears we return
immediately without any errors:

> >>> m.navigate("https://sny.no")
> >>> m.execute_script("alert('hi')")

So I would say we are doing the expected thing here.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Actually based on my investigation and needinfo request on bug 1459118, this actually seems to be relevant and currently unclear in the spec. Means we indeed may return too early.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

So we discussed this issue in our WebDriver Meeting (https://wiki.mozilla.org/WebDriver/Meetings/2018-07-30) today, and agreed that this is conformant to the latest WebDriver spec. Any appearing user prompt should has to immediately stop the script execution.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

There is more discussion in the #webdriver channel based on our outcome and as such https://github.com/w3c/webdriver/issues/1153 has been reopened. Which means that this bug is not yet done. :/

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

I'm going to update the spec later today so that we mention something like (got that from James):

If at any point during the algorithm a user prompt appears, immediately return Completion { [[Type]]: normal, [[Value]]: null, [[Target]]: empty }, but continue to run the other steps of this algorithm in parallel.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

Oh, the proposed content is actually already present in the spec. See the following section's first paragraph:
https://w3c.github.io/webdriver/#dfn-execute-a-function-body

So I assume there is nothing to be done here?

Comment 10

[James Graham [:jgraham]]

It says to return, but it also says to abort all remaining substeps. That doesn't seem right; once the script completes I assume we still need to run the cleanup steps? Getting the detail exactly right here is probably complicated, but we could at least do better to inform people about what the expectations are.

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

PR has been created: https://github.com/w3c/webdriver/pull/1572

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2] (away 02/17 - 02/21)]

The PR has been merged.