Closed Bug 1391691 Opened 8 years ago Closed 8 years ago

Make {Fullscreen,Maximize,Minimize} Window commands idempotent

Categories

(Remote Protocol :: Marionette, enhancement)

Product:
Remote Protocol
Component:
Marionette
Version:
Version 3
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox57 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla57
Tracking Flags:
Tracking Status
firefox57 --- fixed

People

(Reporter: ato, Assigned: ato)

References

(Blocks 1 open bug)

Details

Attachments

(10 files)

Bug 1391691 - Rename GeckoDriver#fullscreen to fullscreenWindow.
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1391691 - Make WebDriver:MinimizeWindow idempotent.
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1391691 - Make WebDriver:MaximizeWindow idempotent.
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1391691 - Make WebDriver:FullscreenWindow idempotent.
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1391691 - Restore window state using Set Window Rect.
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1391691 - Add wdclient documentation for window manipulation.
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1391691 - Fix undefined variable error with client.Session#position().
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1391691 - Bringing window out of fullscreen does not restore x/y position.
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1391691 - Skip needless assertion.
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
Bug 1391691 - Export WindowState properly.
59 bytes, text/x-review-board-request
automatedtester
: review+
Details
The WebDriver specification has changed (again) and the {Fullscreen,Maximize,Minimize} Window commands should now be idempotent (should be a no-op when called a second time).
Assignee: nobody → ato
Blocks: webdriver
Status: NEW → ASSIGNED
Depends on: 1391661
Depends on: 1388036
Comment on attachment 8898910 [details] Bug 1391691 - Make WebDriver:FullscreenWindow idempotent. https://reviewboard.mozilla.org/r/170264/#review175760 ::: testing/marionette/driver.js:3137 (Diff revision 1) > GeckoDriver.prototype.fullscreenWindow = async function(cmd, resp) { > assert.firefox(); > const win = assert.window(this.getCurrentWindow()); > assert.noUserPrompt(this.dialog); > > + if (!win.fullScreen) { Why aren't you using WindowState here?
Comment on attachment 8898914 [details] Bug 1391691 - Bringing window out of fullscreen does not restore x/y position. https://reviewboard.mozilla.org/r/170272/#review175762 ::: testing/web-platform/tests/webdriver/tests/set_window_rect.py:94 (Diff revision 1) > - original = session.window.rect > - > # step 10 > session.window.fullscreen() > assert session.window.state == "fullscreen" > response = set_window_rect(session, {"width": 400, "height": 400}) That's bad. But we allow to pass null params to `set_window_rect` (see step 4 and 5) which should keep/restore to the original values?
Comment on attachment 8898914 [details] Bug 1391691 - Bringing window out of fullscreen does not restore x/y position. https://reviewboard.mozilla.org/r/170272/#review175762 > That's bad. But we allow to pass null params to `set_window_rect` (see step 4 and 5) which should keep/restore to the original values? Well as later steps show it's invalid to pass `null` values. It means we never can test that leaving maximize, minimize, or fullscreen state will restore the original values.
Comment on attachment 8898914 [details] Bug 1391691 - Bringing window out of fullscreen does not restore x/y position. https://reviewboard.mozilla.org/r/170272/#review175762 > Well as later steps show it's invalid to pass `null` values. It means we never can test that leaving maximize, minimize, or fullscreen state will restore the original values. There is nothing in the HTML specification that says fully exiting fullscreen should restore the window to the same X/Y position it was in before. This is a trait of the WM and outside the scope of what we can assert.
Comment on attachment 8898907 [details] Bug 1391691 - Rename GeckoDriver#fullscreen to fullscreenWindow. https://reviewboard.mozilla.org/r/170258/#review175850
Attachment #8898907 - Flags: review?(dburns) → review+
Comment on attachment 8898908 [details] Bug 1391691 - Make WebDriver:MinimizeWindow idempotent. https://reviewboard.mozilla.org/r/170260/#review175862
Attachment #8898908 - Flags: review?(dburns) → review+
Comment on attachment 8898909 [details] Bug 1391691 - Make WebDriver:MaximizeWindow idempotent. https://reviewboard.mozilla.org/r/170262/#review175866
Attachment #8898909 - Flags: review?(dburns) → review+
Comment on attachment 8898910 [details] Bug 1391691 - Make WebDriver:FullscreenWindow idempotent. https://reviewboard.mozilla.org/r/170264/#review175874 As with whimboo's comment, use window state to be consistent with the other commands
Attachment #8898910 - Flags: review?(dburns) → review-
Comment on attachment 8898911 [details] Bug 1391691 - Restore window state using Set Window Rect. https://reviewboard.mozilla.org/r/170266/#review175876
Attachment #8898911 - Flags: review?(dburns) → review+
Comment on attachment 8898912 [details] Bug 1391691 - Add wdclient documentation for window manipulation. https://reviewboard.mozilla.org/r/170268/#review175878
Attachment #8898912 - Flags: review?(dburns) → review+
Comment on attachment 8898913 [details] Bug 1391691 - Fix undefined variable error with client.Session#position(). https://reviewboard.mozilla.org/r/170270/#review175880
Attachment #8898913 - Flags: review?(dburns) → review+
Comment on attachment 8898910 [details] Bug 1391691 - Make WebDriver:FullscreenWindow idempotent. https://reviewboard.mozilla.org/r/170264/#review175874 The other commands aren’t using WindowState, but we can do even if this will get fixed implicitly as part of bug 1311041.
Comment on attachment 8898910 [details] Bug 1391691 - Make WebDriver:FullscreenWindow idempotent. https://reviewboard.mozilla.org/r/170264/#review175884
Attachment #8898910 - Flags: review?(dburns) → review+
Comment on attachment 8898915 [details] Bug 1391691 - Skip needless assertion. https://reviewboard.mozilla.org/r/170274/#review175886 ::: testing/web-platform/tests/webdriver/tests/contexts/maximize_window.py:60 (Diff revision 2) > > assert before_size != session.window.size > assert session.window.state == "maximized" > > > +def test_maximize_twice_is_indempotent(session): I think this test should be in the `maximise() is idempotent` change. also you have misspelled the test. `s/indempotent/idempotent/`
Attachment #8898915 - Flags: review?(dburns) → review+
Comment on attachment 8898914 [details] Bug 1391691 - Bringing window out of fullscreen does not restore x/y position. https://reviewboard.mozilla.org/r/170272/#review175890
Attachment #8898914 - Flags: review?(dburns) → review+
Comment on attachment 8898916 [details] Bug 1391691 - Export WindowState properly. https://reviewboard.mozilla.org/r/170276/#review175894
Attachment #8898916 - Flags: review?(dburns) → review+
Comment on attachment 8898915 [details] Bug 1391691 - Skip needless assertion. https://reviewboard.mozilla.org/r/170274/#review175886 > I think this test should be in the `maximise() is idempotent` change. also you have misspelled the test. `s/indempotent/idempotent/` You’re right. I appear to have messed something up when I rebased and reordered the patches in my last pass. Will address this.
Comment on attachment 8898914 [details] Bug 1391691 - Bringing window out of fullscreen does not restore x/y position. https://reviewboard.mozilla.org/r/170272/#review175762 > There is nothing in the HTML specification that says fully exiting fullscreen should restore the window to the same X/Y position it was in before. This is a trait of the WM and outside the scope of what we can assert. Not sure what we need here, the HTML or fullscreen API spec: https://fullscreen.spec.whatwg.org/#exit-fullscreen? It has at least a resize option. Can you please clarify if that is a webdriver spec issue or not? In any case this can be handled separately.
Comment on attachment 8898914 [details] Bug 1391691 - Bringing window out of fullscreen does not restore x/y position. https://reviewboard.mozilla.org/r/170272/#review175762 > Not sure what we need here, the HTML or fullscreen API spec: https://fullscreen.spec.whatwg.org/#exit-fullscreen? It has at least a resize option. > > Can you please clarify if that is a webdriver spec issue or not? In any case this can be handled separately. We talked about this on IRC, and the WebDriver specification no longer provides primitives for restoring the window to its previous state because the Fullscreen Window command is now idempotent. In any case, restoring a window from fullscreen or iconified state does not necessarily mean the window’s X/Y position will be the same as it was before it went into said state because this behaviour depends on the window manager. It may incidentally be restored to the same position, but this is only incidental. Furthermore, this test uses Set Window Rect to exit fullscreen with a width/height parameter, which definitely means we should not be asserting the returned rect’s X/Y fields.
Pushed by atolfsen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/17aeed1f6454 Export WindowState properly. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/6c48daaad075 Rename GeckoDriver#fullscreen to fullscreenWindow. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/649b0e761c87 Add wdclient documentation for window manipulation. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/6306abc0b5e9 Fix undefined variable error with client.Session#position(). r=automatedtester https://hg.mozilla.org/integration/autoland/rev/fd88b612ac2e Restore window state using Set Window Rect. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/4653134d01ef Bringing window out of fullscreen does not restore x/y position. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/5bddbd90ec7c Skip needless assertion. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/1fd98ace1473 Make WebDriver:MinimizeWindow idempotent. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/46f82e1e2cde Make WebDriver:MaximizeWindow idempotent. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/666ac679317e Make WebDriver:FullscreenWindow idempotent. r=automatedtester
Backout by kwierso@gmail.com: https://hg.mozilla.org/integration/autoland/rev/28b678c9009d Backed out 10 changesets for wpt failures a=backout
(In reply to Wes Kocher (:KWierso) from comment #45) > I had to back these out for failures like > https://treeherder.mozilla.org/logviewer.html#?job_id=124591121&repo=autoland This should’ve been addressed by bug 1388036, but I guess with Autoland we don’t test changes stacked on top of eachother, but on their own separate branches rebased against inbound. So what we have to do here is simply wait for that change to land, then re-land this.
Flags: needinfo?(ato)
Pushed by atolfsen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/23b08fa8fad0 Export WindowState properly. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/821016a04a50 Rename GeckoDriver#fullscreen to fullscreenWindow. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/83080d660f56 Add wdclient documentation for window manipulation. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/5e07d579282d Fix undefined variable error with client.Session#position(). r=automatedtester https://hg.mozilla.org/integration/autoland/rev/c36b163dd034 Restore window state using Set Window Rect. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/2286cf7d577a Bringing window out of fullscreen does not restore x/y position. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/9013ea0fc942 Skip needless assertion. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/ee876a24b7e4 Make WebDriver:MinimizeWindow idempotent. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/ef813a1a750f Make WebDriver:MaximizeWindow idempotent. r=automatedtester https://hg.mozilla.org/integration/autoland/rev/2f25a6f73753 Make WebDriver:FullscreenWindow idempotent. r=automatedtester
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: