1392368 - Window does not resize if the width has the same value when calling WebDriver:SetWindowRect

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Andreas Tolfsen ❲:ato❳]

As described in https://github.com/mozilla/geckodriver/issues/873,
the window will not resize if the message body for the
WebDriver:SetWindowRect command has a width field with the same
width as window.outerWidth.  Vice versa this applies to height.
Setting both fields will however work fine.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1392368 - Update test manifest.

For a transition period until we have fixed geckodriver, we want to coerce
the high-level API methods for setting the window size and position's
body fields to integers.  The low-level tests are still failing.

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

Comment 2

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1392368 - Add boilerplate for missing user prompt tests.

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

Comment 3

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1392368 - Add missing browsing context test.

This covers step 1 of the Set Window Rect algorithm.

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

Comment 4

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1392368 - Drop unnecessary navigation in user prompt tests.

The browsing context always has a document.

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

Comment 5

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1392368 - Correct types and bounds checks for Set Window Rect.

This patch splits types and bounds checks into two tests for clarity
and reduces unnecessary fields for readability.

It also covers a few missing eventualities and edge cases.

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

Comment 6

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1392368 - Add floating pointer tests for Set Window Rect.

Set Window Rect is meant to accept JSON Numbers, which can either be
integers or floats.

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

Comment 7

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1392368 - Test no-op input for Set Window Rect.

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

Comment 8

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1392368 - Improve edge case in-op tests for Set Window Rect.

Marionette has issues with repositioning/resizing the window when one
of width/height/x/y is the rect's existing value.  This is an edge case
it is worth testing for.

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

Comment 9

[Andreas Tolfsen ❲:ato❳]

Attachment: Bug 1392368 - Test restoring window on Set Window Rect exhaustively.

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

Comment 10

[Andreas Tolfsen ❲:ato❳]

I have written some tests for this, but I have so far been unable
to reproduce the issue.  I suspect my window manager is affecting
the tests.  Expect more patches to be appended when I find a way to
reproduce the problem.

In the meantime it should be safe to review the test changes.

Comment 11

[David Burns :automatedtester]

Comment on attachment 8899854 [details]
Bug 1392368 - Add boilerplate for missing user prompt tests.

https://reviewboard.mozilla.org/r/171182/#review176478

Comment 12

[David Burns :automatedtester]

Comment on attachment 8899855 [details]
Bug 1392368 - Add missing browsing context test.

https://reviewboard.mozilla.org/r/171184/#review176480

Comment 13

[David Burns :automatedtester]

Comment on attachment 8899857 [details]
Bug 1392368 - Correct types and bounds checks for Set Window Rect.

https://reviewboard.mozilla.org/r/171188/#review176486

Comment 14

[David Burns :automatedtester]

Comment on attachment 8899858 [details]
Bug 1392368 - Add floating pointer tests for Set Window Rect.

https://reviewboard.mozilla.org/r/171190/#review176488

Comment 15

[David Burns :automatedtester]

Comment on attachment 8899859 [details]
Bug 1392368 - Test no-op input for Set Window Rect.

https://reviewboard.mozilla.org/r/171192/#review176496

::: testing/web-platform/tests/webdriver/tests/set_window_rect.py:250
(Diff revision 1)
> +    {"width": None, "x": None},
> +    {"width": None, "y": None},
> +    {"height": None, "x": None},
> +    {"height": None, "Y": None},
> +
> +    {"width": None, "height": None, "x": None, "y": None},

This feels like it should error, we should get clarifcation on everything being `null`

Comment 16

[David Burns :automatedtester]

Comment on attachment 8899860 [details]
Bug 1392368 - Improve edge case in-op tests for Set Window Rect.

https://reviewboard.mozilla.org/r/171194/#review176504

::: testing/web-platform/tests/webdriver/tests/set_window_rect.py:424
(Diff revision 1)
> -                              "y": original["y"],
> -                              "width": original["width"],
> -                              "height": original["height"],
> -                              "state": original["state"]})
> +    position = session.window.position = (original_x, 345)
> +    assert position == (original_x, 345)
> +
> +
> +def test_move_to_same_y(session):
> +    original_y = session.window.position[1]

Use the key not the index so that it is readable

::: testing/web-platform/tests/webdriver/tests/set_window_rect.py:436
(Diff revision 1)
> +    size = session.window.size = original_size
> +    assert size == original_size
> +
> +
> +def test_resize_to_same_width(session):
> +    original_width = session.window.size[0]

Use the key not the index so its readable

::: testing/web-platform/tests/webdriver/tests/set_window_rect.py:442
(Diff revision 1)
> +    size = session.window.size = (original_width, 345)
> +    assert size == (original_width, 345)
> +
> +
> +def test_resize_to_same_height(session):
> +    original_height = session.window.size[1]

Use the key not the index so its readable

Comment 17

[David Burns :automatedtester]

Comment on attachment 8899861 [details]
Bug 1392368 - Test restoring window on Set Window Rect exhaustively.

https://reviewboard.mozilla.org/r/171196/#review176506

Comment 18

[David Burns :automatedtester]

Comment on attachment 8899860 [details]
Bug 1392368 - Improve edge case in-op tests for Set Window Rect.

https://reviewboard.mozilla.org/r/171194/#review176724

Comment 19

[David Burns :automatedtester]

Comment on attachment 8899856 [details]
Bug 1392368 - Drop unnecessary navigation in user prompt tests.

https://reviewboard.mozilla.org/r/171186/#review177460

Comment 20

[David Burns :automatedtester]

Comment on attachment 8899853 [details]
Bug 1392368 - Update test manifest.

https://reviewboard.mozilla.org/r/171180/#review177462

::: testing/web-platform/tests/tools/webdriver/webdriver/client.py:270
(Diff revision 1)
>      @position.setter
>      @command
>      def position(self, data):
> -        data = x, y
> -        body = {"x": x, "y": y}
> +        x, y = data
> +        # TODO(ato): We may not want to coerce to integers
> +        body = {"x": int(x), "y": int(y)}

As discussed yesterday, this fix should be in geckodriver or in the test and not in the client

Comment 21

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899853 [details]
Bug 1392368 - Update test manifest.

https://reviewboard.mozilla.org/r/171180/#review177462

> As discussed yesterday, this fix should be in geckodriver or in the test and not in the client

Fine, I will encode the integer coercions into the test for the time being and drop this patch.

Comment 22

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899860 [details]
Bug 1392368 - Improve edge case in-op tests for Set Window Rect.

https://reviewboard.mozilla.org/r/171194/#review176504

> Use the key not the index so that it is readable

The return value from session.window.position is (x, y) so indexing
is the only way to get the original Y axis.

This is why I named the variable "original_y".

> Use the key not the index so its readable

There is no other way to access a tuple than by index.  The return
value from session.window.size is (400, 345).

Comment 23

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899854 [details]
Bug 1392368 - Add boilerplate for missing user prompt tests.

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

Comment 24

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899855 [details]
Bug 1392368 - Add missing browsing context test.

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

Comment 25

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899856 [details]
Bug 1392368 - Drop unnecessary navigation in user prompt tests.

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

Comment 26

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899857 [details]
Bug 1392368 - Correct types and bounds checks for Set Window Rect.

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

Comment 27

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899858 [details]
Bug 1392368 - Add floating pointer tests for Set Window Rect.

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

Comment 28

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899859 [details]
Bug 1392368 - Test no-op input for Set Window Rect.

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

Comment 29

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899860 [details]
Bug 1392368 - Improve edge case in-op tests for Set Window Rect.

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

Comment 30

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899861 [details]
Bug 1392368 - Test restoring window on Set Window Rect exhaustively.

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

Comment 31

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899853 [details]
Bug 1392368 - Update test manifest.

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

Comment 32

[Andreas Tolfsen ❲:ato❳]

Comment on attachment 8899853 [details]
Bug 1392368 - Update test manifest.

https://reviewboard.mozilla.org/r/171180/#review178062

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dfd2fa166a3f
Add boilerplate for missing user prompt tests. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/210acb19f93d
Add missing browsing context test. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/81a84d9aed8c
Drop unnecessary navigation in user prompt tests. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/60930efea1e9
Correct types and bounds checks for Set Window Rect. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/4e1c33b177ae
Add floating pointer tests for Set Window Rect. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/8c3d264ab831
Test no-op input for Set Window Rect. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/4619cb7b65a7
Improve edge case in-op tests for Set Window Rect. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/a4ba2d21ddbe
Test restoring window on Set Window Rect exhaustively. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/e58415175f5d
Update test manifest. r=ato

Comment 34

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/dfd2fa166a3f
https://hg.mozilla.org/mozilla-central/rev/210acb19f93d
https://hg.mozilla.org/mozilla-central/rev/81a84d9aed8c
https://hg.mozilla.org/mozilla-central/rev/60930efea1e9
https://hg.mozilla.org/mozilla-central/rev/4e1c33b177ae
https://hg.mozilla.org/mozilla-central/rev/8c3d264ab831
https://hg.mozilla.org/mozilla-central/rev/4619cb7b65a7
https://hg.mozilla.org/mozilla-central/rev/a4ba2d21ddbe
https://hg.mozilla.org/mozilla-central/rev/e58415175f5d