Skip to content

Commit

Permalink
fix: download of attachments in UI Mode (#26407)
Browse files Browse the repository at this point in the history
Fixes #26326.
  • Loading branch information
mxschmitt committed Aug 17, 2023
1 parent 0e6deb7 commit 75ed251
Show file tree
Hide file tree
Showing 17 changed files with 132 additions and 55 deletions.
9 changes: 9 additions & 0 deletions packages/playwright-core/src/client/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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';
}
3 changes: 2 additions & 1 deletion packages/playwright-core/src/client/electron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
3 changes: 2 additions & 1 deletion packages/playwright-core/src/client/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 = {
Expand Down
10 changes: 5 additions & 5 deletions packages/playwright-core/src/protocol/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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'))),
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/server/browserContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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',
Expand Down
3 changes: 2 additions & 1 deletion packages/playwright-core/src/server/browserType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
});
Expand Down
4 changes: 2 additions & 2 deletions packages/playwright-core/src/server/chromium/crBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/download.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
16 changes: 9 additions & 7 deletions packages/playwright-core/src/server/firefox/ffBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
Expand Down
2 changes: 1 addition & 1 deletion packages/playwright-core/src/server/webkit/wkBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}));
Expand Down
20 changes: 10 additions & 10 deletions packages/protocol/src/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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[],
Expand Down Expand Up @@ -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[],
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 12 additions & 2 deletions packages/protocol/src/protocol.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down Expand Up @@ -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?
Expand Down
22 changes: 19 additions & 3 deletions packages/trace-viewer/src/sw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
Expand All @@ -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>();
Expand Down
7 changes: 7 additions & 0 deletions packages/trace-viewer/src/traceModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
}
Expand Down Expand Up @@ -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!;
}
Expand Down Expand Up @@ -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': {
Expand Down
4 changes: 2 additions & 2 deletions packages/trace-viewer/src/ui/attachmentsTab.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
})}
</>;
Expand Down
Loading

0 comments on commit 75ed251

Please sign in to comment.