Andreas Tolfsen ❲:ato❳ Assignee
	Description • 7 years ago

The WebDriverCommand::FullscreenWindow in geckodriver maps to the non-existent Marionette command "fullscreenWindow".  It should be "fullscreen".

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 5 • 7 years ago
mozreview-review

Comment on attachment 8894491 [details]
Bug 1388036 - Map WebDriverCommand::FullscreenWindow correctly.

https://reviewboard.mozilla.org/r/165654/#review170738

::: testing/geckodriver/src/marionette.rs:1044
(Diff revision 1)
>              SetTimeouts(ref x) => (Some("setTimeouts"), Some(x.to_marionette())),
>              SetWindowRect(ref x) => (Some("setWindowRect"), Some(x.to_marionette())),
>              GetWindowRect => (Some("getWindowRect"), None),
>              MinimizeWindow => (Some("WebDriver:MinimizeWindow"), None),
>              MaximizeWindow => (Some("maximizeWindow"), None),
> -            FullscreenWindow => (Some("fullscreenWindow"), None),
> +            FullscreenWindow => (Some("fullscreen"), None),

When it wasn't working correctly so far, why don't we use the `WebDriver:FullscreenWindow` command directly?

	David Burns :automatedtester
	Comment 6 • 7 years ago
mozreview-review

Comment on attachment 8894494 [details]
Bug 1388036 - Add WPT tests for Fullscreen Window.

https://reviewboard.mozilla.org/r/165660/#review170756

::: testing/web-platform/tests/webdriver/tests/fullscreen_window.py:26
(Diff revision 1)
> +def test_handle_user_prompt(session):
> +    # step 2
> +    session.url = alert_doc
> +    response = fullscreen(session)
> +    assert_error(response, "unexpected alert open")
> +

We need to add more tests like in `get_title.py` for prompts

::: testing/web-platform/tests/webdriver/tests/fullscreen_window.py:47
(Diff revision 1)
> +    rect = response.body["value"]
> +    assert "width" in rect
> +    assert "height" in rect
> +    assert "x" in rect
> +    assert "y" in rect
> +    assert isinstance(rect["width"], float)

This should test against `(int, float)`

	David Burns :automatedtester
	Comment 7 • 7 years ago
mozreview-review

Comment on attachment 8894492 [details]
Bug 1388036 - Add client.Session#fullscreen API to WebDriver client.

https://reviewboard.mozilla.org/r/165656/#review170758

	David Burns :automatedtester
	Comment 8 • 7 years ago
mozreview-review

Comment on attachment 8894493 [details]
Bug 1388036 - Clean up fullscreen state after wdspec test.

https://reviewboard.mozilla.org/r/165658/#review170760

	David Burns :automatedtester
	Comment 9 • 7 years ago
mozreview-review

Comment on attachment 8894491 [details]
Bug 1388036 - Map WebDriverCommand::FullscreenWindow correctly.

https://reviewboard.mozilla.org/r/165654/#review170762

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 10 • 7 years ago
mozreview-review-reply

Comment on attachment 8894491 [details]
Bug 1388036 - Map WebDriverCommand::FullscreenWindow correctly.

https://reviewboard.mozilla.org/r/165654/#review170738

> When it wasn't working correctly so far, why don't we use the `WebDriver:FullscreenWindow` command directly?

WebDriver::FullscreenWindow is only available in Firefox 56 and greater.  By using "fullscreen" it will work with earlier Firefoxen.

	Henrik Skupin [:whimboo][⌚️UTC+1]
	Comment 13 • 7 years ago
mozreview-review-reply

Comment on attachment 8894491 [details]
Bug 1388036 - Map WebDriverCommand::FullscreenWindow correctly.

https://reviewboard.mozilla.org/r/165654/#review170738

> WebDriver::FullscreenWindow is only available in Firefox 56 and greater.  By using "fullscreen" it will work with earlier Firefoxen.

Makes sense. Thanks.

	Andreas Tolfsen ❲:ato❳ Assignee
	Comment 14 • 7 years ago

The new fullscreen tests appear to cause too many intermittents with Linux debug builds.  I suspect I need to revisit the actually Marionette implementation and make it more reliable.

	David Burns :automatedtester
	Comment 15 • 7 years ago
mozreview-review

Comment on attachment 8894494 [details]
Bug 1388036 - Add WPT tests for Fullscreen Window.

https://reviewboard.mozilla.org/r/165660/#review171362

	David Burns :automatedtester
	Comment 25 • 7 years ago
mozreview-review

Comment on attachment 8898821 [details]
Bug 1388036 - Restore window when setting window rect.

https://reviewboard.mozilla.org/r/170194/#review175896

	Pulsebot
	Comment 26 • 7 years ago

Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d8f04b1a073f
Restore window when setting window rect. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/8efd7bd68e91
Map WebDriverCommand::FullscreenWindow correctly. r=automatedtester
https://hg.mozilla.org/integration/autoland/rev/c0f5b257ba6b
Add WPT tests for Fullscreen Window. r=automatedtester

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 27 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/d8f04b1a073f
https://hg.mozilla.org/mozilla-central/rev/8efd7bd68e91
https://hg.mozilla.org/mozilla-central/rev/c0f5b257ba6b