1280101 - Set Window Size should not error on window size larger than screen

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Andreas Tolfsen ❲:ato❳]

According to the WebDriver specification, when a requested window size is larger than the screen we should set the window containing the top-level browsing context to a size that is “as close as possible” to the requested size.

This is more in line with the current Selenium behaviour.

http://w3c.github.io/webdriver/webdriver-spec.html#dfn-set-window-size

See also https://github.com/mozilla/geckodriver/issues/56#issuecomment-225713923 for more information.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1280101 - Do not coerce window dimensions to integer;

The specification says nothing about coercing to integer.

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

Comment 2

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1280101 - Remove check for window sizes larger than screen;

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

Comment 3

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1280101 - Point out that test will only pass under X in a WM;

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

Comment 4

[David Burns :automatedtester]

Comment on attachment 8762735 [details]
Bug 1280101 - Do not coerce window dimensions to integer;

https://reviewboard.mozilla.org/r/59234/#review56802

Comment 5

[David Burns :automatedtester]

Comment on attachment 8762736 [details]
Bug 1280101 - Remove check for window sizes larger than screen;

https://reviewboard.mozilla.org/r/59236/#review56804

Comment 6

[David Burns :automatedtester]

Comment on attachment 8762737 [details]
Bug 1280101 - Point out that test will only pass under X in a WM;

https://reviewboard.mozilla.org/r/59238/#review56806

Manipulating windows implies that we will need a window manager of sorts so don't think this comment is necessary

Comment 7

[Andreas Tolfsen ❲:ato❳]

(In reply to David Burns :automatedtester from comment #6)
> Comment on attachment 8762737 [details]
> Bug 1280101 - Point out that test will only pass under X in a WM;
> 
> https://reviewboard.mozilla.org/r/59238/#review56806
> 
> Manipulating windows implies that we will need a window manager of sorts so
> don't think this comment is necessary

Well okay, but multiple people have been confused by this in the past.  Including me.

Comment 8

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8762735 [details]
Bug 1280101 - Do not coerce window dimensions to integer;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59234/diff/1-2/

Comment 9

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8762736 [details]
Bug 1280101 - Remove check for window sizes larger than screen;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/59236/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/eca2ce2b6dd2
Do not coerce window dimensions to integer; r=automatedtester
https://hg.mozilla.org/integration/mozilla-inbound/rev/333518a265a2
Remove check for window sizes larger than screen; r=automatedtester

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/eca2ce2b6dd2
https://hg.mozilla.org/mozilla-central/rev/333518a265a2

Comment 12

[Carsten Book [:Tomcat]]

aurora:

remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/98142136966a
remote:   https://hg.mozilla.org/releases/mozilla-aurora/rev/0ede8b892980

beta:

remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/9df0ea5a8f8f
remote:   https://hg.mozilla.org/releases/mozilla-beta/rev/85ccaed0061c

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out 85ccaed0061c from beta: https://hg.mozilla.org/releases/mozilla-beta/rev/2b22de79c849 to see if that fixes bug 1282937.

Comment 14

[Andreas Tolfsen ❲:ato❳]

I don’t know if I care enough to spend time figuring out why it breaks Windows on Beta.  I guess we can let the patch ride the train to Beta in a natural way.

Comment 15

[soumyamulu]

Can you please confirm whether the fix for the window size error is done or rolled back. I am currently using FF 48+ and Selenium(2.53.1, 3.0.0-beta(123)) all give me the same exception unless i set the window size to 1024X768. This is breaking my tests because the elements are out of the window scope.

Comment 16

[soumyamulu]

Can you please confirm whether the fix for the window size error is done or rolled back. I am currently using FF 48+ and Selenium(2.53.1, 3.0.0-beta(123)) all give me the same exception unless i set the window size to 1024X768. This is breaking my tests because the elements are out of the window scope.

Comment 17

[David Burns :automatedtester]

(In reply to soumyamulu from comment #16)
> Can you please confirm whether the fix for the window size error is done or
> rolled back. I am currently using FF 48+ and Selenium(2.53.1,
> 3.0.0-beta(123)) all give me the same exception unless i set the window size
> to 1024X768. This is breaking my tests because the elements are out of the
> window scope.

This is done and will be available in Firefox 50.