fix(ui-mode/tv): do not open new tab on macOS (#26877)

Fixes https://github.com/microsoft/playwright/issues/9434
2025-06-26 21:40:17 +00:00 · 2023-09-06 22:57:29 +02:00 · 2023-09-06 22:57:29 +02:00 · d79dad09e8
commit d79dad09e8
parent 0527006a54
8 changed files with 108 additions and 98 deletions
--- a/packages/playwright-core/src/server/DEPS.list
+++ b/packages/playwright-core/src/server/DEPS.list
@ -13,7 +13,6 @@
 ./recorder/
 ./registry/
 ./trace/recorder/tracing.ts
-./chromium/crApp.ts

 [playwright.ts]
 ./android/
--- a/packages/playwright-core/src/server/browserType.ts
+++ b/packages/playwright-core/src/server/browserType.ts
@ -20,7 +20,7 @@ import path from 'path';
 import type { BrowserContext } from './browserContext';
 import { normalizeProxySettings, validateBrowserContextOptions } from './browserContext';
 import type { BrowserName } from './registry';
-import { findChromiumChannel, registry } from './registry';
+import { registry } from './registry';
 import type { ConnectionTransport } from './transport';
 import { WebSocketTransport } from './transport';
 import type { BrowserOptions, Browser, BrowserProcess } from './browser';
@ -32,14 +32,13 @@ import { ProgressController } from './progress';
 import type * as types from './types';
 import type * as channels from '@protocol/channels';
 import { DEFAULT_TIMEOUT, TimeoutSettings } from '../common/timeoutSettings';
-import { debugMode, isUnderTest } from '../utils';
+import { debugMode } from '../utils';
 import { existsAsync } from '../utils/fileUtils';
 import { helper } from './helper';
 import { RecentLogsCollector } from '../common/debugLogger';
 import type { CallMetadata } from './instrumentation';
-import { SdkObject, serverSideCallMetadata } from './instrumentation';
+import { SdkObject } from './instrumentation';
 import { ManualPromise } from '../utils/manualPromise';
-import { installAppIcon } from './chromium/crApp';

 export const kNoXServerRunningError = 'Looks like you launched a headed browser without having a XServer running.\n' +
  'Set either \'headless: true\' or use \'xvfb-run <your-playwright-app>\' before running Playwright.\n\n<3 Playwright Team';
@ -85,38 +84,6 @@ export abstract class BrowserType extends SdkObject {
    return browser._defaultContext!;
  }

-  async launchApp(options: {
-    sdkLanguage: string,
-    windowSize: types.Size,
-    windowPosition?: types.Point,
-    persistentContextOptions?: Parameters<BrowserType['launchPersistentContext']>[2];
-  }) {
-    const args = [...options.persistentContextOptions?.args ?? []];
-
-    if (this._name === 'chromium') {
-      args.push(
-          '--app=data:text/html,',
-          `--window-size=${options.windowSize.width},${options.windowSize.height}`,
-          ...(options.windowPosition ? [`--window-position=${options.windowPosition.x},${options.windowPosition.y}`] : []),
-          '--test-type=',
-      );
-    }
-
-    const context = await this.launchPersistentContext(serverSideCallMetadata(), '', {
-      channel: findChromiumChannel(options.sdkLanguage),
-      noDefaultViewport: true,
-      ignoreDefaultArgs: ['--enable-automation'],
-      colorScheme: 'no-override',
-      acceptDownloads: isUnderTest() ? 'accept' : 'internal-browser-default',
-      ...options?.persistentContextOptions,
-      args,
-    });
-    const [page] = context.pages();
-    if (this._name === 'chromium')
-      await installAppIcon(page);
-    return { context, page };
-  }
-
  async _innerLaunchWithRetries(progress: Progress, options: types.LaunchOptions, persistent: channels.BrowserNewContextParams | undefined, protocolLogger: types.ProtocolLogger, userDataDir?: string): Promise<Browser> {
    try {
      return await this._innerLaunch(progress, options, persistent, protocolLogger, userDataDir);
--- a/packages/playwright-core/src/server/chromium/crApp.ts
+++ b/packages/playwright-core/src/server/chromium/crApp.ts
@ -1,53 +0,0 @@
-/**
- * Copyright (c) Microsoft Corporation.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-import fs from 'fs';
-import path from 'path';
-import { isUnderTest } from '../../utils';
-import type { Page } from '../page';
-import { registryDirectory } from '../registry';
-import type { CRPage } from './crPage';
-
-export async function installAppIcon(page: Page) {
-  const icon = await fs.promises.readFile(require.resolve('./appIcon.png'));
-  const crPage = page._delegate as CRPage;
-  await crPage._mainFrameSession._client.send('Browser.setDockTile', {
-    image: icon.toString('base64')
-  });
-}
-
-export async function syncLocalStorageWithSettings(page: Page, appName: string) {
-  if (isUnderTest())
-    return;
-  const settingsFile = path.join(registryDirectory, '.settings', `${appName}.json`);
-  await page.exposeBinding('_saveSerializedSettings', false, (_, settings) => {
-    fs.mkdirSync(path.dirname(settingsFile), { recursive: true });
-    fs.writeFileSync(settingsFile, settings);
-  });
-
-  const settings = await fs.promises.readFile(settingsFile, 'utf-8').catch(() => ('{}'));
-  await page.addInitScript(
-      `(${String((settings: any) => {
-        // iframes w/ snapshots, etc.
-        if (location && location.protocol === 'data:')
-          return;
-        Object.entries(settings).map(([k, v]) => localStorage[k] = v);
-        (window as any).saveSettings = () => {
-          (window as any)._saveSerializedSettings(JSON.stringify({ ...localStorage }));
-        };
-      })})(${settings});
-  `);
-}
--- a/packages/playwright-core/src/server/launchApp.ts
+++ b/packages/playwright-core/src/server/launchApp.ts
@ -0,0 +1,99 @@
+/**
+ * Copyright (c) Microsoft Corporation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import fs from 'fs';
+import path from 'path';
+import type { Page } from './page';
+import { findChromiumChannel } from './registry';
+import { isUnderTest } from '../utils';
+import { serverSideCallMetadata } from './instrumentation';
+import type * as types from './types';
+import type { BrowserType } from './browserType';
+import type { CRPage } from './chromium/crPage';
+import { registryDirectory } from './registry';
+
+export async function launchApp(browserType: BrowserType, options: {
+  sdkLanguage: string,
+  windowSize: types.Size,
+  windowPosition?: types.Point,
+  persistentContextOptions?: Parameters<BrowserType['launchPersistentContext']>[2];
+}) {
+  const args = [...options.persistentContextOptions?.args ?? []];
+
+  if (browserType.name() === 'chromium') {
+    args.push(
+        '--app=data:text/html,',
+        `--window-size=${options.windowSize.width},${options.windowSize.height}`,
+        ...(options.windowPosition ? [`--window-position=${options.windowPosition.x},${options.windowPosition.y}`] : []),
+        '--test-type=',
+    );
+  }
+
+  const context = await browserType.launchPersistentContext(serverSideCallMetadata(), '', {
+    channel: findChromiumChannel(options.sdkLanguage),
+    noDefaultViewport: true,
+    ignoreDefaultArgs: ['--enable-automation'],
+    colorScheme: 'no-override',
+    acceptDownloads: isUnderTest() ? 'accept' : 'internal-browser-default',
+    ...options?.persistentContextOptions,
+    args,
+  });
+  const [page] = context.pages();
+  // Chromium on macOS opens a new tab when clicking on the dock icon.
+  // See https://github.com/microsoft/playwright/issues/9434
+  if (browserType.name() === 'chromium' && process.platform === 'darwin') {
+    context.on('page', async (newPage: Page) => {
+      if (newPage.mainFrame().url() === 'chrome://new-tab-page/') {
+        await page.bringToFront();
+        await newPage.close(serverSideCallMetadata());
+      }
+    });
+  }
+  if (browserType.name() === 'chromium')
+    await installAppIcon(page);
+  return { context, page };
+}
+
+async function installAppIcon(page: Page) {
+  const icon = await fs.promises.readFile(require.resolve('./chromium/appIcon.png'));
+  const crPage = page._delegate as CRPage;
+  await crPage._mainFrameSession._client.send('Browser.setDockTile', {
+    image: icon.toString('base64')
+  });
+}
+
+export async function syncLocalStorageWithSettings(page: Page, appName: string) {
+  if (isUnderTest())
+    return;
+  const settingsFile = path.join(registryDirectory, '.settings', `${appName}.json`);
+  await page.exposeBinding('_saveSerializedSettings', false, (_, settings) => {
+    fs.mkdirSync(path.dirname(settingsFile), { recursive: true });
+    fs.writeFileSync(settingsFile, settings);
+  });
+
+  const settings = await fs.promises.readFile(settingsFile, 'utf-8').catch(() => ('{}'));
+  await page.addInitScript(
+      `(${String((settings: any) => {
+        // iframes w/ snapshots, etc.
+        if (location && location.protocol === 'data:')
+          return;
+        Object.entries(settings).map(([k, v]) => localStorage[k] = v);
+        (window as any).saveSettings = () => {
+          (window as any)._saveSerializedSettings(JSON.stringify({ ...localStorage }));
+        };
+      })})(${settings});
+  `);
+}
--- a/packages/playwright-core/src/server/recorder/DEPS.list
+++ b/packages/playwright-core/src/server/recorder/DEPS.list
@ -7,6 +7,3 @@
 ../../utils/**
 ../../utilsBundle.ts
 ../../zipBundle.ts
-
-[recorderApp.ts]
-../chromium/crApp.ts
--- a/packages/playwright-core/src/server/recorder/recorderApp.ts
+++ b/packages/playwright-core/src/server/recorder/recorderApp.ts
@ -23,9 +23,10 @@ import { serverSideCallMetadata } from '../instrumentation';
 import type { CallLog, EventData, Mode, Source } from '@recorder/recorderTypes';
 import { isUnderTest } from '../../utils';
 import { mime } from '../../utilsBundle';
-import { syncLocalStorageWithSettings } from '../chromium/crApp';
+import { syncLocalStorageWithSettings } from '../launchApp';
 import type { Recorder } from '../recorder';
 import type { BrowserContext } from '../browserContext';
+import { launchApp } from '../launchApp';

 declare global {
  interface Window {
@ -115,7 +116,7 @@ export class RecorderApp extends EventEmitter implements IRecorderApp {
    const sdkLanguage = inspectedContext.attribution.playwright.options.sdkLanguage;
    const headed = !!inspectedContext._browser.options.headful;
    const recorderPlaywright = (require('../playwright').createPlaywright as typeof import('../playwright').createPlaywright)({ sdkLanguage: 'javascript', isInternalPlaywright: true });
-    const { context, page } = await recorderPlaywright.chromium.launchApp({
+    const { context, page } = await launchApp(recorderPlaywright.chromium, {
      sdkLanguage,
      windowSize: { width: 600, height: 600 },
      windowPosition: { x: 1020, y: 10 },
--- a/packages/playwright-core/src/server/trace/viewer/DEPS.list
+++ b/packages/playwright-core/src/server/trace/viewer/DEPS.list
@ -4,4 +4,3 @@
 ../../../generated/
 ../../../utils/
 ../../../utilsBundle.ts
-../../chromium/crApp.ts
--- a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts
+++ b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts
@ -18,13 +18,14 @@ import path from 'path';
 import fs from 'fs';
 import { HttpServer } from '../../../utils/httpServer';
 import { createGuid, gracefullyProcessExitDoNotHang, isUnderTest } from '../../../utils';
-import { syncLocalStorageWithSettings } from '../../chromium/crApp';
+import { syncLocalStorageWithSettings } from '../../launchApp';
 import { serverSideCallMetadata } from '../../instrumentation';
 import { createPlaywright } from '../../playwright';
 import { ProgressController } from '../../progress';
 import { open, wsServer } from '../../../utilsBundle';
 import type { Page } from '../../page';
 import type { BrowserType } from '../../browserType';
+import { launchApp } from '../../launchApp';

 export type Transport = {
  sendEvent?: (method: string, params: any) => void;
@ -133,7 +134,7 @@ export async function openTraceViewerApp(traceUrls: string[], browserName: strin
  const traceViewerPlaywright = createPlaywright({ sdkLanguage: 'javascript', isInternalPlaywright: true });
  const traceViewerBrowser = isUnderTest() ? 'chromium' : browserName;

-  const { context, page } = await traceViewerPlaywright[traceViewerBrowser as 'chromium'].launchApp({
+  const { context, page } = await launchApp(traceViewerPlaywright[traceViewerBrowser as 'chromium'], {
    // TODO: store language in the trace.
    sdkLanguage: traceViewerPlaywright.options.sdkLanguage,
    windowSize: { width: 1280, height: 800 },