fix: download of attachments in UI Mode (#26407)

Fixes https://github.com/microsoft/playwright/issues/26326.

packages/playwright-core/src/client/browserContext.ts

@@ -451,6 +451,7 @@ export async function prepareBrowserContextParams(options: BrowserContextOptions
     colorScheme: options.colorScheme === null ? 'no-override' : options.colorScheme,
     reducedMotion: options.reducedMotion === null ? 'no-override' : options.reducedMotion,
     forcedColors: options.forcedColors === null ? 'no-override' : options.forcedColors,
+    acceptDownloads: toAcceptDownloadsProtocol(options.acceptDownloads),
   };
   if (!contextParams.recordVideo && options.videosPath) {
     contextParams.recordVideo = {
@@ -460,3 +461,11 @@ export async function prepareBrowserContextParams(options: BrowserContextOptions
   }
   return contextParams;
 }
+
+function toAcceptDownloadsProtocol(acceptDownloads?: boolean) {
+  if (acceptDownloads === undefined)
+    return undefined;
+  if (acceptDownloads === true)
+    return 'accept';
+  return 'deny';
+}

packages/playwright-core/src/client/electron.ts

@@ -29,11 +29,12 @@ import type { Page } from './page';
 import type { Env, WaitForEventOptions, Headers, BrowserContextOptions } from './types';
 import { Waiter } from './waiter';
 
-type ElectronOptions = Omit<channels.ElectronLaunchOptions, 'env'|'extraHTTPHeaders'|'recordHar'|'colorScheme'> & {
+type ElectronOptions = Omit<channels.ElectronLaunchOptions, 'env'|'extraHTTPHeaders'|'recordHar'|'colorScheme'|'acceptDownloads'> & {
   env?: Env,
   extraHTTPHeaders?: Headers,
   recordHar?: BrowserContextOptions['recordHar'],
   colorScheme?: 'dark' | 'light' | 'no-preference' | null,
+  acceptDownloads?: boolean,
 };
 
 type ElectronAppType = typeof import('electron');

packages/playwright-core/src/client/types.ts

@@ -47,7 +47,7 @@ export type SetStorageState = {
 export type LifecycleEvent = channels.LifecycleEvent;
 export const kLifecycleEvents: Set<LifecycleEvent> = new Set(['load', 'domcontentloaded', 'networkidle', 'commit']);
 
-export type BrowserContextOptions = Omit<channels.BrowserNewContextOptions, 'viewport' | 'noDefaultViewport' | 'extraHTTPHeaders' | 'storageState' | 'recordHar' | 'colorScheme' | 'reducedMotion' | 'forcedColors'> & {
+export type BrowserContextOptions = Omit<channels.BrowserNewContextOptions, 'viewport' | 'noDefaultViewport' | 'extraHTTPHeaders' | 'storageState' | 'recordHar' | 'colorScheme' | 'reducedMotion' | 'forcedColors' | 'acceptDownloads'> & {
   viewport?: Size | null;
   extraHTTPHeaders?: Headers;
   logger?: Logger;
@@ -69,6 +69,7 @@ export type BrowserContextOptions = Omit<channels.BrowserNewContextOptions, 'vie
   colorScheme?: 'dark' | 'light' | 'no-preference' | null;
   reducedMotion?: 'reduce' | 'no-preference' | null;
   forcedColors?: 'active' | 'none' | null;
+  acceptDownloads?: boolean;
 };
 
 type LaunchOverrides = {

packages/playwright-core/src/protocol/validator.ts

@@ -554,7 +554,7 @@ scheme.BrowserTypeLaunchPersistentContextParams = tObject({
   colorScheme: tOptional(tEnum(['dark', 'light', 'no-preference', 'no-override'])),
   reducedMotion: tOptional(tEnum(['reduce', 'no-preference', 'no-override'])),
   forcedColors: tOptional(tEnum(['active', 'none', 'no-override'])),
-  acceptDownloads: tOptional(tBoolean),
+  acceptDownloads: tOptional(tEnum(['accept', 'deny', 'internal-browser-default'])),
   baseURL: tOptional(tString),
   recordVideo: tOptional(tObject({
     dir: tString,
@@ -630,7 +630,7 @@ scheme.BrowserNewContextParams = tObject({
   colorScheme: tOptional(tEnum(['dark', 'light', 'no-preference', 'no-override'])),
   reducedMotion: tOptional(tEnum(['reduce', 'no-preference', 'no-override'])),
   forcedColors: tOptional(tEnum(['active', 'none', 'no-override'])),
-  acceptDownloads: tOptional(tBoolean),
+  acceptDownloads: tOptional(tEnum(['accept', 'deny', 'internal-browser-default'])),
   baseURL: tOptional(tString),
   recordVideo: tOptional(tObject({
     dir: tString,
@@ -691,7 +691,7 @@ scheme.BrowserNewContextForReuseParams = tObject({
   colorScheme: tOptional(tEnum(['dark', 'light', 'no-preference', 'no-override'])),
   reducedMotion: tOptional(tEnum(['reduce', 'no-preference', 'no-override'])),
   forcedColors: tOptional(tEnum(['active', 'none', 'no-override'])),
-  acceptDownloads: tOptional(tBoolean),
+  acceptDownloads: tOptional(tEnum(['accept', 'deny', 'internal-browser-default'])),
   baseURL: tOptional(tString),
   recordVideo: tOptional(tObject({
     dir: tString,
@@ -2212,7 +2212,7 @@ scheme.ElectronLaunchParams = tObject({
   cwd: tOptional(tString),
   env: tOptional(tArray(tType('NameValue'))),
   timeout: tOptional(tNumber),
-  acceptDownloads: tOptional(tBoolean),
+  acceptDownloads: tOptional(tEnum(['accept', 'deny', 'internal-browser-default'])),
   bypassCSP: tOptional(tBoolean),
   colorScheme: tOptional(tEnum(['dark', 'light', 'no-preference', 'no-override'])),
   extraHTTPHeaders: tOptional(tArray(tType('NameValue'))),
@@ -2442,7 +2442,7 @@ scheme.AndroidDeviceLaunchBrowserParams = tObject({
   colorScheme: tOptional(tEnum(['dark', 'light', 'no-preference', 'no-override'])),
   reducedMotion: tOptional(tEnum(['reduce', 'no-preference', 'no-override'])),
   forcedColors: tOptional(tEnum(['active', 'none', 'no-override'])),
-  acceptDownloads: tOptional(tBoolean),
+  acceptDownloads: tOptional(tEnum(['accept', 'deny', 'internal-browser-default'])),
   baseURL: tOptional(tString),
   recordVideo: tOptional(tObject({
     dir: tString,

packages/playwright-core/src/server/browserContext.ts

@@ -604,7 +604,7 @@ export function validateBrowserContextOptions(options: channels.BrowserNewContex
   if (options.noDefaultViewport && !!options.isMobile)
     throw new Error(`"isMobile" option is not supported with null "viewport"`);
   if (options.acceptDownloads === undefined)
-    options.acceptDownloads = true;
+    options.acceptDownloads = 'accept';
   if (!options.viewport && !options.noDefaultViewport)
     options.viewport = { width: 1280, height: 720 };
   if (options.recordVideo) {
@@ -685,7 +685,7 @@ const defaultNewContextParamValues: channels.BrowserNewContextForReuseParams = {
   offline: false,
   isMobile: false,
   hasTouch: false,
-  acceptDownloads: true,
+  acceptDownloads: 'accept',
   strictSelectors: false,
   serviceWorkers: 'allow',
   locale: 'en-US',

packages/playwright-core/src/server/browserType.ts

@@ -32,7 +32,7 @@ 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 } from '../utils';
+import { debugMode, isUnderTest } from '../utils';
 import { existsAsync } from '../utils/fileUtils';
 import { helper } from './helper';
 import { RecentLogsCollector } from '../common/debugLogger';
@@ -107,6 +107,7 @@ export abstract class BrowserType extends SdkObject {
       noDefaultViewport: true,
       ignoreDefaultArgs: ['--enable-automation'],
       colorScheme: 'no-override',
+      acceptDownloads: isUnderTest() ? 'accept' : 'internal-browser-default',
       ...options?.persistentContextOptions,
       args,
     });

packages/playwright-core/src/server/chromium/crBrowser.ts

@@ -343,9 +343,9 @@ export class CRBrowserContext extends BrowserContext {
   override async _initialize() {
     assert(!Array.from(this._browser._crPages.values()).some(page => page._browserContext === this));
     const promises: Promise<any>[] = [super._initialize()];
-    if (this._browser.options.name !== 'electron' && this._browser.options.name !== 'clank') {
+    if (this._browser.options.name !== 'electron' && this._browser.options.name !== 'clank' && this._options.acceptDownloads !== 'internal-browser-default') {
       promises.push(this._browser._session.send('Browser.setDownloadBehavior', {
-        behavior: this._options.acceptDownloads ? 'allowAndName' : 'deny',
+        behavior: this._options.acceptDownloads === 'accept' ? 'allowAndName' : 'deny',
         browserContextId: this._browserContextId,
         downloadPath: this._browser.options.downloadsPath,
         eventsEnabled: true,

packages/playwright-core/src/server/download.ts

@@ -26,7 +26,7 @@ export class Download {
   private _suggestedFilename: string | undefined;
 
   constructor(page: Page, downloadsPath: string, uuid: string, url: string, suggestedFilename?: string) {
-    const unaccessibleErrorMessage = !page._browserContext._options.acceptDownloads ? 'Pass { acceptDownloads: true } when you are creating your browser context.' : undefined;
+    const unaccessibleErrorMessage = page._browserContext._options.acceptDownloads === 'deny' ? 'Pass { acceptDownloads: true } when you are creating your browser context.' : undefined;
     this.artifact = new Artifact(page, path.join(downloadsPath, uuid), unaccessibleErrorMessage, () => {
       return this._page._browserContext.cancelDownload(uuid);
     });

packages/playwright-core/src/server/firefox/ffBrowser.ts

@@ -174,13 +174,15 @@ export class FFBrowserContext extends BrowserContext {
     assert(!this._ffPages().length);
     const browserContextId = this._browserContextId;
     const promises: Promise<any>[] = [super._initialize()];
-    promises.push(this._browser._connection.send('Browser.setDownloadOptions', {
-      browserContextId,
-      downloadOptions: {
-        behavior: this._options.acceptDownloads ? 'saveToDisk' : 'cancel',
-        downloadsDir: this._browser.options.downloadsPath,
-      },
-    }));
+    if (this._options.acceptDownloads !== 'internal-browser-default') {
+      promises.push(this._browser._connection.send('Browser.setDownloadOptions', {
+        browserContextId,
+        downloadOptions: {
+          behavior: this._options.acceptDownloads === 'accept' ? 'saveToDisk' : 'cancel',
+          downloadsDir: this._browser.options.downloadsPath,
+        },
+      }));
+    }
     if (this._options.viewport) {
       const viewport = {
         viewportSize: { width: this._options.viewport.width, height: this._options.viewport.height },

packages/playwright-core/src/server/webkit/wkBrowser.ts

@@ -222,7 +222,7 @@ export class WKBrowserContext extends BrowserContext {
     const browserContextId = this._browserContextId;
     const promises: Promise<any>[] = [super._initialize()];
     promises.push(this._browser._browserSession.send('Playwright.setDownloadBehavior', {
-      behavior: this._options.acceptDownloads ? 'allow' : 'deny',
+      behavior: this._options.acceptDownloads === 'accept' ? 'allow' : 'deny',
       downloadPath: this._browser.options.downloadsPath,
       browserContextId
     }));

packages/protocol/src/channels.ts

@@ -965,7 +965,7 @@ export type BrowserTypeLaunchPersistentContextParams = {
   colorScheme?: 'dark' | 'light' | 'no-preference' | 'no-override',
   reducedMotion?: 'reduce' | 'no-preference' | 'no-override',
   forcedColors?: 'active' | 'none' | 'no-override',
-  acceptDownloads?: boolean,
+  acceptDownloads?: 'accept' | 'deny' | 'internal-browser-default',
   baseURL?: string,
   recordVideo?: {
     dir: string,
@@ -1036,7 +1036,7 @@ export type BrowserTypeLaunchPersistentContextOptions = {
   colorScheme?: 'dark' | 'light' | 'no-preference' | 'no-override',
   reducedMotion?: 'reduce' | 'no-preference' | 'no-override',
   forcedColors?: 'active' | 'none' | 'no-override',
-  acceptDownloads?: boolean,
+  acceptDownloads?: 'accept' | 'deny' | 'internal-browser-default',
   baseURL?: string,
   recordVideo?: {
     dir: string,
@@ -1138,7 +1138,7 @@ export type BrowserNewContextParams = {
   colorScheme?: 'dark' | 'light' | 'no-preference' | 'no-override',
   reducedMotion?: 'reduce' | 'no-preference' | 'no-override',
   forcedColors?: 'active' | 'none' | 'no-override',
-  acceptDownloads?: boolean,
+  acceptDownloads?: 'accept' | 'deny' | 'internal-browser-default',
   baseURL?: string,
   recordVideo?: {
     dir: string,
@@ -1196,7 +1196,7 @@ export type BrowserNewContextOptions = {
   colorScheme?: 'dark' | 'light' | 'no-preference' | 'no-override',
   reducedMotion?: 'reduce' | 'no-preference' | 'no-override',
   forcedColors?: 'active' | 'none' | 'no-override',
-  acceptDownloads?: boolean,
+  acceptDownloads?: 'accept' | 'deny' | 'internal-browser-default',
   baseURL?: string,
   recordVideo?: {
     dir: string,
@@ -1257,7 +1257,7 @@ export type BrowserNewContextForReuseParams = {
   colorScheme?: 'dark' | 'light' | 'no-preference' | 'no-override',
   reducedMotion?: 'reduce' | 'no-preference' | 'no-override',
   forcedColors?: 'active' | 'none' | 'no-override',
-  acceptDownloads?: boolean,
+  acceptDownloads?: 'accept' | 'deny' | 'internal-browser-default',
   baseURL?: string,
   recordVideo?: {
     dir: string,
@@ -1315,7 +1315,7 @@ export type BrowserNewContextForReuseOptions = {
   colorScheme?: 'dark' | 'light' | 'no-preference' | 'no-override',
   reducedMotion?: 'reduce' | 'no-preference' | 'no-override',
   forcedColors?: 'active' | 'none' | 'no-override',
-  acceptDownloads?: boolean,
+  acceptDownloads?: 'accept' | 'deny' | 'internal-browser-default',
   baseURL?: string,
   recordVideo?: {
     dir: string,
@@ -3986,7 +3986,7 @@ export type ElectronLaunchParams = {
   cwd?: string,
   env?: NameValue[],
   timeout?: number,
-  acceptDownloads?: boolean,
+  acceptDownloads?: 'accept' | 'deny' | 'internal-browser-default',
   bypassCSP?: boolean,
   colorScheme?: 'dark' | 'light' | 'no-preference' | 'no-override',
   extraHTTPHeaders?: NameValue[],
@@ -4021,7 +4021,7 @@ export type ElectronLaunchOptions = {
   cwd?: string,
   env?: NameValue[],
   timeout?: number,
-  acceptDownloads?: boolean,
+  acceptDownloads?: 'accept' | 'deny' | 'internal-browser-default',
   bypassCSP?: boolean,
   colorScheme?: 'dark' | 'light' | 'no-preference' | 'no-override',
   extraHTTPHeaders?: NameValue[],
@@ -4412,7 +4412,7 @@ export type AndroidDeviceLaunchBrowserParams = {
   colorScheme?: 'dark' | 'light' | 'no-preference' | 'no-override',
   reducedMotion?: 'reduce' | 'no-preference' | 'no-override',
   forcedColors?: 'active' | 'none' | 'no-override',
-  acceptDownloads?: boolean,
+  acceptDownloads?: 'accept' | 'deny' | 'internal-browser-default',
   baseURL?: string,
   recordVideo?: {
     dir: string,
@@ -4468,7 +4468,7 @@ export type AndroidDeviceLaunchBrowserOptions = {
   colorScheme?: 'dark' | 'light' | 'no-preference' | 'no-override',
   reducedMotion?: 'reduce' | 'no-preference' | 'no-override',
   forcedColors?: 'active' | 'none' | 'no-override',
-  acceptDownloads?: boolean,
+  acceptDownloads?: 'accept' | 'deny' | 'internal-browser-default',
   baseURL?: string,
   recordVideo?: {
     dir: string,

packages/protocol/src/protocol.yml

@@ -474,7 +474,12 @@ ContextOptions:
       - active
       - none
       - no-override
-    acceptDownloads: boolean?
+    acceptDownloads:
+      type: enum?
+      literals:
+      - accept
+      - deny
+      - internal-browser-default
     baseURL: string?
     recordVideo:
       type: object?
@@ -3136,7 +3141,12 @@ Electron:
           type: array?
           items: NameValue
         timeout: number?
-        acceptDownloads: boolean?
+        acceptDownloads:
+          type: enum?
+          literals:
+          - accept
+          - deny
+          - internal-browser-default
         bypassCSP: boolean?
         colorScheme:
           type: enum?

packages/trace-viewer/src/sw.ts

@@ -122,9 +122,7 @@ async function doFetch(event: FetchEvent): Promise<Response> {
         // We will accept explicit ?trace= value as well as the clientId associated with the trace.
         if (traceUrl !== trace && !traceUrls.includes(trace))
           continue;
-        const blob = await traceModel!.resourceForSha1(relativePath.slice('/sha1/'.length));
-        if (blob)
-          return new Response(blob, { status: 200 });
+        return await serveResource(traceModel, relativePath.slice('/sha1/'.length));
       }
       return new Response(null, { status: 404 });
     }
@@ -145,6 +143,24 @@ async function doFetch(event: FetchEvent): Promise<Response> {
   return snapshotServer.serveResource(lookupUrls, request.method, snapshotUrl);
 }
 
+async function serveResource(traceModel: TraceModel, sha1: string): Promise<Response> {
+  const blob = await traceModel!.resourceForSha1(sha1);
+  if (blob)
+    return new Response(blob, { status: 200, headers: headersForResource(traceModel, sha1) });
+  return new Response(null, { status: 404 });
+}
+
+function headersForResource(traceModel: TraceModel, sha1: string): Headers | undefined {
+  const attachment = traceModel.attachmentForSha1(sha1);
+  if (!attachment)
+    return;
+  const headers = new Headers();
+  headers.set('Content-Disposition', `attachment; filename="${attachment.name}"`);
+  if (attachment.contentType)
+    headers.set('Content-Type', attachment.contentType);
+  return headers;
+}
+
 async function gc() {
   const clients = await self.clients.matchAll();
   const usedTraces = new Set<string>();

packages/trace-viewer/src/traceModel.ts

@@ -35,6 +35,7 @@ export class TraceModel {
   private _snapshotStorage: SnapshotStorage | undefined;
   private _version: number | undefined;
   private _backend!: TraceModelBackend;
+  private _attachments = new Map<string, trace.AfterActionTraceEventAttachment>();
 
   constructor() {
   }
@@ -112,6 +113,10 @@ export class TraceModel {
     return this._backend.readBlob('resources/' + sha1);
   }
 
+  attachmentForSha1(sha1: string): trace.AfterActionTraceEventAttachment | undefined {
+    return this._attachments.get(sha1);
+  }
+
   storage(): SnapshotStorage {
     return this._snapshotStorage!;
   }
@@ -169,6 +174,8 @@ export class TraceModel {
         existing!.result = event.result;
         existing!.error = event.error;
         existing!.attachments = event.attachments;
+        for (const attachment of event.attachments?.filter(a => a.sha1) || [])
+          this._attachments.set(attachment.sha1!, attachment);
         break;
       }
       case 'action': {

packages/trace-viewer/src/ui/attachmentsTab.tsx

@@ -56,13 +56,13 @@ export const AttachmentsSection: React.FunctionComponent<{
     {[...screenshots].map((a, i) => {
       return <div className='attachment-item' key={`screenshot-${i}`}>
         <div><img draggable='false' src={attachmentURL(traceUrl, a)} /></div>
-        <div><a target='_blank' href={attachmentURL(traceUrl, a)}>{a.name}</a></div>
+        <div><a href={attachmentURL(traceUrl, a)}>{a.name}</a></div>
       </div>;
     })}
     {otherAttachments.size ? <div className='attachments-section'>Attachments</div> : undefined}
     {[...otherAttachments].map((a, i) => {
       return <div className='attachment-item' key={`attachment-${i}`}>
-        <a target='_blank' href={attachmentURL(traceUrl, a)}>{a.name}</a>
+        <a href={attachmentURL(traceUrl, a)}>{a.name}</a>
       </div>;
     })}
   </>;

packages/trace/src/trace.ts

@@ -73,6 +73,14 @@ export type InputActionTraceEvent = {
   point?: Point;
 };
 
+export type AfterActionTraceEventAttachment = {
+  name: string;
+  contentType: string;
+  path?: string;
+  sha1?: string;
+  base64?: string;
+};
+
 export type AfterActionTraceEvent = {
   type: 'after',
   callId: string;
@@ -80,13 +88,7 @@ export type AfterActionTraceEvent = {
   afterSnapshot?: string;
   log: string[];
   error?: SerializedError['error'];
-  attachments?: {
-    name: string;
-    contentType: string;
-    path?: string;
-    sha1?: string;
-    base64?: string;
-  }[];
+  attachments?: AfterActionTraceEventAttachment[];
   result?: any;
 };
 

tests/playwright-test/ui-mode-test-attachments.spec.ts

@@ -14,6 +14,7 @@
  * limitations under the License.
  */
 
+import fs from 'fs';
 import { test, expect, retries } from './ui-mode-fixtures';
 
 test.describe.configure({ mode: 'parallel', retries });
@@ -32,12 +33,11 @@ test('should contain file attachment', async ({ runUITest }) => {
   await expect(page.getByTestId('status-line')).toHaveText('1/1 passed (100%)');
   await page.getByText('Attachments').click();
   await page.getByText('attach "note"', { exact: true }).click();
-  const popupPromise = page.waitForEvent('popup');
+  const downloadPromise = page.waitForEvent('download');
   await page.getByRole('link', { name: 'note' }).click();
-  const popup = await popupPromise;
-  await popup.waitForLoadState();
-  const content = await popup.content();
-  expect(content).toContain('attach test');
+  const download = await downloadPromise;
+  expect(download.suggestedFilename()).toBe('note');
+  expect((await readAllFromStream(await download.createReadStream())).toString()).toContain('attach test');
 });
 
 test('should contain string attachment', async ({ runUITest }) => {
@@ -54,10 +54,38 @@ test('should contain string attachment', async ({ runUITest }) => {
   await expect(page.getByTestId('status-line')).toHaveText('1/1 passed (100%)');
   await page.getByText('Attachments').click();
   await page.getByText('attach "note"', { exact: true }).click();
-  const popupPromise = page.waitForEvent('popup');
+  const downloadPromise = page.waitForEvent('download');
   await page.getByRole('link', { name: 'note' }).click();
-  const popup = await popupPromise;
-  await popup.waitForLoadState();
-  const content = await popup.content();
-  expect(content).toContain('text42');
+  const download = await downloadPromise;
+  expect(download.suggestedFilename()).toBe('note');
+  expect((await readAllFromStream(await download.createReadStream())).toString()).toEqual('text42');
 });
+
+test('should contain attachment with filename and extension', async ({ runUITest, asset }) => {
+  const { page } = await runUITest({
+    'a.test.ts': `
+      import { test } from '@playwright/test';
+      test('attach test', async () => {
+        await test.info().attach('screenshot.png', { path: ${JSON.stringify(asset('pptr.png'))} });
+      });
+    `,
+  });
+  await page.getByText('attach test').click();
+  await page.getByTitle('Run all').click();
+  await expect(page.getByTestId('status-line')).toHaveText('1/1 passed (100%)');
+  await page.getByText('Attachments').click();
+  await page.getByText('attach "screenshot.png"', { exact: true }).click();
+  const downloadPromise = page.waitForEvent('download');
+  await page.getByRole('link', { name: 'screenshot.png' }).click();
+  const download = await downloadPromise;
+  expect(download.suggestedFilename()).toBe('screenshot.png');
+  expect(await readAllFromStream(await download.createReadStream())).toEqual(fs.readFileSync(asset('pptr.png')));
+});
+
+function readAllFromStream(stream: NodeJS.ReadableStream): Promise<Buffer> {
+  return new Promise(resolve => {
+    const chunks: Buffer[] = [];
+    stream.on('data', chunk => chunks.push(chunk));
+    stream.on('end', () => resolve(Buffer.concat(chunks)));
+  });
+}