Skip to content

Commit

Permalink
feat: detect Firefox in connect() automatically (#8718)
Browse files Browse the repository at this point in the history 
This PR implements automatic detection of the Firefox product when the `.connect()` method is used. This partially undoes the breaking change in #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).
  • Loading branch information
@OrKoN @jrandolf
OrKoN authored and jrandolf committed Aug 2, 2022
1 parent fae4fa5 commit 2abd772
Show file tree
Hide file tree
Showing 8 changed files with 17 additions and 54 deletions.
1 change: 0 additions & 1 deletion docs/api/puppeteer.connectoptions.md
View file
Expand Up @@ -18,5 +18,4 @@ export interface ConnectOptions extends BrowserConnectOptions
| --------------------------------------------------------------------- | --------- | --------------------------------------------------------- | ----------------- |
| [browserURL?](./puppeteer.connectoptions.browserurl.md) | | string | <i>(Optional)</i> |
| [browserWSEndpoint?](./puppeteer.connectoptions.browserwsendpoint.md) | | string | <i>(Optional)</i> |
| [product?](./puppeteer.connectoptions.product.md) | | [Product](./puppeteer.product.md) | <i>(Optional)</i> |
| [transport?](./puppeteer.connectoptions.transport.md) | | [ConnectionTransport](./puppeteer.connectiontransport.md) | <i>(Optional)</i> |
13 changes: 0 additions & 13 deletions docs/api/puppeteer.connectoptions.product.md
View file

This file was deleted.

8 changes: 5 additions & 3 deletions src/common/BrowserConnector.ts
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -75,7 +74,6 @@ export async function _connectToBrowser(
browserWSEndpoint?: string;
browserURL?: string;
transport?: ConnectionTransport;
product?: Product;
}
): Promise<Browser> {
const {
Expand All @@ -87,7 +85,6 @@ export async function _connectToBrowser(
slowMo = 0,
targetFilter,
_isPageTarget: isPageTarget,
product,
} = options;

assert(
Expand All @@ -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'
Expand Down
2 changes: 0 additions & 2 deletions src/common/Puppeteer.ts
View file
Expand Up @@ -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,
Expand All @@ -43,7 +42,6 @@ export interface ConnectOptions extends BrowserConnectOptions {
browserWSEndpoint?: string;
browserURL?: string;
transport?: ConnectionTransport;
product?: Product;
}

/**
Expand Down
3 changes: 0 additions & 3 deletions src/node/Puppeteer.ts
View file
Expand Up @@ -110,9 +110,6 @@ export class PuppeteerNode extends Puppeteer {
* @returns Promise which resolves to browser instance.
*/
override connect(options: ConnectOptions): Promise<Browser> {
if (options.product) {
this._productName = options.product;
}
return super.connect(options);
}

Expand Down
6 changes: 2 additions & 4 deletions test/src/browser.spec.ts
View file
Expand Up @@ -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();
Expand All @@ -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();
Expand Down
4 changes: 1 addition & 3 deletions test/src/fixtures.spec.ts
View file
Expand Up @@ -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, {
Expand All @@ -94,7 +93,6 @@ describe('Fixtures', function () {
});
const browser = await puppeteer.connect({
browserWSEndpoint: await wsEndPointPromise,
product: isFirefox ? 'firefox' : 'chrome',
});
const promises = [
new Promise(resolve => {
Expand Down
34 changes: 9 additions & 25 deletions test/src/launcher.spec.ts
View file
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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([
Expand Down Expand Up @@ -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(
Expand All @@ -695,29 +688,26 @@ 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'),
remoteBrowser.close(),
]);
});
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();

const browser = await puppeteer.connect({
browserWSEndpoint,
ignoreHTTPSErrors: true,
product: isFirefox ? 'firefox' : 'chrome',
});
const page = await browser.newPage();
let error!: Error;
Expand All @@ -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();
Expand All @@ -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');
},
Expand Down Expand Up @@ -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();
Expand All @@ -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 => {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 2abd772

Please sign in to comment.