Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: detect Firefox in connect() automatically #8718

Merged
merged 1 commit into from Aug 2, 2022
Merged

feat: detect Firefox in connect() automatically #8718

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion docs/api/puppeteer.connectoptions.md
View file
Open in desktop
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
Open in desktop

This file was deleted.

8 changes: 5 additions & 3 deletions src/common/BrowserConnector.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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