inlineImages: Setting of image.crossOrigin is not always necessary (#1468)

Setting of the `crossorigin` attribute is not necessary for same-origin images, and causes an immediate image reload (albeit from cache) necessitating the use of a load event listener which subsequently mutates the snapshot. This change allows us to avoid the mutation of the snapshot for the same-origin case. * Modify inlineImages test to remove delay and show that we can inline images without mutation * Add an explicit test for when the `image.crossOrigin = 'anonymous';` method is necessary. Uses a combination of about:blank and our test server to simulate a cross-origin context * Other test changes: there were some spurious rrweb mutations being generated by the addition of the crossorigin attribute that are now elimnated from the rrweb/__snapshots__/integration.test.ts.snap after this PR - this is good
2024-05-23 11:08:49 +01:00
parent 81c54abab2
commit 4014305944
8 changed files with 95 additions and 117 deletions
--- a/.changeset/inlineImage-maybeNot-crossOrigin.md
+++ b/.changeset/inlineImage-maybeNot-crossOrigin.md
@@ -0,0 +1,6 @@
+---
+"rrweb": patch
+"rrweb-snapshot": patch
+---
+
+inlineImages: during snapshot avoid adding an event listener for inlining of same-origin images (async listener mutates the snapshot which can be problematic)
--- a/packages/rrweb-snapshot/src/snapshot.ts
+++ b/packages/rrweb-snapshot/src/snapshot.ts
@@ -747,8 +747,9 @@ function serializeElementNode(
      canvasCtx = canvasService.getContext('2d');
    }
    const image = n as HTMLImageElement;
-    const oldValue = image.crossOrigin;
-    image.crossOrigin = 'anonymous';
+    const imageSrc: string =
+      image.currentSrc || image.getAttribute('src') || '<unknown-src>';
+    const priorCrossOrigin = image.crossOrigin;
    const recordInlineImage = () => {
      image.removeEventListener('load', recordInlineImage);
      try {
@@ -760,13 +761,23 @@ function serializeElementNode(
          dataURLOptions.quality,
        );
      } catch (err) {
-        console.warn(
-          `Cannot inline img src=${image.currentSrc}! Error: ${err as string}`,
-        );
+        if (image.crossOrigin !== 'anonymous') {
+          image.crossOrigin = 'anonymous';
+          if (image.complete && image.naturalWidth !== 0)
+            recordInlineImage(); // too early due to image reload
+          else image.addEventListener('load', recordInlineImage);
+          return;
+        } else {
+          console.warn(
+            `Cannot inline img src=${imageSrc}! Error: ${err as string}`,
+          );
+        }
+      }
+      if (image.crossOrigin === 'anonymous') {
+        priorCrossOrigin
+          ? (attributes.crossOrigin = priorCrossOrigin)
+          : image.removeAttribute('crossorigin');
      }
-      oldValue
-        ? (attributes.crossOrigin = oldValue)
-        : image.removeAttribute('crossorigin');
    };
    // The image content may not have finished loading yet.
    if (image.complete && image.naturalWidth !== 0) recordInlineImage();
--- a/packages/rrweb-snapshot/test/snapshots/integration.test.ts.snap
+++ b/packages/rrweb-snapshot/test/snapshots/integration.test.ts.snap
@@ -338,6 +338,7 @@ exports[`integration tests [html file]: mask-text.html 1`] = `
 exports[`integration tests [html file]: picture.html 1`] = `
 "<html xmlns=\\"http://www.w3.org/1999/xhtml\\"><head></head><body>
    <picture>
+      <!-- these are 404 - not sure if that's intentional -->
      <source type=\\"image/webp\\" srcset=\\"http://localhost:3030/assets/img/characters/robot.webp\\" />
      <img src=\\"http://localhost:3030/assets/img/characters/robot.png\\" />
    </picture>
--- a/packages/rrweb-snapshot/test/html/picture.html
+++ b/packages/rrweb-snapshot/test/html/picture.html
@@ -1,6 +1,7 @@
 <html xmlns="http://www.w3.org/1999/xhtml">
  <body>
    <picture>
+      <!-- these are 404 - not sure if that's intentional -->
      <source type="image/webp" srcset="assets/img/characters/robot.webp" />
      <img src="assets/img/characters/robot.png" />
    </picture>
--- a/packages/rrweb-snapshot/test/images/rrweb-favicon-20x20.png
+++ b/packages/rrweb-snapshot/test/images/rrweb-favicon-20x20.png
--- a/packages/rrweb-snapshot/test/integration.test.ts
+++ b/packages/rrweb-snapshot/test/integration.test.ts
@@ -6,7 +6,7 @@ import * as puppeteer from 'puppeteer';
 import * as rollup from 'rollup';
 import * as typescript from 'rollup-plugin-typescript2';
 import * as assert from 'assert';
-import { waitForRAF } from './utils';
+import { waitForRAF, getServerURL } from './utils';

 const _typescript = typescript as unknown as () => rollup.Plugin;

@@ -209,12 +209,63 @@ iframe.contentDocument.querySelector('center').clientHeight
        inlineImages: true,
        inlineStylesheet: false
    })`);
-    await waitForRAF(page);
-    const snapshot = (await page.evaluate(
-      'JSON.stringify(snapshot, null, 2);',
-    )) as string;
-    assert(snapshot.includes('"rr_dataURL"'));
-    assert(snapshot.includes('data:image/webp;base64,'));
+    // don't wait, as we want to ensure that the same-origin image can be inlined immediately
+    const bodyChildren = (await page.evaluate(`
+      snapshot.childNodes[0].childNodes[1].childNodes.filter((cn) => cn.type === 2);
+`)) as any[];
+    expect(bodyChildren[1]).toEqual(
+      expect.objectContaining({
+        tagName: 'img',
+        attributes: {
+          src: expect.stringMatching(/images\/robot.png$/),
+          alt: 'This is a robot',
+          rr_dataURL: expect.stringMatching(/^data:image\/webp;base64,/),
+        },
+      }),
+    );
+  });
+
+  it('correctly saves cross-origin images offline', async () => {
+    const page: puppeteer.Page = await browser.newPage();
+
+    await page.goto('about:blank', {
+      waitUntil: 'load',
+    });
+    await page.setContent(
+      `
+<html xmlns="http://www.w3.org/1999/xhtml">
+  <body>
+    <img src="${getServerURL(
+      server,
+    )}/images/rrweb-favicon-20x20.png" alt="CORS restricted but has access-control-allow-origin: *" />
+  </body>
+</html>
+`,
+      {
+        waitUntil: 'load',
+      },
+    );
+
+    await page.waitForSelector('img', { timeout: 1000 });
+    await page.evaluate(`${code}var snapshot = rrweb.snapshot(document, {
+        dataURLOptions: { type: "image/webp", quality: 0.8 },
+        inlineImages: true,
+        inlineStylesheet: false
+    })`);
+    await waitForRAF(page); // need a small wait, as after the crossOrigin="anonymous" change, the snapshot triggers a reload of the image (after which, the snapshot is mutated)
+    const bodyChildren = (await page.evaluate(`
+      snapshot.childNodes[0].childNodes[1].childNodes.filter((cn) => cn.type === 2);
+`)) as any[];
+    expect(bodyChildren[0]).toEqual(
+      expect.objectContaining({
+        tagName: 'img',
+        attributes: {
+          src: getServerURL(server) + '/images/rrweb-favicon-20x20.png',
+          alt: 'CORS restricted but has access-control-allow-origin: *',
+          rr_dataURL: expect.stringMatching(/^data:image\/webp;base64,/),
+        },
+      }),
+    );
  });

  it('correctly saves blob:images offline', async () => {
--- a/packages/rrweb-snapshot/test/utils.ts
+++ b/packages/rrweb-snapshot/test/utils.ts
@@ -1,4 +1,5 @@
 import * as puppeteer from 'puppeteer';
+import * as http from 'http';

 export async function waitForRAF(page: puppeteer.Page) {
  return await page.evaluate(() => {
@@ -9,3 +10,12 @@ export async function waitForRAF(page: puppeteer.Page) {
    });
  });
 }
+
+export function getServerURL(server: http.Server): string {
+  const address = server.address();
+  if (address && typeof address !== 'string') {
+    return `http://localhost:${address.port}`;
+  } else {
+    return `${address}`;
+  }
+}
--- a/packages/rrweb/test/snapshots/integration.test.ts.snap
+++ b/packages/rrweb/test/snapshots/integration.test.ts.snap
@@ -12777,40 +12777,6 @@ exports[`record integration tests should record images inside iframe with blob u
        }
      ]
    }
-  },
-  {
-    \\"type\\": 3,
-    \\"data\\": {
-      \\"source\\": 0,
-      \\"texts\\": [],
-      \\"attributes\\": [
-        {
-          \\"id\\": 41,
-          \\"attributes\\": {
-            \\"crossorigin\\": \\"anonymous\\"
-          }
-        }
-      ],
-      \\"removes\\": [],
-      \\"adds\\": []
-    }
-  },
-  {
-    \\"type\\": 3,
-    \\"data\\": {
-      \\"source\\": 0,
-      \\"texts\\": [],
-      \\"attributes\\": [
-        {
-          \\"id\\": 41,
-          \\"attributes\\": {
-            \\"crossorigin\\": null
-          }
-        }
-      ],
-      \\"removes\\": [],
-      \\"adds\\": []
-    }
  }
 ]"
 `;
@@ -13245,40 +13211,6 @@ exports[`record integration tests should record images inside iframe with blob u
        }
      ]
    }
-  },
-  {
-    \\"type\\": 3,
-    \\"data\\": {
-      \\"source\\": 0,
-      \\"texts\\": [],
-      \\"attributes\\": [
-        {
-          \\"id\\": 47,
-          \\"attributes\\": {
-            \\"crossorigin\\": \\"anonymous\\"
-          }
-        }
-      ],
-      \\"removes\\": [],
-      \\"adds\\": []
-    }
-  },
-  {
-    \\"type\\": 3,
-    \\"data\\": {
-      \\"source\\": 0,
-      \\"texts\\": [],
-      \\"attributes\\": [
-        {
-          \\"id\\": 47,
-          \\"attributes\\": {
-            \\"crossorigin\\": null
-          }
-        }
-      ],
-      \\"removes\\": [],
-      \\"adds\\": []
-    }
  }
 ]"
 `;
@@ -13486,40 +13418,6 @@ exports[`record integration tests should record images with blob url 1`] = `
        }
      ]
    }
-  },
-  {
-    \\"type\\": 3,
-    \\"data\\": {
-      \\"source\\": 0,
-      \\"texts\\": [],
-      \\"attributes\\": [
-        {
-          \\"id\\": 24,
-          \\"attributes\\": {
-            \\"crossorigin\\": \\"anonymous\\"
-          }
-        }
-      ],
-      \\"removes\\": [],
-      \\"adds\\": []
-    }
-  },
-  {
-    \\"type\\": 3,
-    \\"data\\": {
-      \\"source\\": 0,
-      \\"texts\\": [],
-      \\"attributes\\": [
-        {
-          \\"id\\": 24,
-          \\"attributes\\": {
-            \\"crossorigin\\": null
-          }
-        }
-      ],
-      \\"removes\\": [],
-      \\"adds\\": []
-    }
  }
 ]"
 `;