Closed Bug 1392368 Opened 7 years ago Closed 7 years ago

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

Categories

(Remote Protocol :: Marionette, defect)

Version 3
defect
Not set
normal

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: ato, Assigned: ato)

References

(Blocks 1 open bug, )

Details

Attachments

(9 files)

59 bytes, text/x-review-board-request
ato
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
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.
Assignee: nobody → ato
Blocks: webdriver
Status: NEW → ASSIGNED
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 on attachment 8899854 [details]
Bug 1392368 - Add boilerplate for missing user prompt tests.

https://reviewboard.mozilla.org/r/171182/#review176478
Attachment #8899854 - Flags: review?(dburns) → review+
Comment on attachment 8899855 [details]
Bug 1392368 - Add missing browsing context test.

https://reviewboard.mozilla.org/r/171184/#review176480
Attachment #8899855 - Flags: review?(dburns) → review+
Comment on attachment 8899857 [details]
Bug 1392368 - Correct types and bounds checks for Set Window Rect.

https://reviewboard.mozilla.org/r/171188/#review176486
Attachment #8899857 - Flags: review?(dburns) → review+
Comment on attachment 8899858 [details]
Bug 1392368 - Add floating pointer tests for Set Window Rect.

https://reviewboard.mozilla.org/r/171190/#review176488
Attachment #8899858 - Flags: review?(dburns) → review+
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`
Attachment #8899859 - Flags: review?(dburns) → review+
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
Attachment #8899860 - Flags: review?(dburns) → review-
Comment on attachment 8899861 [details]
Bug 1392368 - Test restoring window on Set Window Rect exhaustively.

https://reviewboard.mozilla.org/r/171196/#review176506
Attachment #8899861 - Flags: review?(dburns) → review+
Comment on attachment 8899860 [details]
Bug 1392368 - Improve edge case in-op tests for Set Window Rect.

https://reviewboard.mozilla.org/r/171194/#review176724
Attachment #8899860 - Flags: review- → review+
Comment on attachment 8899856 [details]
Bug 1392368 - Drop unnecessary navigation in user prompt tests.

https://reviewboard.mozilla.org/r/171186/#review177460
Attachment #8899856 - Flags: review?(dburns) → review+
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
Attachment #8899853 - Flags: review?(dburns) → review-
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 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).
Attachment #8899853 - Flags: review?(dburns) → review?(ato)
Comment on attachment 8899853 [details]
Bug 1392368 - Update test manifest.

https://reviewboard.mozilla.org/r/171180/#review178062
Attachment #8899853 - Flags: review?(ato) → review+
Pushed by atolfsen@mozilla.com:
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
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: