1283959 - SetWindowSize response does not comply with WebDriver Spec

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Nelson João Morais (:njasm)]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160604131506

Steps to reproduce:

Marionette setWindowSize() does not return new width nor new height on success.
https://dxr.mozilla.org/mozilla-central/source/testing/marionette/driver.js#2483

It does not comply with the WebDriver Spec 
https://w3c.github.io/webdriver/webdriver-spec.html#set-window-size


https://w3c.github.io/webdriver/webdriver-spec.html#maximize-window

Comment 1

[Nelson João Morais (:njasm)]

discard third link from my submission it was a copy/paste mistake.

Comment 2

[Nelson João Morais (:njasm)]

I would like to work on this bug if confirmed.

I believe a simple call like:

this.GetWindowSize(cmd, resp);

would suffice(?) after:

win.resizeTo(width, height);

in https://dxr.mozilla.org/mozilla-central/source/testing/marionette/driver.js#2494

Besides adding a new unit test to confirm the result.

Comment 3

[David Burns :automatedtester]

Yup that looks like the exact right thing to do.

Comment 4

[Nelson João Morais (:njasm)]

Attachment: bug_1283959_setWindowSize_return_new_size.patch1

ok, I'm uploading a patch for review.

Comment 5

[David Burns :automatedtester]

(In reply to Nelson João Morais (:njasm) from comment #4)
> Created attachment 8770762 [details] [diff] [review]
> bug_1283959_setWindowSize_return_new_size.patch1
> 
> ok, I'm uploading a patch for review.

Please can you post this, and any other Marionette patches, to Mozreview

Comment 6

[Nelson João Morais (:njasm)]

Attachment: Bug 1283959 - Add SetWindowSize to return new width and height to comply with WD Spec

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

Comment 7

[Nelson João Morais (:njasm)]

Sure. pushed to mozreview.

Comment 8

[David Burns :automatedtester]

Comment on attachment 8770861 [details]
Bug 1283959 - Add SetWindowSize to return new width and height to comply with WD Spec

https://reviewboard.mozilla.org/r/64196/#review61422

Comment 9

[David Burns :automatedtester]

I have triggered autoland to land this code when the trees reopen. Thank you very much for your patch.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a86bfdfc575d
Add SetWindowSize to return new width and height to comply with WD Spec r=automatedtester

Comment 11

[Nelson João Morais (:njasm)]

So, by autoland you mean that I don't have to take no more actions in mozreview board, the trigger will run the tests, and if green, the patch will be merged into the tree?

If the patch land, will mozreview update the status of this bug to Resolved? or should I do it manually? 

Sorry for so many questions, it's my first time contributing to Mozilla, just wanna follow the workflow.

No need to thanks, if you have more "good first bugs" that you think I'm able to grab them, send them over, I'll be happy to help!

Comment 12

[Carsten Book [:Tomcat]]

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

Comment 13

[David Burns :automatedtester]

(In reply to Nelson João Morais (:njasm) from comment #11)
> So, by autoland you mean that I don't have to take no more actions in
> mozreview board, the trigger will run the tests, and if green, the patch
> will be merged into the tree?
> 
> If the patch land, will mozreview update the status of this bug to Resolved?
> or should I do it manually? 
> 
> Sorry for so many questions, it's my first time contributing to Mozilla,
> just wanna follow the workflow.
> 
> No need to thanks, if you have more "good first bugs" that you think I'm
> able to grab them, send them over, I'll be happy to help!

The automated systems were going to take over and land and document the change.