1279203 - Make Get Page Source spec conformant

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Andreas Tolfsen ❲:ato❳]

The Marionette implementation uses the XMLSerializer on the document object, but should be returning the outerHTML untouched.

Comment 1

[Andreas Tolfsen ❲:ato❳]

The relevant spec: https://w3c.github.io/webdriver/webdriver-spec.html#dfn-get-page-source

Comment 2

[Vangelis Katsikaros]

Attachment: 1279203.diff

Make the Get Page Source webdriver command spec conformant.

Freshly built firefox on top of "957458d No bug, Automated blocklist update from host bld-linux64-spot-409 - a=blocklist-update", and successfully passes 

./mach test testing/marionette/harness/marionette/tests/unit/test_pagesource.py

Comment 3

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8814983 [details] [diff] [review]
1279203.diff

Review of attachment 8814983 [details] [diff] [review]:
-----------------------------------------------------------------

This looks correct, although you don’t have to assign it to the `source` variable before returning it.

Did you run the test_pagesource.py tests?  Are they passing?

It would also be good if you could add a test there that compares the return value of `self.marionette.execute_script("return document.documentElement.outerHTML")` to that of `self.marionette.page_source`.

For the next patch you upload, please make it a .patch file and not a .diff.  If you use hg, do `hg export -r REV` for this.

Comment 4

[Vangelis Katsikaros]

Attachment: 1279203.patch

Thanks for the review!

Yep, the test passes as mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=1279203#c2

For the updated patch:
1) I am using git and created a patch with `git format-patch master --stdout > ~/1279203.patch` (I had my work on a separate branch)
2) Taken care of the useless `source` variable assignment.
3) Taken care of the test & execute_script (completely forgot about this, sorry). I rerun `./mach test testing/marionette/harness/marionette/tests/unit/test_pagesource.py` and passes ok (output below).

```
gecko-dev$ git log -1 --oneline 
d5b571e Bug 1279203 - Make Get Page Source spec conformant
gecko-dev$ ./mach test testing/marionette/harness/marionette/tests/unit/test_pagesource.py 
 0:00.00 LOG: MainThread INFO Using workspace for temporary data: "/home/vag/projects/gecko-dev"
 0:00.00 LOG: MainThread mozversion INFO application_buildid: 20161128185016
 0:00.00 LOG: MainThread mozversion INFO application_display_name: Nightly
 0:00.00 LOG: MainThread mozversion INFO application_id: {ec8030f7-c20a-464f-9b0e-13a3a9e97384}
 0:00.00 LOG: MainThread mozversion INFO application_name: Firefox
 0:00.00 LOG: MainThread mozversion INFO application_remotingname: firefox
 0:00.00 LOG: MainThread mozversion INFO application_vendor: Mozilla
 0:00.00 LOG: MainThread mozversion INFO application_version: 53.0a1
 0:00.00 LOG: MainThread mozversion INFO platform_buildid: 20161128185016
 0:00.00 LOG: MainThread mozversion INFO platform_version: 53.0a1
 0:00.01 LOG: MainThread INFO Application command: /home/vag/projects/gecko-dev/obj-x86_64-pc-linux-gnu/dist/bin/firefox -no-remote -marionette -profile /tmp/tmpfSHwqn.mozrunner
 0:02.37 LOG: MainThread INFO Profile path is /tmp/tmpfSHwqn.mozrunner
 0:02.37 LOG: MainThread INFO Starting fixture servers
 0:02.42 LOG: MainThread INFO Fixture server listening on http://192.168.1.50:41412/
 0:02.42 LOG: MainThread INFO Fixture server listening on https://192.168.1.50:46170/
 0:02.42 LOG: MainThread INFO e10s is enabled
 0:02.42 LOG: MainThread mozversion INFO application_buildid: 20161128185016
 0:02.42 LOG: MainThread mozversion INFO application_display_name: Nightly
 0:02.42 LOG: MainThread mozversion INFO application_id: {ec8030f7-c20a-464f-9b0e-13a3a9e97384}
 0:02.42 LOG: MainThread mozversion INFO application_name: Firefox
 0:02.42 LOG: MainThread mozversion INFO application_remotingname: firefox
 0:02.42 LOG: MainThread mozversion INFO application_vendor: Mozilla
 0:02.42 LOG: MainThread mozversion INFO application_version: 53.0a1
 0:02.42 LOG: MainThread mozversion INFO platform_buildid: 20161128185016
 0:02.42 LOG: MainThread mozversion INFO platform_version: 53.0a1
 0:02.42 SUITE_START: MainThread 1
 0:04.35 TEST_START: MainThread test_pagesource.py TestPageSource.testShouldReturnAXMLDocumentSource
 0:04.69 TEST_END: MainThread PASS
 0:04.69 TEST_START: MainThread test_pagesource.py TestPageSource.testShouldReturnTheSourceOfAPage
 0:04.87 TEST_END: MainThread PASS
 0:04.87 TEST_START: MainThread test_pagesource.py TestPageSource.testShouldReturnTheSourceOfAPageWhenThereAreUnicodeChars
 0:05.20 TEST_END: MainThread PASS
 0:05.20 LOG: MainThread INFO 
SUMMARY
-------
 0:05.20 LOG: MainThread INFO passed: 3
 0:05.20 LOG: MainThread INFO failed: 0
 0:05.20 LOG: MainThread INFO todo: 0
 0:05.20 SUITE_END: MainThread 
Summary
=======

Ran 3 tests
Expected results: 3
Unexpected results: 0

OK
```

Comment 5

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1279203 - Make Get Page Source command spec conformant;

This change aligns the Marionette command for getting the page's source
with the WebDriver specification, which mandates that we should return
the current browsing context's active document's outerHTML.

Review commit: https://reviewboard.mozilla.org/r/96538/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96538/

Comment 6

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8815676 [details]
Bug 1279203 - Make Get Page Source command spec conformant;

https://reviewboard.mozilla.org/r/96538/#review96734

Comment 7

[Andreas Tolfsen ❲:ato❳]

I’ve submitted the patch to mozreview and triggered a try run.  Will land once green.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f707a4378942
Make Get Page Source command spec conformant; r=ato

Comment 9

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/f707a4378942

Comment 10

[Andreas Tolfsen ❲:ato❳]

Sheriffs: Test-only uplift because Marionette is hidden behind flag.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5805d86370c2

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8adf693e9a0c