From 48317af1cc938955c357f9e33d921183f2e6dcae Mon Sep 17 00:00:00 2001
From: Dmitry Gozman <dgozman@gmail.com>
Date: Fri, 12 Jan 2024 12:11:39 -0800
Subject: [PATCH] feat(trace): preserve noscript when javascript is disabled
 (#28971)

Closes #27504, closes #27532.
---
 .../src/server/trace/recorder/snapshotter.ts  |  2 +-
 .../trace/recorder/snapshotterInjected.ts     |  5 ++---
 packages/trace-viewer/src/snapshotRenderer.ts | 21 +++++++++++--------
 tests/library/trace-viewer.spec.ts            | 18 ++++++++++++++++
 4 files changed, 33 insertions(+), 13 deletions(-)

diff --git a/packages/playwright-core/src/server/trace/recorder/snapshotter.ts b/packages/playwright-core/src/server/trace/recorder/snapshotter.ts
index 2340f55a26..30bb86c8bf 100644
--- a/packages/playwright-core/src/server/trace/recorder/snapshotter.ts
+++ b/packages/playwright-core/src/server/trace/recorder/snapshotter.ts
@@ -86,7 +86,7 @@ export class Snapshotter {
       eventsHelper.addEventListener(this._context, BrowserContext.Events.Page, this._onPage.bind(this)),
     ];
 
-    const initScript = `(${frameSnapshotStreamer})("${this._snapshotStreamer}")`;
+    const initScript = `(${frameSnapshotStreamer})("${this._snapshotStreamer}", ${!!this._context._options.javaScriptEnabled})`;
     await this._context.addInitScript(initScript);
     await this._runInAllFrames(initScript);
   }
diff --git a/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts b/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts
index 7a2dfd4462..22ece79459 100644
--- a/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts
+++ b/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts
@@ -31,7 +31,7 @@ export type SnapshotData = {
   collectionTime: number,
 };
 
-export function frameSnapshotStreamer(snapshotStreamer: string) {
+export function frameSnapshotStreamer(snapshotStreamer: string, removeNoScript: boolean) {
   // Communication with Playwright.
   if ((window as any)[snapshotStreamer])
     return;
@@ -81,7 +81,6 @@ export function frameSnapshotStreamer(snapshotStreamer: string) {
   }
 
   class Streamer {
-    private _removeNoScript = true;
     private _lastSnapshotNumber = 0;
     private _staleStyleSheets = new Set<CSSStyleSheet>();
     private _readingStyleSheet = false;  // To avoid invalidating due to our own reads.
@@ -337,7 +336,7 @@ export function frameSnapshotStreamer(snapshotStreamer: string) {
           if (rel === 'preload' || rel === 'prefetch')
             return;
         }
-        if (this._removeNoScript && nodeName === 'NOSCRIPT')
+        if (removeNoScript && nodeName === 'NOSCRIPT')
           return;
         if (nodeName === 'META' && (node as HTMLMetaElement).httpEquiv.toLowerCase() === 'content-security-policy')
           return;
diff --git a/packages/trace-viewer/src/snapshotRenderer.ts b/packages/trace-viewer/src/snapshotRenderer.ts
index 81e62126c5..9642c7c1ad 100644
--- a/packages/trace-viewer/src/snapshotRenderer.ts
+++ b/packages/trace-viewer/src/snapshotRenderer.ts
@@ -65,15 +65,18 @@ export class SnapshotRenderer {
           }
         } else if (typeof n[0] === 'string') {
           // Element node.
-          const builder: string[] = [];
-          builder.push('<', n[0]);
+          // Note that <noscript> will not be rendered by default in the trace viewer, because
+          // JS is enabled. So rename it to <x-noscript>.
+          const nodeName = n[0] === 'NOSCRIPT' ? 'X-NOSCRIPT' : n[0];
           const attrs = Object.entries(n[1] || {});
+          const builder: string[] = [];
+          builder.push('<', nodeName);
           const kCurrentSrcAttribute = '__playwright_current_src__';
-          const isFrame = n[0] === 'IFRAME' || n[0] === 'FRAME';
-          const isAnchor = n[0] === 'A';
-          const isImg = n[0] === 'IMG';
+          const isFrame = nodeName === 'IFRAME' || nodeName === 'FRAME';
+          const isAnchor = nodeName === 'A';
+          const isImg = nodeName === 'IMG';
           const isImgWithCurrentSrc = isImg && attrs.some(a => a[0] === kCurrentSrcAttribute);
-          const isSourceInsidePictureWithCurrentSrc = n[0] === 'SOURCE' && parentTag === 'PICTURE' && parentAttrs?.some(a => a[0] === kCurrentSrcAttribute);
+          const isSourceInsidePictureWithCurrentSrc = nodeName === 'SOURCE' && parentTag === 'PICTURE' && parentAttrs?.some(a => a[0] === kCurrentSrcAttribute);
           for (const [attr, value] of attrs) {
             let attrName = attr;
             if (isFrame && attr.toLowerCase() === 'src') {
@@ -99,9 +102,9 @@ export class SnapshotRenderer {
           }
           builder.push('>');
           for (let i = 2; i < n.length; i++)
-            builder.push(visit(n[i], snapshotIndex, n[0], attrs));
-          if (!autoClosing.has(n[0]))
-            builder.push('</', n[0], '>');
+            builder.push(visit(n[i], snapshotIndex, nodeName, attrs));
+          if (!autoClosing.has(nodeName))
+            builder.push('</', nodeName, '>');
           (n as any)._string = builder.join('');
         } else {
           // Why are we here? Let's not throw, just in case.
diff --git a/tests/library/trace-viewer.spec.ts b/tests/library/trace-viewer.spec.ts
index 6bfd299bee..2b506c4169 100644
--- a/tests/library/trace-viewer.spec.ts
+++ b/tests/library/trace-viewer.spec.ts
@@ -1121,3 +1121,21 @@ test('should highlight locator in iframe while typing', async ({ page, runAndTra
     expect(Math.abs(elementBox.y - highlightBox.y)).toBeLessThan(5);
   }
 });
+
+test('should preserve noscript when javascript is disabled', async ({ browser, server, showTraceViewer }) => {
+  const traceFile = test.info().outputPath('trace.zip');
+  const page = await browser.newPage({ javaScriptEnabled: false });
+  await page.context().tracing.start({ snapshots: true, screenshots: true, sources: true });
+  await page.goto(server.EMPTY_PAGE);
+  await page.setContent(`
+    <body>
+      <noscript>javascript is disabled!</noscript>
+    </body>
+  `);
+  await page.context().tracing.stop({ path: traceFile });
+  await page.close();
+
+  const traceViewer = await showTraceViewer([traceFile]);
+  const frame = await traceViewer.snapshotFrame('page.setContent');
+  await expect(frame.getByText('javascript is disabled!')).toBeVisible();
+});