Skip to content

fix(trace): don't replace unknown braces in action title #36920

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

Merged
merged 7 commits into from
Aug 6, 2025
Merged

fix(trace): don't replace unknown braces in action title #36920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9a9c7b7
fix(trace): don't replace unknown braces in action title
Skn0tt Aug 5, 2025
4f3de66
address
Skn0tt Aug 5, 2025
76c2884
replace "optional" case with special case
Skn0tt Aug 5, 2025
db733fc
Merge branch 'main' into action-list-only-template-known-params
Skn0tt Aug 5, 2025
0fa6f55
amend
Skn0tt Aug 5, 2025
7b97492
revert
Skn0tt Aug 5, 2025
6ecff00
adress feedback
Skn0tt Aug 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 29 additions & 22 deletions packages/playwright-core/src/utils/isomorphic/protocolFormatter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,44 +16,51 @@

import { methodMetainfo } from './protocolMetainfo';

export function formatProtocolParam(params: Record<string, string> | undefined, name: string): string {
export function formatProtocolParam(params: Record<string, string> | undefined, alternatives: string): string | undefined {
if (!params)
return '';
if (name === 'url') {
try {
const urlObject = new URL(params[name]);
if (urlObject.protocol === 'data:')
return urlObject.protocol;
if (urlObject.protocol === 'about:')
return params[name];
return urlObject.pathname + urlObject.search;
} catch (error) {
return params[name];
return undefined;

for (const name of alternatives.split('|')) {
if (name === 'url') {
try {
const urlObject = new URL(params[name]);
if (urlObject.protocol === 'data:')
return urlObject.protocol;
if (urlObject.protocol === 'about:')
return params[name];
return urlObject.pathname + urlObject.search;
} catch (error) {
if (params[name] !== undefined)
return params[name];
}
}
if (name === 'timeNumber' && params[name] !== undefined) {
// eslint-disable-next-line no-restricted-globals
return new Date(params[name]).toString();
Copy link
Contributor

@dgozman dgozman Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return new Date(params[name]).toString();
if (params[name] !== undefined)
return new Date(params[name]).toString();

}

const value = deepParam(params, name);
if (value !== undefined)
return value;
}
if (name === 'timeNumber') {
// eslint-disable-next-line no-restricted-globals
return new Date(params[name]).toString();
}
return deepParam(params, name);
}

function deepParam(params: Record<string, any>, name: string): string {
function deepParam(params: Record<string, any>, name: string): string | undefined {
const tokens = name.split('.');
let current = params;
for (const token of tokens) {
if (typeof current !== 'object' || current === null)
return '';
return undefined;
current = current[token];
}
if (current === undefined)
return '';
return undefined;
return String(current);
}

export function renderTitleForCall(metadata: { title?: string, type: string, method: string, params: Record<string, string> | undefined }) {
const titleFormat = metadata.title ?? methodMetainfo.get(metadata.type + '.' + metadata.method)?.title ?? metadata.method;
return titleFormat.replace(/\{([^}]+)\}/g, (_, p1) => {
return formatProtocolParam(metadata.params, p1);
return titleFormat.replace(/\{([^}]+)\}/g, (fullMatch, p1) => {
return formatProtocolParam(metadata.params, p1) ?? fullMatch;
});
}
12 changes: 6 additions & 6 deletions packages/playwright-core/src/utils/isomorphic/protocolMetainfo.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,13 @@ export const methodMetainfo = new Map<string, { internal?: boolean, title?: stri
['BrowserContext.harExport', { internal: true, }],
['BrowserContext.createTempFiles', { internal: true, }],
['BrowserContext.updateSubscription', { internal: true, }],
['BrowserContext.clockFastForward', { title: 'Fast forward clock "{ticksNumber}{ticksString}"', }],
['BrowserContext.clockInstall', { title: 'Install clock "{timeNumber}{timeString}"', }],
['BrowserContext.clockPauseAt', { title: 'Pause clock "{timeNumber}{timeString}"', }],
['BrowserContext.clockFastForward', { title: 'Fast forward clock "{ticksNumber|ticksString}"', }],
['BrowserContext.clockInstall', { title: 'Install clock "{timeNumber|timeString}"', }],
['BrowserContext.clockPauseAt', { title: 'Pause clock "{timeNumber|timeString}"', }],
['BrowserContext.clockResume', { title: 'Resume clock', }],
['BrowserContext.clockRunFor', { title: 'Run clock "{ticksNumber}{ticksString}"', }],
['BrowserContext.clockSetFixedTime', { title: 'Set fixed time "{timeNumber}{timeString}"', }],
['BrowserContext.clockSetSystemTime', { title: 'Set system time "{timeNumber}{timeString}"', }],
['BrowserContext.clockRunFor', { title: 'Run clock "{ticksNumber|ticksString}"', }],
['BrowserContext.clockSetFixedTime', { title: 'Set fixed time "{timeNumber|timeString}"', }],
['BrowserContext.clockSetSystemTime', { title: 'Set system time "{timeNumber|timeString}"', }],
['BrowserContext.clockUninstall', { title: 'Uninstall clock', }],
['Page.addInitScript', { }],
['Page.close', { title: 'Close page', }],
Expand Down
12 changes: 6 additions & 6 deletions packages/protocol/src/protocol.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1425,19 +1425,19 @@ BrowserContext:
enabled: boolean

clockFastForward:
title: Fast forward clock "{ticksNumber}{ticksString}"
title: Fast forward clock "{ticksNumber|ticksString}"
parameters:
ticksNumber: float?
ticksString: string?

clockInstall:
title: Install clock "{timeNumber}{timeString}"
title: Install clock "{timeNumber|timeString}"
parameters:
timeNumber: float?
timeString: string?

clockPauseAt:
title: Pause clock "{timeNumber}{timeString}"
title: Pause clock "{timeNumber|timeString}"
parameters:
timeNumber: float?
timeString: string?
Expand All @@ -1446,19 +1446,19 @@ BrowserContext:
title: Resume clock

clockRunFor:
title: Run clock "{ticksNumber}{ticksString}"
title: Run clock "{ticksNumber|ticksString}"
parameters:
ticksNumber: float?
ticksString: string?

clockSetFixedTime:
title: Set fixed time "{timeNumber}{timeString}"
title: Set fixed time "{timeNumber|timeString}"
parameters:
timeNumber: float?
timeString: string?

clockSetSystemTime:
title: Set system time "{timeNumber}{timeString}"
title: Set system time "{timeNumber|timeString}"
parameters:
timeNumber: float?
timeString: string?
Expand Down
11 changes: 8 additions & 3 deletions packages/trace-viewer/src/ui/actionList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,16 @@ export function renderTitleForCall(action: ActionTraceEvent): { elements: React.
title.push(chunk);

const param = formatProtocolParam(action.params, quotedText);
if (match.index === 0)
if (param === undefined) {
elements.push(fullMatch);
title.push(fullMatch);
} else if (match.index === 0) {
elements.push(param);
else
title.push(param);
} else {
elements.push(<span className='action-title-param'>{param}</span>);
title.push(param);
title.push(param);
}
currentIndex = match.index + fullMatch.length;
}

Expand Down
8 changes: 5 additions & 3 deletions tests/library/trace-viewer.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,12 +110,14 @@ test('should open trace viewer on specific host', async ({ showTraceViewer }, te
});

test('should show tracing.group in the action list with location', async ({ runAndTrace, page, context }) => {
test.info().annotations.push({ type: 'issue', description: 'https://github.com/microsoft/playwright/issues/36483' });

const traceViewer = await test.step('create trace with groups', async () => {
await page.context().tracing.group('ignored group');
return await runAndTrace(async () => {
await context.tracing.group('outer group');
await page.goto(`data:text/html,<!DOCTYPE html><body><div>Hello world</div></body>`);
await context.tracing.group('inner group 1', { location: { file: __filename, line: 17, column: 1 } });
await context.tracing.group('inner group 1 {{ eager_beaver }}', { location: { file: __filename, line: 17, column: 1 } });
await page.locator('body').click();
await context.tracing.groupEnd();
await context.tracing.group('inner group 2');
Expand All @@ -128,7 +130,7 @@ test('should show tracing.group in the action list with location', async ({ runA
await expect(traceViewer.actionTitles).toHaveText([
/outer group/,
/Navigate/,
/inner group 1/,
/inner group 1 {{ eager_beaver }}/,
/inner group 2/,
/toBeVisible/,
]);
Expand All @@ -138,7 +140,7 @@ test('should show tracing.group in the action list with location', async ({ runA
await expect(traceViewer.actionTitles).toHaveText([
/outer group/,
/Navigate/,
/inner group 1/,
/inner group 1 {{ eager_beaver }}/,
/Click.*locator/,
/inner group 2/,
]);
Expand Down
Loading