Closed Bug 1400226 Opened 2 years ago Closed 2 years ago

Wait for visibilitychange event in WebDriver:MinimizeWindow

Categories

(Testing :: Marionette, defect, P1)

Version 3
defect

Tracking

(firefox58 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox58 --- fixed

People

(Reporter: ato, Assigned: ato)

References

Details

Attachments

(1 file)

The sizemodechange event is not strongly connected to the
visibilitychange event that the WPT minimize_window.py test is
now using to ascertain whether the window has been successfully
iconified.

Because Marionette uses the sizemodechange event it is
causing intermittents such as https://bugzil.la/1397007.  You
can also read a lengthy summary I did on the problem in
https://bugzil.la/1397007#c11.

The fix for the problem is to wait for the visibilitychange DOM
event content.
Assignee: nobody → ato
Status: NEW → ASSIGNED
Priority: -- → P1
Depends on: 1400225
Blocks: 1397007
Blocks: 1397069
The sizemodechange event is not strongly connected to the
visibilitychange event that the WPT minimize_window.py test is now
using to ascertain whether the window has been successfully iconified.

Because Marionette uses the sizemodechange event it is causing
intermittents such as https://bugzil.la/1397007.  You can also read a
lengthy summary I did on the problem in https://bugzil.la/1397007#c11.

The fix for the problem is to wait for the visibilitychange DOM
event content.

MozReview-Commit-ID: B6i33Ee5iMC
Attachment #8910282 - Flags: review?(hskupin)
Comment on attachment 8910282 [details] [diff] [review]
Wait for visibilitychange event on window minimize/restore. r?whimboo

Review of attachment 8910282 [details] [diff] [review]:
-----------------------------------------------------------------

A mozreview request would be nicer, maybe you plan one now that the dependency has been fixed. Until then a question I would like see answered first. Thanks.

::: testing/marionette/driver.js
@@ +3720,2 @@
>   */
> +async function restoreWindow(chromeWindow, contentWindow) {

Why can't we add this method to the BrowserContext class, which has both the information for chrome and content window available, and IMO would be a better fit for this feature.
Attachment #8910282 - Flags: review?(hskupin)
(In reply to Henrik Skupin (:whimboo) from comment #3)
> ::: testing/marionette/driver.js
> @@ +3720,2 @@
> >   */
> > +async function restoreWindow(chromeWindow, contentWindow) {
> 
> Why can't we add this method to the BrowserContext class, which
> has both the information for chrome and content window available,
> and IMO would be a better fit for this feature.

This will get moved to a class similar in nature to browser.Context
with http://bugzil.la/marionette-window-tracking, but considering
none of the other window manipulation helpers are there, it makes
sense to leave it in its current place for now.
Comment on attachment 8910282 [details] [diff] [review]
Wait for visibilitychange event on window minimize/restore. r?whimboo

Can we get this patch landed soon so that we can resolve
https://bugzil.la/1397007?
Attachment #8910282 - Flags: review?(hskupin)
Attachment #8910282 - Flags: review?(hskupin) → review+
Pushed by atolfsen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6470a12114de
Wait for visibilitychange event on window minimize/restore. r=whimboo
https://hg.mozilla.org/mozilla-central/rev/6470a12114de
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.