Run JSON.stringify on return value from Execute (Async) Script

RESOLVED FIXED in Firefox 55

Status

enhancement
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ato, Assigned: ato)

Tracking

(Blocks 1 bug)

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

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(3 attachments)

Assignee

Description

2 years ago
The WebDriver specification recently allowed an object’s toJSON to represent the return value from the Execute Script and Execute Async Script commands.

The change was made in https://github.com/w3c/webdriver/commit/1ee4c61c11004f38bb3db0fb6a1d740e185a2196.

We should change Marionette so that it calls JSON.stringify, which implicitly calls obj.toJSON, on the return value from these commands.
Assignee

Updated

2 years ago
Blocks: webdriver
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Assignee: nobody → ato
Status: NEW → ASSIGNED
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Assignee

Updated

2 years ago
Attachment #8865545 - Flags: review?(mjzffr)
Attachment #8865546 - Flags: review?(mjzffr)
Attachment #8865547 - Flags: review?(mjzffr)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8865546 [details]
Bug 1363053 - Return JSON representation of return value

https://reviewboard.mozilla.org/r/137168/#review141234

::: testing/marionette/harness/marionette_harness/tests/unit/test_execute_script.py:333
(Diff revision 2)
>  
>      def test_access_global_objects_from_chrome(self):
>          # test inspection of arguments
>          self.marionette.execute_script("__webDriverArguments.toString()")
>  
> +    def test_toJSON(self):

It would be useful to test the "unsafe json" path as well.
Attachment #8865546 - Flags: review?(mjzffr) → review+

Comment 8

2 years ago
mozreview-review
Comment on attachment 8865545 [details]
Bug 1363053 - Move element.fromJson/toJson to evaluate module

https://reviewboard.mozilla.org/r/137166/#review141240
Attachment #8865545 - Flags: review?(mjzffr) → review+

Comment 9

2 years ago
mozreview-review
Comment on attachment 8865547 [details]
Bug 1363053 - Use Node.ELEMENT_NODE instead of magic number

https://reviewboard.mozilla.org/r/137170/#review141244

::: commit-message-089ab:5
(Diff revision 2)
> +Bug 1363053 - Use Node.ELEMENT_NODE instead of magic number
> +
> +This patch removes the magic number and instead uses the ELEMENT_NODE
> +constant which exists on all objects of the Node prototype chain.
> +We already test that obj has a nodeType own property.

I'll take your word for it, but I'm curious: where is this check?
Attachment #8865547 - Flags: review?(mjzffr) → review+
Assignee

Comment 10

2 years ago
mozreview-review-reply
Comment on attachment 8865546 [details]
Bug 1363053 - Return JSON representation of return value

https://reviewboard.mozilla.org/r/137168/#review141234

> It would be useful to test the "unsafe json" path as well.

Sure!  That actually caught a bug, so good call.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 16

2 years ago
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ffd5e79bff48
Move element.fromJson/toJson to evaluate module r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/dc14ec8d4358
Return JSON representation of return value r=maja_zf
https://hg.mozilla.org/integration/autoland/rev/5cdef0a42ffc
Use Node.ELEMENT_NODE instead of magic number r=maja_zf
You need to log in before you can comment on or make changes to this bug.