From c33a12d8f0cfab062797beb82bf205040a5ab6e7 Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Thu, 6 Feb 2020 19:01:03 -0800
Subject: [PATCH] feat(firefox): ensure that new pages get browser context
 userAgent option (#872)

---
 package.json                |  2 +-
 src/firefox/ffBrowser.ts    | 12 +++++++++++-
 src/firefox/ffConnection.ts |  2 +-
 src/firefox/ffPage.ts       |  2 --
 test/browsercontext.spec.js | 13 +++++++++++++
 5 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/package.json b/package.json
index f429302957..2a6db56d23 100644
--- a/package.json
+++ b/package.json
@@ -9,7 +9,7 @@
   "main": "index.js",
   "playwright": {
     "chromium_revision": "737027",
-    "firefox_revision": "1021",
+    "firefox_revision": "1025",
     "webkit_revision": "1134"
   },
   "scripts": {
diff --git a/src/firefox/ffBrowser.ts b/src/firefox/ffBrowser.ts
index a67df172d2..86f1dd5777 100644
--- a/src/firefox/ffBrowser.ts
+++ b/src/firefox/ffBrowser.ts
@@ -56,6 +56,7 @@ export class FFBrowser extends platform.EventEmitter implements Browser {
       helper.addEventListener(this._connection, 'Target.targetCreated', this._onTargetCreated.bind(this)),
       helper.addEventListener(this._connection, 'Target.targetDestroyed', this._onTargetDestroyed.bind(this)),
       helper.addEventListener(this._connection, 'Target.targetInfoChanged', this._onTargetInfoChanged.bind(this)),
+      helper.addEventListener(this._connection, 'Target.attachedToTarget', this._onAttachedToTarget.bind(this)),
     ];
   }
 
@@ -64,7 +65,9 @@ export class FFBrowser extends platform.EventEmitter implements Browser {
   }
 
   async newContext(options: BrowserContextOptions = {}): Promise<BrowserContext> {
-    const {browserContextId} = await this._connection.send('Target.createBrowserContext');
+    const {browserContextId} = await this._connection.send('Target.createBrowserContext', {
+      userAgent: options.userAgent
+    });
     // TODO: move ignoreHTTPSErrors to browser context level.
     if (options.ignoreHTTPSErrors)
       await this._connection.send('Browser.setIgnoreHTTPSErrors', { enabled: true });
@@ -143,6 +146,13 @@ export class FFBrowser extends platform.EventEmitter implements Browser {
     target._url = url;
   }
 
+  _onAttachedToTarget(payload: Protocol.Target.attachedToTargetPayload) {
+    const {targetId, type} = payload.targetInfo;
+    const target = this._targets.get(targetId)!;
+    if (type === 'page')
+      target.page();
+  }
+
   async close() {
     await Promise.all(this.browserContexts().map(context => context.close()));
     helper.removeEventListeners(this._eventListeners);
diff --git a/src/firefox/ffConnection.ts b/src/firefox/ffConnection.ts
index a6da12347e..8ccb0f1c97 100644
--- a/src/firefox/ffConnection.ts
+++ b/src/firefox/ffConnection.ts
@@ -99,7 +99,7 @@ export class FFConnection extends platform.EventEmitter {
       const sessionId = object.params.sessionId;
       const session = new FFSession(this, object.params.targetInfo.type, sessionId, message => this._rawSend({...message, sessionId}));
       this._sessions.set(sessionId, session);
-    } else if (object.method === 'Browser.detachedFromTarget') {
+    } else if (object.method === 'Target.detachedFromTarget') {
       const session = this._sessions.get(object.params.sessionId);
       if (session) {
         session._onClosed();
diff --git a/src/firefox/ffPage.ts b/src/firefox/ffPage.ts
index 0860e76f79..12666b6a4d 100644
--- a/src/firefox/ffPage.ts
+++ b/src/firefox/ffPage.ts
@@ -89,8 +89,6 @@ export class FFPage implements PageDelegate {
       promises.push(this._session.send('Page.setBypassCSP', { enabled: true }));
     if (options.javaScriptEnabled === false)
       promises.push(this._session.send('Page.setJavascriptEnabled', { enabled: false }));
-    if (options.userAgent)
-      promises.push(this._session.send('Page.setUserAgent', { userAgent: options.userAgent }));
     await Promise.all(promises);
   }
 
diff --git a/test/browsercontext.spec.js b/test/browsercontext.spec.js
index b046b2c3ba..3eecf0aa22 100644
--- a/test/browsercontext.spec.js
+++ b/test/browsercontext.spec.js
@@ -31,6 +31,19 @@ module.exports.describe = function({testRunner, expect, playwright, CHROMIUM, WE
       await context.close();
       expect(browser.browserContexts().length).toBe(0);
     });
+    it.skip(CHROMIUM)('popup should inherit user agent', async function({newContext, server}) {
+      const context = await newContext({
+        userAgent: 'hey'
+      });
+      const page = await context.newPage();
+      await page.goto(server.EMPTY_PAGE);
+      const evaluatePromise = page.evaluate(url => window.open(url), server.PREFIX + '/dummy.html');
+      const popupPromise = page.waitForEvent('popup');
+      const request = await server.waitForRequest('/dummy.html');
+      await evaluatePromise;
+      await popupPromise;
+      expect(request.headers['user-agent']).toBe('hey');
+    });
     it('window.open should use parent tab context', async function({newContext, server}) {
       const context = await newContext();
       const page = await context.newPage();