Evaluating script fails to return value field when user prompt appears

RESOLVED FIXED in Firefox 55

Status

RESOLVED FIXED
a year ago
a year ago

People

(Reporter: ato, Assigned: ato)

Tracking

(Blocks: 1 bug)

Version 3
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

a year ago
When a user prompt, such as window.alert, is made to appear when a script is being evaluated by the GeckoDriver#execute_ function used by Execute Script and Execute Async Script, the returned JSON Object is missing the ‘value’ field.

This is because the dialogueObserver in testing/marionette/proxy.js interrupts the synchronous call to the content frame script, causing it to return with an undefined value.

This was reported in https://github.com/mozilla/geckodriver/issues/431.
(Assignee)

Updated

a year ago
Assignee: nobody → ato
Blocks: 721859
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 3

a year ago
mozreview-review
Comment on attachment 8875255 [details]
Bug 1370850 - Serialise undefined script evaluation return value to null;

https://reviewboard.mozilla.org/r/146656/#review151366

::: testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py:375
(Diff revision 2)
>              sandbox=None)
>          self.assert_is_web_element(el)
>  
> +    def test_return_value_on_alert(self):
> +        res = self.marionette._send_message("executeScript", {"script": "alert()"})
> +        self.assertIn("value", res)

Suggestion: also check that the value is None.
Attachment #8875255 - Flags: review?(mjzffr) → review+
(Assignee)

Comment 4

a year ago
mozreview-review-reply
Comment on attachment 8875255 [details]
Bug 1370850 - Serialise undefined script evaluation return value to null;

https://reviewboard.mozilla.org/r/146656/#review151366

> Suggestion: also check that the value is None.

Good point.  This was an omission from my side.
Comment hidden (mozreview-request)

Comment 6

a year ago
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/eaef7cd5e288
Serialise undefined script evaluation return value to null; r=maja_zf
Backed out for failing modified Marionette test test_execute_script.py TestExecuteContent.test_return_value_on_alert:

https://hg.mozilla.org/integration/autoland/rev/00c3f75ed9a00df54df729135cbd12651cd736b8

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=eaef7cd5e2884314fb81c70d1b9576772a38e415&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=105556932&repo=autoland

[task 2017-06-08T17:31:59.054356Z] 17:31:59     INFO - TEST-START | test_execute_script.py TestExecuteContent.test_return_value_on_alert
[task 2017-06-08T17:31:59.222102Z] 17:31:59  WARNING - Failed to gather test failure debug: 
[task 2017-06-08T17:31:59.223308Z] 17:31:59  WARNING - stacktrace:
[task 2017-06-08T17:31:59.223891Z] 17:31:59  WARNING - 	WebDriverError@chrome://marionette/content/error.js:222:5
[task 2017-06-08T17:31:59.224952Z] 17:31:59  WARNING - 	UnexpectedAlertOpenError@chrome://marionette/content/error.js:486:5
[task 2017-06-08T17:31:59.225945Z] 17:31:59  WARNING - 	assert.that/<@chrome://marionette/content/assert.js:355:13
[task 2017-06-08T17:31:59.226909Z] 17:31:59  WARNING - 	assert.noUserPrompt@chrome://marionette/content/assert.js:129:3
[task 2017-06-08T17:31:59.228003Z] 17:31:59  WARNING - 	GeckoDriver.prototype.getPageSource@chrome://marionette/content/driver.js:1036:3
[task 2017-06-08T17:31:59.228885Z] 17:31:59  WARNING - 	TaskImpl_run@resource://gre/modules/Task.jsm:321:42
[task 2017-06-08T17:31:59.229460Z] 17:31:59  WARNING - 	TaskImpl@resource://gre/modules/Task.jsm:279:3
[task 2017-06-08T17:31:59.230175Z] 17:31:59  WARNING - 	asyncFunction@resource://gre/modules/Task.jsm:254:14
[task 2017-06-08T17:31:59.230715Z] 17:31:59  WARNING - 	Task_spawn@resource://gre/modules/Task.jsm:168:12
[task 2017-06-08T17:31:59.231244Z] 17:31:59  WARNING - 	TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:391:16
[task 2017-06-08T17:31:59.231818Z] 17:31:59  WARNING - 	TaskImpl_run@resource://gre/modules/Task.jsm:329:15
[task 2017-06-08T17:31:59.232291Z] 17:31:59  WARNING - 	TaskImpl@resource://gre/modules/Task.jsm:279:3
[task 2017-06-08T17:31:59.232816Z] 17:31:59  WARNING - 	asyncFunction@resource://gre/modules/Task.jsm:254:14
[task 2017-06-08T17:31:59.233521Z] 17:31:59  WARNING - 	Task_spawn@resource://gre/modules/Task.jsm:168:12
[task 2017-06-08T17:31:59.234230Z] 17:31:59  WARNING - 	execute@chrome://marionette/content/server.js:510:15
[task 2017-06-08T17:31:59.235517Z] 17:31:59  WARNING - 	onPacket@chrome://marionette/content/server.js:481:7
[task 2017-06-08T17:31:59.236236Z] 17:31:59  WARNING - 	_onJSONObjectReady/<@chrome://marionette/content/transport.js:480:9
[task 2017-06-08T17:31:59.236972Z] 17:31:59  WARNING - 	openTabPrompt@jar:file:///home/worker/workspace/build/application/firefox/omni.ja!/components/nsPrompter.js:420:13
[task 2017-06-08T17:31:59.237699Z] 17:31:59  WARNING - 	openPrompt@jar:file:///home/worker/workspace/build/application/firefox/omni.ja!/components/nsPrompter.js:535:17
[task 2017-06-08T17:31:59.238451Z] 17:31:59  WARNING - 	alert@jar:file:///home/worker/workspace/build/application/firefox/omni.ja!/components/nsPrompter.js:602:9
[task 2017-06-08T17:31:59.239081Z] 17:31:59  WARNING - 	@dummy file:0:15
[task 2017-06-08T17:31:59.239224Z] 17:31:59  WARNING - 	@dummy file:0:2
[task 2017-06-08T17:31:59.240655Z] 17:31:59  WARNING - 	evaluate.sandbox/promise<@chrome://marionette/content/evaluate.js:154:13
[task 2017-06-08T17:31:59.241653Z] 17:31:59  WARNING - 	evaluate.sandbox@chrome://marionette/content/evaluate.js:105:17
[task 2017-06-08T17:31:59.241771Z] 17:31:59  WARNING - 	execute@chrome://marionette/content/listener.js:733:19
[task 2017-06-08T17:31:59.241899Z] 17:31:59  WARNING - 	TaskImpl_run@resource://gre/modules/Task.jsm:321:42
[task 2017-06-08T17:31:59.242525Z] 17:31:59  WARNING - 	TaskImpl@resource://gre/modules/Task.jsm:279:3
[task 2017-06-08T17:31:59.242660Z] 17:31:59  WARNING - 	asyncFunction@resource://gre/modules/Task.jsm:254:14
[task 2017-06-08T17:31:59.243199Z] 17:31:59  WARNING - 	Task_spawn@resource://gre/modules/Task.jsm:168:12
[task 2017-06-08T17:31:59.243361Z] 17:31:59  WARNING - 	TaskImpl_handleResultValue@resource://gre/modules/Task.jsm:391:16
[task 2017-06-08T17:31:59.243511Z] 17:31:59  WARNING - 	TaskImpl_run@resource://gre/modules/Task.jsm:329:15
[task 2017-06-08T17:31:59.244481Z] 17:31:59  WARNING - 	TaskImpl@resource://gre/modules/Task.jsm:279:3
[task 2017-06-08T17:31:59.244577Z] 17:31:59  WARNING - 	asyncFunction@resource://gre/modules/Task.jsm:254:14
[task 2017-06-08T17:31:59.244706Z] 17:31:59  WARNING - 	Task_spawn@resource://gre/modules/Task.jsm:168:12
[task 2017-06-08T17:31:59.245186Z] 17:31:59  WARNING - 	dispatch/<@chrome://marionette/content/listener.js:427:15
[task 2017-06-08T17:31:59.245239Z] 17:31:59  WARNING - 
[task 2017-06-08T17:31:59.419109Z] 17:31:59     INFO - TEST-UNEXPECTED-FAIL | test_execute_script.py TestExecuteContent.test_return_value_on_alert | AssertionError: {u'value': None} is not None
[task 2017-06-08T17:31:59.423178Z] 17:31:59     INFO - Traceback (most recent call last):
[task 2017-06-08T17:31:59.423275Z] 17:31:59     INFO -   File "/home/worker/workspace/build/venv/local/lib/python2.7/site-packages/marionette_harness/marionette_test/testcases.py", line 157, in run
[task 2017-06-08T17:31:59.423717Z] 17:31:59     INFO -     testMethod()
[task 2017-06-08T17:31:59.423818Z] 17:31:59     INFO -   File "/home/worker/workspace/build/tests/marionette/tests/testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py", line 363, in test_return_value_on_alert
[task 2017-06-08T17:31:59.423868Z] 17:31:59     INFO -     self.assertIsNone(res)
[task 2017-06-08T17:31:59.423909Z] 17:31:59     INFO - TEST-INFO took 168ms
Flags: needinfo?(ato)
(Assignee)

Comment 8

a year ago
The assertion should be

> self.assertIsNone(res["value"])
Flags: needinfo?(ato)
Comment hidden (mozreview-request)

Comment 10

a year ago
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fb3cbcd197fe
Serialise undefined script evaluation return value to null; r=maja_zf

Comment 11

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/fb3cbcd197fe
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Depends on: 1371905
Depends on: 1371924
You need to log in before you can comment on or make changes to this bug.