From b473d9dcf73b890a9d64ddaed128913ac0232aea Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Thu, 26 Mar 2020 16:13:11 -0700 Subject: [PATCH] chore(firefox): remove FFPage._initialize to ensure early initialization (#1554) --- src/firefox/ffPage.ts | 23 ++++++----------------- test/emulation.spec.js | 10 ++++++++-- test/popup.spec.js | 7 ++----- 3 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts index e9585f8268..3c923e6414 100644 --- a/src/firefox/ffPage.ts +++ b/src/firefox/ffPage.ts @@ -83,24 +83,13 @@ export class FFPage implements PageDelegate { ]; this._pagePromise = new Promise(f => this._pageCallback = f); session.once(FFSessionEvents.Disconnected, () => this._page._didDisconnect()); - this._initialize(); - } - - async _initialize() { - try { - await Promise.all([ - // TODO: we should get rid of this call to resolve before any early events arrive, e.g. dialogs. - this._session.send('Page.addScriptToEvaluateOnNewDocument', { - script: '', - worldName: UTILITY_WORLD_NAME, - }), - new Promise(f => this._session.once('Page.ready', f)), - ]); + this._session.once('Page.ready', () => { this._pageCallback(this._page); - } catch (e) { - this._pageCallback(e); - } - this._initialized = true; + this._initialized = true; + }); + // Ideally, we somehow ensure that utility world is created before Page.ready arrives, but currently it is racy. + // Therefore, we can end up with an initialized page without utility world, although very unlikely. + this._session.send('Page.addScriptToEvaluateOnNewDocument', { script: '', worldName: UTILITY_WORLD_NAME }).catch(this._pageCallback); } _initializedPage(): Page | null { diff --git a/test/emulation.spec.js b/test/emulation.spec.js index a0928cfeee..eeaf53035e 100644 --- a/test/emulation.spec.js +++ b/test/emulation.spec.js @@ -399,9 +399,15 @@ module.exports.describe = function({testRunner, expect, playwright, headless, FF }); }); - describe('focus', function() { - it.fail(!headless)('should think that it is focused by default', async({page}) => { + describe.fail(!headless)('focus', function() { + it('should think that it is focused by default', async({page}) => { expect(await page.evaluate('document.hasFocus()')).toBe(true); }); + it.fail(FFOX)('should think that all pages are focused', async({page}) => { + const page2 = await page.context().newPage(); + expect(await page.evaluate('document.hasFocus()')).toBe(true); + expect(await page2.evaluate('document.hasFocus()')).toBe(true); + await page2.close(); + }); }); }; diff --git a/test/popup.spec.js b/test/popup.spec.js index fe066be77b..71ccf8a911 100644 --- a/test/popup.spec.js +++ b/test/popup.spec.js @@ -220,14 +220,11 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE expect(popup).toBeTruthy(); await context.close(); }); - it.fail(FFOX)('should be able to capture alert', async({browser}) => { - // Firefox: - // - immediately closes dialog by itself, without protocol call; - // - waits for Page.addScriptToEvaluateOnNewDocument before resolving page(), which is too late. + it('should be able to capture alert', async({browser}) => { const context = await browser.newContext(); const page = await context.newPage(); const evaluatePromise = page.evaluate(() => { - const win = window.open('about:blank'); + const win = window.open(''); win.alert('hello'); }); const popup = await page.waitForEvent('popup');