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

Author: Skn0tt

packages/playwright-core/src/utils/isomorphic/protocolFormatter.ts

@@ -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();
+    }
+
+    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;
   });
 }

packages/playwright-core/src/utils/isomorphic/protocolMetainfo.ts

@@ -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', }],

packages/protocol/src/protocol.yml

@@ -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?
@@ -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?

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

@@ -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;
   }
 

tests/library/trace-viewer.spec.ts

@@ -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');
@@ -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/,
   ]);
@@ -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/,
   ]);