chore(firefox): remove FFPage._initialize to ensure early initializat…

…ion (#1554)
microsoft · Mar 26, 2020 · b473d9d · b473d9d
1 parent f420cbb
commit b473d9d
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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');