Closed Bug 1421028 Opened 6 years ago Closed 6 years ago

browser/modules/test/browser/browser_urlBar_zoom.js should wait for stable state before testing

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 59
Tracking Status
firefox59 --- fixed

People

(Reporter: arai, Assigned: arai)

References

Details

Attachments

(1 file)

https://searchfox.org/mozilla-central/rev/8839daefd69087d7ac2655b72790d3a25b6a815c/browser/modules/test/browser/browser_urlBar_zoom.js#20-22
>   await labelUpdatePromise;
>   info("Zoom increased to " + Math.floor(ZoomManager.zoom * 100) + "%");
>   is(zoomResetButton.hidden, false, "Zoom reset button is now visible");

the test fails after bug 1414180 (actually one bug that will block it, not yet filed), because of the test is running too early.
the test fails if I put `await new Promise(r => setTimeout(r, 1000));` after `await labelUpdatePromise;`, because the updateZoomUI is called while the wait,
and it resets the zoom level to 100% (at least for button's PoV).

https://dxr.mozilla.org/mozilla-central/rev/da90245d47b17c750560dedb5cbe1973181166e3/browser/modules/ZoomUI.jsm#67
> function updateZoomUI(aBrowser, aAnimate = false) {
> ...

it's called from FullZoom__notifyOnLocationChange.
https://dxr.mozilla.org/mozilla-central/rev/da90245d47b17c750560dedb5cbe1973181166e3/browser/base/content/browser-fullZoom.js#526-530
>   _notifyOnLocationChange: function FullZoom__notifyOnLocationChange(browser) {
>     this._executeSoon(function() {
>       Services.obs.notifyObservers(browser, "browser-fullZoom:location-change");
>     });
>   },

currently the test is passing because it's checking the state before the reset happens.

the test should wait for the stable state (all OnLocationChange event is dispatched), before starting testing.
(In reply to Tooru Fujisawa [:arai] from comment #0)
> the test fails if I put `await new Promise(r => setTimeout(r, 1000));` after
> `await labelUpdatePromise;`

to be clear, this fails even before bug 1414180 fix.
actually, browser-fullZoom is using executeSoon (Services.tm.dispatchToMainThread),
so the test should wait more.
(or perhaps browser-fullZoom should detect the collision...?)
Assignee: nobody → arai.unmht
Status: NEW → ASSIGNED
The easiest way here is to create new tab and wait for stable state there, so that the test can start from stable state regardless of the beginning state of the test itself.
Attachment #8932210 - Flags: review?(dtownsend)
(In reply to Tooru Fujisawa [:arai] from comment #3)
> Wair for stable state before starting in

oops, I'll fix the typo (Wair => Wait) when landing.
Attachment #8932210 - Flags: review?(dtownsend) → review+
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5cd84aa65776
Wait for stable state before starting in browser/modules/test/browser/browser_urlBar_zoom.js. r=Mossop
https://hg.mozilla.org/integration/mozilla-inbound/rev/5cd84aa657765868aa55c08f0d88df422eb5975c
Bug 1421028 - Wait for stable state before starting in browser/modules/test/browser/browser_urlBar_zoom.js. r=Mossop
https://hg.mozilla.org/mozilla-central/rev/5cd84aa65776
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 59
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: