Closed Bug 1363053 Opened 4 years ago Closed 4 years ago

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

Categories

(Testing :: Marionette, enhancement)

Version 3
enhancement
Not set
normal

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: ato, Assigned: ato)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

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.
Blocks: webdriver
Assignee: nobody → ato
Status: NEW → ASSIGNED
Attachment #8865545 - Flags: review?(mjzffr)
Attachment #8865546 - Flags: review?(mjzffr)
Attachment #8865547 - Flags: review?(mjzffr)
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 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 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+
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.
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.