SetWindowSize response does not comply with WebDriver Spec

RESOLVED FIXED in Firefox 50

Status

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: njasm, Unassigned)

Tracking

(Blocks 1 bug)

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

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

3 years ago
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
Reporter

Comment 1

3 years ago
discard third link from my submission it was a copy/paste mistake.
OS: Unspecified → All
Hardware: Unspecified → All
Reporter

Updated

3 years ago
Blocks: webdriver
Reporter

Comment 2

3 years ago
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.
Flags: needinfo?(dburns)
Yup that looks like the exact right thing to do.
Flags: needinfo?(dburns)
Reporter

Comment 4

3 years ago
ok, I'm uploading a patch for review.
Attachment #8770762 - Flags: review?(dburns)
Reporter

Updated

3 years ago
Attachment #8770762 - Flags: review?(ato)
(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
Attachment #8770762 - Flags: review?(dburns)
Attachment #8770762 - Flags: review?(ato)
Reporter

Updated

3 years ago
Attachment #8770861 - Flags: review?(dburns)
Reporter

Comment 7

3 years ago
Sure. pushed to mozreview.
Attachment #8770861 - Flags: review?(dburns) → review+
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
I have triggered autoland to land this code when the trees reopen. Thank you very much for your patch.

Comment 10

3 years ago
Pushed by dburns@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a86bfdfc575d
Add SetWindowSize to return new width and height to comply with WD Spec r=automatedtester
Reporter

Comment 11

3 years ago
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!
Flags: needinfo?(dburns)
Reporter

Updated

3 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Comment 12

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a86bfdfc575d
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
(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.
Flags: needinfo?(dburns)
You need to log in before you can comment on or make changes to this bug.