chore: miscellaneous trace viewer fixes (#23695)

- properly annotate continued requests - nest `attach` steps inside the related `expect` step - fix primary-id-to-non-primary-id mapping - make sure images in trace are not draggable Fixes #23693 --------- Signed-off-by: Andrey Lushnikov <aslushnikov@gmail.com> Co-authored-by: Max Schmitt <max@schmitt.mx>
microsoft · Jun 14, 2023 · 3c0fab4 · 3c0fab4
1 parent 9e63668
commit 3c0fab4
Show file tree

Hide file tree

Showing 11 changed files with 16 additions and 13 deletions.
diff --git a/packages/playwright-core/src/server/chromium/crServiceWorker.ts b/packages/playwright-core/src/server/chromium/crServiceWorker.ts
@@ -114,7 +114,7 @@ export class CRServiceWorker extends Worker {
       const r = new network.Route(request, route);
       if (this._browserContext._requestInterceptor?.(r, request))
         return;
-      r.continue();
+      r.continue({ isFallback: true });
     }
   }
 

diff --git a/packages/playwright-core/src/server/frames.ts b/packages/playwright-core/src/server/frames.ts
@@ -294,7 +294,7 @@ export class FrameManager {
       frame.setPendingDocument({ documentId: request._documentId, request });
     if (request._isFavicon) {
       if (route)
-        route.continue(request, {});
+        route.continue(request, { isFallback: true });
       return;
     }
     this._page.emitOnContext(BrowserContext.Events.Request, request);
@@ -306,7 +306,7 @@ export class FrameManager {
         return;
       if (this._page._browserContext._requestInterceptor?.(r, request))
         return;
-      r.continue();
+      r.continue({ isFallback: true });
     }
   }
 

diff --git a/packages/playwright-core/src/server/network.ts b/packages/playwright-core/src/server/network.ts
@@ -313,7 +313,7 @@ export class Route extends SdkObject {
     headers.push({ name: 'vary', value: 'Origin' });
   }
 
-  async continue(overrides: types.NormalizedContinueOverrides = {}) {
+  async continue(overrides: types.NormalizedContinueOverrides) {
     this._startHandling();
     if (overrides.url) {
       const newUrl = new URL(overrides.url);

diff --git a/packages/playwright-core/src/server/types.ts b/packages/playwright-core/src/server/types.ts
@@ -146,7 +146,7 @@ export type NormalizedContinueOverrides = {
   method?: string,
   headers?: HeadersArray,
   postData?: Buffer,
-  isFallback?: boolean,
+  isFallback: boolean,
 };
 
 export type EmulatedSize = { viewport: Size, screen: Size };

diff --git a/packages/playwright-test/src/worker/testInfo.ts b/packages/playwright-test/src/worker/testInfo.ts
@@ -238,7 +238,8 @@ export class TestInfoImpl implements TestInfo {
       const visit = (step: TestStepInternal) => {
         // Never nest into under another lax element, it could be a series
         // of no-reply actions, ala page.continue().
-        if (!step.endWallTime && step.category === data.category && !step.laxParent)
+        const canNest = step.category === data.category || step.category === 'expect' && data.category === 'attach';
+        if (!step.endWallTime && canNest && !step.laxParent)
           parentStep = step;
         step.steps.forEach(visit);
       };
@@ -352,6 +353,7 @@ export class TestInfoImpl implements TestInfo {
       title: `attach "${name}"`,
       category: 'attach',
       wallTime: Date.now(),
+      laxParent: true,
     });
     this._attachWithoutStep(attachment);
     step.complete({});

diff --git a/packages/trace-viewer/src/ui/attachmentsTab.tsx b/packages/trace-viewer/src/ui/attachmentsTab.tsx
@@ -45,7 +45,7 @@ export const AttachmentsTab: React.FunctionComponent<{
     {screenshots.size ? <div className='attachments-section'>Screenshots</div> : undefined}
     {[...screenshots].map((a, i) => {
       return <div className='attachment-item' key={`screenshot-${i}`}>
-        <div><img src={attachmentURL(traceUrl, a)} /></div>
+        <div><img draggable='false' src={attachmentURL(traceUrl, a)} /></div>
         <div><a target='_blank' href={attachmentURL(traceUrl, a)}>{a.name}</a></div>
       </div>;
     })}

diff --git a/packages/trace-viewer/src/ui/modelUtil.ts b/packages/trace-viewer/src/ui/modelUtil.ts
@@ -138,6 +138,8 @@ function mergeActions(contexts: ContextEntry[]) {
           existing.parentId = nonPrimaryIdToPrimaryId.get(action.parentId) ?? action.parentId;
         continue;
       }
+      if (action.parentId)
+        action.parentId = nonPrimaryIdToPrimaryId.get(action.parentId) ?? action.parentId;
       map.set(key, { ...action, context });
     }
   }

diff --git a/packages/trace-viewer/src/ui/networkResourceDetails.tsx b/packages/trace-viewer/src/ui/networkResourceDetails.tsx
@@ -103,7 +103,7 @@ export const NetworkResourceDetails: React.FunctionComponent<{
         {resource.request.postData ? <div className='network-request-body'>{formatBody(requestBody, requestContentType)}</div> : ''}
         <div className='network-request-details-header'>Response Body</div>
         {!resource.response.content._sha1 ? <div className='network-request-response-body'>Response body is not available for this request.</div> : ''}
-        {responseBody !== null && responseBody.dataUrl ? <img src={responseBody.dataUrl} /> : ''}
+        {responseBody !== null && responseBody.dataUrl ? <img draggable='false' src={responseBody.dataUrl} /> : ''}
         {responseBody !== null && responseBody.text ? <div className='network-request-response-body'>{formatBody(responseBody.text, resource.response.content.mimeType)}</div> : ''}
       </div>
     </Expandable>

diff --git a/packages/web/src/components/imageDiffView.tsx b/packages/web/src/components/imageDiffView.tsx
@@ -161,7 +161,7 @@ const ImageWithSize: React.FunctionComponent<{
       <span style={{ flex: 'none', margin: '0 5px' }}>x</span>
       <span style={{ flex: '1 1 0', textAlign: 'start' }}>{ size ? size.height : ''}</span>
     </div>
-    <img src={src} onLoad={() => {
+    <img draggable='false' src={src} onLoad={() => {
       onLoad?.();
       if (ref.current)
         setSize({ width: ref.current.naturalWidth, height: ref.current.naturalHeight });

diff --git a/tests/library/trace-viewer.spec.ts b/tests/library/trace-viewer.spec.ts
@@ -230,7 +230,8 @@ test('should have network request overrides', async ({ page, server, runAndTrace
   await traceViewer.selectAction('http://localhost');
   await traceViewer.showNetworkTab();
   await expect(traceViewer.networkRequests).toContainText([/200GET\/frame.htmltext\/html/]);
-  await expect(traceViewer.networkRequests).toContainText([/abort.*style.cssx-unknown/]);
+  await expect(traceViewer.networkRequests).toContainText([/aborted.*style.cssx-unknown/]);
+  await expect(traceViewer.networkRequests).not.toContainText([/continued/]);
 });
 
 test('should have network request overrides 2', async ({ page, server, runAndTrace }) => {
@@ -241,7 +242,7 @@ test('should have network request overrides 2', async ({ page, server, runAndTra
   await traceViewer.selectAction('http://localhost');
   await traceViewer.showNetworkTab();
   await expect(traceViewer.networkRequests).toContainText([/200GET\/frame.htmltext\/html/]);
-  await expect(traceViewer.networkRequests).toContainText([/continue.*script.jsapplication\/javascript/]);
+  await expect(traceViewer.networkRequests).toContainText([/continued.*script.jsapplication\/javascript/]);
 });
 
 test('should show snapshot URL', async ({ page, runAndTrace, server }) => {

diff --git a/tests/playwright-test/ui-mode-trace.spec.ts b/tests/playwright-test/ui-mode-trace.spec.ts
@@ -91,15 +91,13 @@ test('should merge screenshot assertions', async ({  runUITest }, testInfo) => {
   await page.getByText('trace test').dblclick();
 
   const listItem = page.getByTestId('action-list').getByRole('listitem');
-  // TODO: fixme.
   await expect(
       listItem,
       'action list'
   ).toHaveText([
     /Before Hooks[\d.]+m?s/,
     /page.setContent[\d.]+m?s/,
     /expect.toHaveScreenshot[\d.]+m?s/,
-    /attach "trace-test-1-actual\.png"[\d.]+m?s/,
     /After Hooks[\d.]+m?s/,
     /fixture: page[\d.]+m?s/,
     /fixture: context[\d.]+m?s/,