David Burns :automatedtester Assignee
	Description • 7 years ago

This is from https://github.com/mozilla/geckodriver/issues/769

	David Burns :automatedtester Assignee
	Comment 1 • 7 years ago

This is dependendent on a new webdriver rust being released after https://github.com/mozilla/webdriver-rust/pull/100 being merged in.

	James Graham [:jgraham]
	Comment 3 • 7 years ago
mozreview-review

Comment on attachment 8875242 [details]
Bug 1370510: Implement Fullscreen Window command

https://reviewboard.mozilla.org/r/146652/#review150704

::: testing/geckodriver/Cargo.toml:30
(Diff revision 1)
>  slog = "1"
>  slog-atomic = "0.4"
>  slog-stdlog = "1"
>  slog-stream = "1"
>  uuid = "0.1.18"
> -webdriver = "0.25"
> +webdriver = {path = "/Users/dburns/development/geckodriver/webdriver-rust/"}

Hmm? This will require a webdriver version bump to 0.26.

::: testing/geckodriver/src/marionette.rs:817
(Diff revision 1)
> +                let height = try_opt!(
> +                    try_opt!(resp.result.find("height"),
> +                             ErrorStatus::UnknownError,
> +                             "Failed to find height field").as_u64(),
> +                    ErrorStatus::UnknownError,
> +                    "Failed to interpret width as integer");

height

	Andreas Tolfsen ❲:ato❳
	Comment 5 • 7 years ago
mozreview-review

Comment on attachment 8875242 [details]
Bug 1370510: Implement Fullscreen Window command

https://reviewboard.mozilla.org/r/146652/#review150710

::: commit-message-1b21c:3
(Diff revision 2)
> +Bug 1370510: Implement Fullscreen command r?jgraham
> +
> +This implements the Geckodriver support for the Fullscreen command.

s/Geckodriver/geckodriver/

::: commit-message-1b21c:3
(Diff revision 2)
> +Bug 1370510: Implement Fullscreen command r?jgraham
> +
> +This implements the Geckodriver support for the Fullscreen command.

s/Fullscreen/Fullscreen Window/

::: commit-message-1b21c:4
(Diff revision 2)
> +Bug 1370510: Implement Fullscreen command r?jgraham
> +
> +This implements the Geckodriver support for the Fullscreen command.
> +WebDriver-Rust support was added in https://github.com/mozilla/webdriver-rust/pull/100

I would rephrase this:

> The API was introduced in the webdriver crate in …

And also break the line at ~72 characters.

::: testing/geckodriver/Cargo.toml:30
(Diff revision 2)
>  slog = "1"
>  slog-atomic = "0.4"
>  slog-stdlog = "1"
>  slog-stream = "1"
>  uuid = "0.1.18"
> -webdriver = "0.25"
> +webdriver = "0.26.0"

To upgrade this dependency you need to call `./mach vendor rust` instead of `cargo update`.  Your changes will cause the build on CI to fail because it uses dependencies stored in third_party/rust; and not from $HOME/.cargo.  This means you have to check in Cargo.lock again.

::: testing/geckodriver/src/marionette.rs:833
(Diff revision 2)
> +                WebDriverResponse::WindowRect(WindowRectResponse {x: x,
> +                                                                  y: y,
> +                                                                  width: width,
> +                                                                  height: height})

This should respond with WebDriverResponse::FullscreenWindow(WindowRectResponse) I think?

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

Comment on attachment 8875242 [details]
Bug 1370510: Implement Fullscreen Window command

https://reviewboard.mozilla.org/r/146652/#review150710

> This should respond with WebDriverResponse::FullscreenWindow(WindowRectResponse) I think?

There isnt a `WebDriverResponse::FullscreenWindow` in webdriver-rust so it can't be that. `WindowRect` is used for all the different types of calls around Window Rect info

	James Graham [:jgraham]
	Comment 11 • 7 years ago
mozreview-review

Comment on attachment 8875299 [details]
Bug 1370510 - Update WebDriver Crate dependency;

https://reviewboard.mozilla.org/r/146714/#review151218

	James Graham [:jgraham]
	Comment 12 • 7 years ago
mozreview-review

Comment on attachment 8875242 [details]
Bug 1370510: Implement Fullscreen Window command

https://reviewboard.mozilla.org/r/146652/#review151220

	Pulsebot
	Comment 13 • 7 years ago

Pushed by dburns@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a7d219eb3c53
Implement Fullscreen Window command r=jgraham
https://hg.mozilla.org/integration/autoland/rev/f6f6770d93af
Update WebDriver Crate dependency; r=jgraham

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

https://hg.mozilla.org/mozilla-central/rev/a7d219eb3c53
https://hg.mozilla.org/mozilla-central/rev/f6f6770d93af