From 2abd772c9c3d2b86deb71541eaac41aceef94356 Mon Sep 17 00:00:00 2001 From: Alex Rudenko Date: Tue, 2 Aug 2022 10:50:05 +0200 Subject: [PATCH] feat: detect Firefox in connect() automatically (#8718) This PR implements automatic detection of the Firefox product when the `.connect()` method is used. This partially undoes the breaking change in https://github.com/puppeteer/puppeteer/pull/8520 but it's also a breaking change on its own since we don't accept an explicit product name anymore (it does not look like it was used anyway). --- docs/api/puppeteer.connectoptions.md | 1 - docs/api/puppeteer.connectoptions.product.md | 13 -------- src/common/BrowserConnector.ts | 8 +++-- src/common/Puppeteer.ts | 2 -- src/node/Puppeteer.ts | 3 -- test/src/browser.spec.ts | 6 ++-- test/src/fixtures.spec.ts | 4 +-- test/src/launcher.spec.ts | 34 ++++++-------------- 8 files changed, 17 insertions(+), 54 deletions(-) delete mode 100644 docs/api/puppeteer.connectoptions.product.md diff --git a/docs/api/puppeteer.connectoptions.md b/docs/api/puppeteer.connectoptions.md index f8db000b..fa3b691a 100644 --- a/docs/api/puppeteer.connectoptions.md +++ b/docs/api/puppeteer.connectoptions.md @@ -18,5 +18,4 @@ export interface ConnectOptions extends BrowserConnectOptions | --------------------------------------------------------------------- | --------- | --------------------------------------------------------- | ----------------- | | [browserURL?](./puppeteer.connectoptions.browserurl.md) | | string | (Optional) | | [browserWSEndpoint?](./puppeteer.connectoptions.browserwsendpoint.md) | | string | (Optional) | -| [product?](./puppeteer.connectoptions.product.md) | | [Product](./puppeteer.product.md) | (Optional) | | [transport?](./puppeteer.connectoptions.transport.md) | | [ConnectionTransport](./puppeteer.connectiontransport.md) | (Optional) | diff --git a/docs/api/puppeteer.connectoptions.product.md b/docs/api/puppeteer.connectoptions.product.md deleted file mode 100644 index d3f4d3b2..00000000 --- a/docs/api/puppeteer.connectoptions.product.md +++ /dev/null @@ -1,13 +0,0 @@ ---- -sidebar_label: ConnectOptions.product ---- - -# ConnectOptions.product property - -**Signature:** - -```typescript -interface ConnectOptions { - product?: Product; -} -``` diff --git a/src/common/BrowserConnector.ts b/src/common/BrowserConnector.ts index be425b88..3d4346f2 100644 --- a/src/common/BrowserConnector.ts +++ b/src/common/BrowserConnector.ts @@ -26,7 +26,6 @@ import {Connection} from './Connection.js'; import {ConnectionTransport} from './ConnectionTransport.js'; import {getFetch} from './fetch.js'; import {Viewport} from './PuppeteerViewport.js'; -import {Product} from './Product.js'; /** * Generic browser options that can be passed when launching any browser or when * connecting to an existing browser instance. @@ -75,7 +74,6 @@ export async function _connectToBrowser( browserWSEndpoint?: string; browserURL?: string; transport?: ConnectionTransport; - product?: Product; } ): Promise { const { @@ -87,7 +85,6 @@ export async function _connectToBrowser( slowMo = 0, targetFilter, _isPageTarget: isPageTarget, - product, } = options; assert( @@ -111,6 +108,11 @@ export async function _connectToBrowser( await WebSocketClass.create(connectionURL); connection = new Connection(connectionURL, connectionTransport, slowMo); } + const version = await connection.send('Browser.getVersion'); + + const product = version.product.toLowerCase().includes('firefox') + ? 'firefox' + : 'chrome'; const {browserContextIds} = await connection.send( 'Target.getBrowserContexts' diff --git a/src/common/Puppeteer.ts b/src/common/Puppeteer.ts index 085d5b8f..d160cbe2 100644 --- a/src/common/Puppeteer.ts +++ b/src/common/Puppeteer.ts @@ -19,7 +19,6 @@ import {ConnectionTransport} from './ConnectionTransport.js'; import {devices} from './DeviceDescriptors.js'; import {errors} from './Errors.js'; import {networkConditions} from './NetworkConditions.js'; -import {Product} from './Product.js'; import { clearCustomQueryHandlers, CustomQueryHandler, @@ -43,7 +42,6 @@ export interface ConnectOptions extends BrowserConnectOptions { browserWSEndpoint?: string; browserURL?: string; transport?: ConnectionTransport; - product?: Product; } /** diff --git a/src/node/Puppeteer.ts b/src/node/Puppeteer.ts index df484230..8c639b5f 100644 --- a/src/node/Puppeteer.ts +++ b/src/node/Puppeteer.ts @@ -110,9 +110,6 @@ export class PuppeteerNode extends Puppeteer { * @returns Promise which resolves to browser instance. */ override connect(options: ConnectOptions): Promise { - if (options.product) { - this._productName = options.product; - } return super.connect(options); } diff --git a/test/src/browser.spec.ts b/test/src/browser.spec.ts index 0054a651..375b3dba 100644 --- a/test/src/browser.spec.ts +++ b/test/src/browser.spec.ts @@ -63,12 +63,11 @@ describe('Browser specs', function () { expect(process!.pid).toBeGreaterThan(0); }); it('should not return child_process for remote browser', async () => { - const {browser, puppeteer, isFirefox} = getTestState(); + const {browser, puppeteer} = getTestState(); const browserWSEndpoint = browser.wsEndpoint(); const remoteBrowser = await puppeteer.connect({ browserWSEndpoint, - product: isFirefox ? 'firefox' : 'chrome', }); expect(remoteBrowser.process()).toBe(null); remoteBrowser.disconnect(); @@ -77,12 +76,11 @@ describe('Browser specs', function () { describe('Browser.isConnected', () => { it('should set the browser connected state', async () => { - const {browser, puppeteer, isFirefox} = getTestState(); + const {browser, puppeteer} = getTestState(); const browserWSEndpoint = browser.wsEndpoint(); const newBrowser = await puppeteer.connect({ browserWSEndpoint, - product: isFirefox ? 'firefox' : 'chrome', }); expect(newBrowser.isConnected()).toBe(true); newBrowser.disconnect(); diff --git a/test/src/fixtures.spec.ts b/test/src/fixtures.spec.ts index bc606f23..e20054dc 100644 --- a/test/src/fixtures.spec.ts +++ b/test/src/fixtures.spec.ts @@ -68,8 +68,7 @@ describe('Fixtures', function () { expect(dumpioData).toContain('DevTools listening on ws://'); }); it('should close the browser when the node process closes', async () => { - const {defaultBrowserOptions, puppeteerPath, puppeteer, isFirefox} = - getTestState(); + const {defaultBrowserOptions, puppeteerPath, puppeteer} = getTestState(); const {spawn, execSync} = await import('child_process'); const options = Object.assign({}, defaultBrowserOptions, { @@ -94,7 +93,6 @@ describe('Fixtures', function () { }); const browser = await puppeteer.connect({ browserWSEndpoint: await wsEndPointPromise, - product: isFirefox ? 'firefox' : 'chrome', }); const promises = [ new Promise(resolve => { diff --git a/test/src/launcher.spec.ts b/test/src/launcher.spec.ts index b1ef5ee3..51a2d0eb 100644 --- a/test/src/launcher.spec.ts +++ b/test/src/launcher.spec.ts @@ -139,13 +139,11 @@ describe('Launcher specs', function () { describe('Browser.disconnect', function () { it('should reject navigation when browser closes', async () => { - const {server, puppeteer, defaultBrowserOptions, isFirefox} = - getTestState(); + const {server, puppeteer, defaultBrowserOptions} = getTestState(); server.setRoute('/one-style.css', () => {}); const browser = await puppeteer.launch(defaultBrowserOptions); const remote = await puppeteer.connect({ browserWSEndpoint: browser.wsEndpoint(), - product: isFirefox ? 'firefox' : 'chrome', }); const page = await remote.newPage(); const navigationPromise = page @@ -165,14 +163,12 @@ describe('Launcher specs', function () { await browser.close(); }); it('should reject waitForSelector when browser closes', async () => { - const {server, puppeteer, defaultBrowserOptions, isFirefox} = - getTestState(); + const {server, puppeteer, defaultBrowserOptions} = getTestState(); server.setRoute('/empty.html', () => {}); const browser = await puppeteer.launch(defaultBrowserOptions); const remote = await puppeteer.connect({ browserWSEndpoint: browser.wsEndpoint(), - product: isFirefox ? 'firefox' : 'chrome', }); const page = await remote.newPage(); const watchdog = page @@ -188,13 +184,11 @@ describe('Launcher specs', function () { }); describe('Browser.close', function () { it('should terminate network waiters', async () => { - const {server, puppeteer, defaultBrowserOptions, isFirefox} = - getTestState(); + const {server, puppeteer, defaultBrowserOptions} = getTestState(); const browser = await puppeteer.launch(defaultBrowserOptions); const remote = await puppeteer.connect({ browserWSEndpoint: browser.wsEndpoint(), - product: isFirefox ? 'firefox' : 'chrome', }); const newPage = await remote.newPage(); const results = await Promise.all([ @@ -671,12 +665,11 @@ describe('Launcher specs', function () { describe('Puppeteer.connect', function () { it('should be able to connect multiple times to the same browser', async () => { - const {puppeteer, defaultBrowserOptions, isFirefox} = getTestState(); + const {puppeteer, defaultBrowserOptions} = getTestState(); const originalBrowser = await puppeteer.launch(defaultBrowserOptions); const otherBrowser = await puppeteer.connect({ browserWSEndpoint: originalBrowser.wsEndpoint(), - product: isFirefox ? 'firefox' : 'chrome', }); const page = await otherBrowser.newPage(); expect( @@ -695,12 +688,11 @@ describe('Launcher specs', function () { await originalBrowser.close(); }); it('should be able to close remote browser', async () => { - const {defaultBrowserOptions, puppeteer, isFirefox} = getTestState(); + const {defaultBrowserOptions, puppeteer} = getTestState(); const originalBrowser = await puppeteer.launch(defaultBrowserOptions); const remoteBrowser = await puppeteer.connect({ browserWSEndpoint: originalBrowser.wsEndpoint(), - product: isFirefox ? 'firefox' : 'chrome', }); await Promise.all([ utils.waitEvent(originalBrowser, 'disconnected'), @@ -708,8 +700,7 @@ describe('Launcher specs', function () { ]); }); it('should support ignoreHTTPSErrors option', async () => { - const {httpsServer, puppeteer, defaultBrowserOptions, isFirefox} = - getTestState(); + const {httpsServer, puppeteer, defaultBrowserOptions} = getTestState(); const originalBrowser = await puppeteer.launch(defaultBrowserOptions); const browserWSEndpoint = originalBrowser.wsEndpoint(); @@ -717,7 +708,6 @@ describe('Launcher specs', function () { const browser = await puppeteer.connect({ browserWSEndpoint, ignoreHTTPSErrors: true, - product: isFirefox ? 'firefox' : 'chrome', }); const page = await browser.newPage(); let error!: Error; @@ -739,8 +729,7 @@ describe('Launcher specs', function () { }); // @see https://github.com/puppeteer/puppeteer/issues/4197 itFailsFirefox('should support targetFilter option', async () => { - const {server, puppeteer, defaultBrowserOptions, isFirefox} = - getTestState(); + const {server, puppeteer, defaultBrowserOptions} = getTestState(); const originalBrowser = await puppeteer.launch(defaultBrowserOptions); const browserWSEndpoint = originalBrowser.wsEndpoint(); @@ -753,7 +742,6 @@ describe('Launcher specs', function () { const browser = await puppeteer.connect({ browserWSEndpoint, - product: isFirefox ? 'firefox' : 'chrome', targetFilter: (targetInfo: Protocol.Target.TargetInfo) => { return !targetInfo.url?.includes('should-be-ignored'); }, @@ -837,8 +825,7 @@ describe('Launcher specs', function () { } ); it('should be able to reconnect', async () => { - const {puppeteer, server, defaultBrowserOptions, isFirefox} = - getTestState(); + const {puppeteer, server, defaultBrowserOptions} = getTestState(); const browserOne = await puppeteer.launch(defaultBrowserOptions); const browserWSEndpoint = browserOne.wsEndpoint(); const pageOne = await browserOne.newPage(); @@ -847,7 +834,6 @@ describe('Launcher specs', function () { const browserTwo = await puppeteer.connect({ browserWSEndpoint, - product: isFirefox ? 'firefox' : 'chrome', }); const pages = await browserTwo.pages(); const pageTwo = pages.find(page => { @@ -991,16 +977,14 @@ describe('Launcher specs', function () { itFailsFirefox( 'should be emitted when: browser gets closed, disconnected or underlying websocket gets closed', async () => { - const {puppeteer, defaultBrowserOptions, isFirefox} = getTestState(); + const {puppeteer, defaultBrowserOptions} = getTestState(); const originalBrowser = await puppeteer.launch(defaultBrowserOptions); const browserWSEndpoint = originalBrowser.wsEndpoint(); const remoteBrowser1 = await puppeteer.connect({ browserWSEndpoint, - product: isFirefox ? 'firefox' : 'chrome', }); const remoteBrowser2 = await puppeteer.connect({ browserWSEndpoint, - product: isFirefox ? 'firefox' : 'chrome', }); let disconnectedOriginal = 0;