On Ubuntu 18.04 "window.moveTo()" shows asynchronous behavior
Categories
(Core :: Widget: Gtk, defect)
Tracking
()
People
(Reporter: whimboo, Unassigned)
References
(Blocks 3 open bugs)
Details
As it is stated on MDN (https://developer.mozilla.org/en-US/docs/Web/API/Window/moveTo) or by several browser chrome tests (https://searchfox.org/mozilla-central/search?q=window.moveTo&path=) this method is synchronous.
But since Edwin started to experiment with Ubuntu 18.04 the behavior seems to have been changed. Several of tests in our CI are failing because the window hasn't been moved yet when the call to moveTo() returned.
Over on bug 1600809 a patch got landed which fixes affected devtools tests by adding a wrapper around the call. I don't think this patch is adequate, and we should fix moveTo() to make it synchronous again.
Karl, not sure if it is you or who else could have a look at it. Help is appreciated. Thanks!
|
Reporter | |
Comment 1•4 years ago
|
||
Here some latest findings from Edwin:
(In reply to Edwin Takahashi (:egao, :etakahashi) from bug 1600391 comment 23)
> I found that if I replace the default gnome desktop environment with just the `compiz` window manager, then all tests (including this one) pass reliably.
>
> see try: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&selectedJob=279931219&revision=a5c059e5668e3db07165855f67038f3c8a07866d
>
> Having `compiz` also fixes a bunch of other tests that implicitly relied on it, resulting in fewer unexpected failures. Which is good.
>
> But, as I was about to put up a patch to use `compiz` window manager full-time in bug 1601828, it was brought to attention that this is modifying the environment for sake of the test, so I have decided against replacing `gnome` with `compiz` for the time being. Which means tests will continue to fail.
>
> So, it's definitely something to do with the gnome desktop environment that is causing this async-iness to occur when repositioning and resizing windows. Why this happens, I still don't know, and I'm running out of ideas to try.
|
||
Comment 2•4 years ago
•
|
||
Thanks for filing this on my behalf, appreciated it - I should have done this a bit earlier before spending time digging into this.
|
|
||
Comment 3•4 years ago
|
||
(disclaimer: I don't really know what I am doing with JS)
I toyed around with the SetWindowRect method in driver.js to see if I could make it synchronous, or at least add some checks or delays to allow the desktop environment to complete the resize/reposition. So far, the best I have managed is a ~40% intermittently passing rate, in this patch:
|
||
Comment 4•4 years ago
|
||
Window moves are asynchronous and always have been. It is possible that the browser is too slow to detect this on some systems, but reliable tests wait for the move to complete. Tests that don't wait will have unstable results.
The app can only request that a window is moved. The window manager decides whether or not to honor the request.
https://developer.mozilla.org/en-US/docs/Web/API/Window/moveTo doesn't clearly indicate that the method is asynchronous, but also doesn't indicate that the method should wait for a response from the window manager or return an error if the request is rejected.
It plausible that position getters could lie about the current position to make tests think that the move is synchronous, but that would come with its own set of problems. I don't think that would be beneficial change.
|
|
||
Comment 5•4 years ago
|
||
wontfix re making position getters lie or blocking on a response from the window manager.
|
|
||
Comment 6•4 years ago
|
||
To clarify, is it correct that your position is that tests that reposition or resize windows should explicitly add a wait prior to checking the status?
|
|
||
Comment 7•4 years ago
|
||
Yes, if tests need the window to be in a particular position, then they should wait for windows to be in that position.
If the unstable results are blocking other work, then the appropriate response to such tests would be to needinfo test owners, and annotate as unstable, if possible, or disable otherwise.
|
|
||
Comment 8•4 years ago
|
||
Understood, thanks for clarification. This way I can at least provide some rationale to test owners.
| Comment hidden (Intermittent Failures Robot) |
Description
•