Skip to content

Commit 4014305

Browse files
eoghanmurrayeoghanmurray
authored
inlineImages: Setting of image.crossOrigin is not always necessary (rrweb-io#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
1 parent 81c54ab commit 4014305

8 files changed

Lines changed: 95 additions & 117 deletions

File tree

‎.changeset/inlineImage-maybeNot-crossOrigin.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"rrweb": patch
3+
"rrweb-snapshot": patch
4+
---
5+
6+
inlineImages: during snapshot avoid adding an event listener for inlining of same-origin images (async listener mutates the snapshot which can be problematic)

‎packages/rrweb-snapshot/src/snapshot.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -747,8 +747,9 @@ function serializeElementNode(
747747
canvasCtx = canvasService.getContext('2d');
748748
}
749749
const image = n as HTMLImageElement;
750-
const oldValue = image.crossOrigin;
751-
image.crossOrigin = 'anonymous';
750+
const imageSrc: string =
751+
image.currentSrc || image.getAttribute('src') || '<unknown-src>';
752+
const priorCrossOrigin = image.crossOrigin;
752753
const recordInlineImage = () => {
753754
image.removeEventListener('load', recordInlineImage);
754755
try {
@@ -760,13 +761,23 @@ function serializeElementNode(
760761
dataURLOptions.quality,
761762
);
762763
} catch (err) {
763-
console.warn(
764-
`Cannot inline img src=${image.currentSrc}! Error: ${err as string}`,
765-
);
764+
if (image.crossOrigin !== 'anonymous') {
765+
image.crossOrigin = 'anonymous';
766+
if (image.complete && image.naturalWidth !== 0)
767+
recordInlineImage(); // too early due to image reload
768+
else image.addEventListener('load', recordInlineImage);
769+
return;
770+
} else {
771+
console.warn(
772+
`Cannot inline img src=${imageSrc}! Error: ${err as string}`,
773+
);
774+
}
775+
}
776+
if (image.crossOrigin === 'anonymous') {
777+
priorCrossOrigin
778+
? (attributes.crossOrigin = priorCrossOrigin)
779+
: image.removeAttribute('crossorigin');
766780
}
767-
oldValue
768-
? (attributes.crossOrigin = oldValue)
769-
: image.removeAttribute('crossorigin');
770781
};
771782
// The image content may not have finished loading yet.
772783
if (image.complete && image.naturalWidth !== 0) recordInlineImage();

‎packages/rrweb-snapshot/test/__snapshots__/integration.test.ts.snap‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ exports[`integration tests [html file]: mask-text.html 1`] = `
338338
exports[`integration tests [html file]: picture.html 1`] = `
339339
"<html xmlns=\\"http://www.w3.org/1999/xhtml\\"><head></head><body>
340340
<picture>
341+
<!-- these are 404 - not sure if that's intentional -->
341342
<source type=\\"image/webp\\" srcset=\\"http://localhost:3030/assets/img/characters/robot.webp\\" />
342343
<img src=\\"http://localhost:3030/assets/img/characters/robot.png\\" />
343344
</picture>

‎packages/rrweb-snapshot/test/html/picture.html‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<html xmlns="http://www.w3.org/1999/xhtml">
22
<body>
33
<picture>
4+
<!-- these are 404 - not sure if that's intentional -->
45
<source type="image/webp" srcset="assets/img/characters/robot.webp" />
56
<img src="assets/img/characters/robot.png" />
67
</picture>

‎packages/rrweb-snapshot/test/images/rrweb-favicon-20x20.png‎

487 Bytes
Loading

‎packages/rrweb-snapshot/test/integration.test.ts‎

Lines changed: 58 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import * as puppeteer from 'puppeteer';
66
import * as rollup from 'rollup';
77
import * as typescript from 'rollup-plugin-typescript2';
88
import * as assert from 'assert';
9-
import { waitForRAF } from './utils';
9+
import { waitForRAF, getServerURL } from './utils';
1010

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

@@ -209,12 +209,63 @@ iframe.contentDocument.querySelector('center').clientHeight
209209
inlineImages: true,
210210
inlineStylesheet: false
211211
})`);
212-
await waitForRAF(page);
213-
const snapshot = (await page.evaluate(
214-
'JSON.stringify(snapshot, null, 2);',
215-
)) as string;
216-
assert(snapshot.includes('"rr_dataURL"'));
217-
assert(snapshot.includes('data:image/webp;base64,'));
212+
// don't wait, as we want to ensure that the same-origin image can be inlined immediately
213+
const bodyChildren = (await page.evaluate(`
214+
snapshot.childNodes[0].childNodes[1].childNodes.filter((cn) => cn.type === 2);
215+
`)) as any[];
216+
expect(bodyChildren[1]).toEqual(
217+
expect.objectContaining({
218+
tagName: 'img',
219+
attributes: {
220+
src: expect.stringMatching(/images\/robot.png$/),
221+
alt: 'This is a robot',
222+
rr_dataURL: expect.stringMatching(/^data:image\/webp;base64,/),
223+
},
224+
}),
225+
);
226+
});
227+
228+
it('correctly saves cross-origin images offline', async () => {
229+
const page: puppeteer.Page = await browser.newPage();
230+
231+
await page.goto('about:blank', {
232+
waitUntil: 'load',
233+
});
234+
await page.setContent(
235+
`
236+
<html xmlns="http://www.w3.org/1999/xhtml">
237+
<body>
238+
<img src="${getServerURL(
239+
server,
240+
)}/images/rrweb-favicon-20x20.png" alt="CORS restricted but has access-control-allow-origin: *" />
241+
</body>
242+
</html>
243+
`,
244+
{
245+
waitUntil: 'load',
246+
},
247+
);
248+
249+
await page.waitForSelector('img', { timeout: 1000 });
250+
await page.evaluate(`${code}var snapshot = rrweb.snapshot(document, {
251+
dataURLOptions: { type: "image/webp", quality: 0.8 },
252+
inlineImages: true,
253+
inlineStylesheet: false
254+
})`);
255+
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)
256+
const bodyChildren = (await page.evaluate(`
257+
snapshot.childNodes[0].childNodes[1].childNodes.filter((cn) => cn.type === 2);
258+
`)) as any[];
259+
expect(bodyChildren[0]).toEqual(
260+
expect.objectContaining({
261+
tagName: 'img',
262+
attributes: {
263+
src: getServerURL(server) + '/images/rrweb-favicon-20x20.png',
264+
alt: 'CORS restricted but has access-control-allow-origin: *',
265+
rr_dataURL: expect.stringMatching(/^data:image\/webp;base64,/),
266+
},
267+
}),
268+
);
218269
});
219270

220271
it('correctly saves blob:images offline', async () => {

‎packages/rrweb-snapshot/test/utils.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as puppeteer from 'puppeteer';
2+
import * as http from 'http';
23

34
export async function waitForRAF(page: puppeteer.Page) {
45
return await page.evaluate(() => {
@@ -9,3 +10,12 @@ export async function waitForRAF(page: puppeteer.Page) {
910
});
1011
});
1112
}
13+
14+
export function getServerURL(server: http.Server): string {
15+
const address = server.address();
16+
if (address && typeof address !== 'string') {
17+
return `http://localhost:${address.port}`;
18+
} else {
19+
return `${address}`;
20+
}
21+
}

‎packages/rrweb/test/__snapshots__/integration.test.ts.snap‎

Lines changed: 0 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -12777,40 +12777,6 @@ exports[`record integration tests should record images inside iframe with blob u
1277712777
}
1277812778
]
1277912779
}
12780-
},
12781-
{
12782-
\\"type\\": 3,
12783-
\\"data\\": {
12784-
\\"source\\": 0,
12785-
\\"texts\\": [],
12786-
\\"attributes\\": [
12787-
{
12788-
\\"id\\": 41,
12789-
\\"attributes\\": {
12790-
\\"crossorigin\\": \\"anonymous\\"
12791-
}
12792-
}
12793-
],
12794-
\\"removes\\": [],
12795-
\\"adds\\": []
12796-
}
12797-
},
12798-
{
12799-
\\"type\\": 3,
12800-
\\"data\\": {
12801-
\\"source\\": 0,
12802-
\\"texts\\": [],
12803-
\\"attributes\\": [
12804-
{
12805-
\\"id\\": 41,
12806-
\\"attributes\\": {
12807-
\\"crossorigin\\": null
12808-
}
12809-
}
12810-
],
12811-
\\"removes\\": [],
12812-
\\"adds\\": []
12813-
}
1281412780
}
1281512781
]"
1281612782
`;
@@ -13245,40 +13211,6 @@ exports[`record integration tests should record images inside iframe with blob u
1324513211
}
1324613212
]
1324713213
}
13248-
},
13249-
{
13250-
\\"type\\": 3,
13251-
\\"data\\": {
13252-
\\"source\\": 0,
13253-
\\"texts\\": [],
13254-
\\"attributes\\": [
13255-
{
13256-
\\"id\\": 47,
13257-
\\"attributes\\": {
13258-
\\"crossorigin\\": \\"anonymous\\"
13259-
}
13260-
}
13261-
],
13262-
\\"removes\\": [],
13263-
\\"adds\\": []
13264-
}
13265-
},
13266-
{
13267-
\\"type\\": 3,
13268-
\\"data\\": {
13269-
\\"source\\": 0,
13270-
\\"texts\\": [],
13271-
\\"attributes\\": [
13272-
{
13273-
\\"id\\": 47,
13274-
\\"attributes\\": {
13275-
\\"crossorigin\\": null
13276-
}
13277-
}
13278-
],
13279-
\\"removes\\": [],
13280-
\\"adds\\": []
13281-
}
1328213214
}
1328313215
]"
1328413216
`;
@@ -13486,40 +13418,6 @@ exports[`record integration tests should record images with blob url 1`] = `
1348613418
}
1348713419
]
1348813420
}
13489-
},
13490-
{
13491-
\\"type\\": 3,
13492-
\\"data\\": {
13493-
\\"source\\": 0,
13494-
\\"texts\\": [],
13495-
\\"attributes\\": [
13496-
{
13497-
\\"id\\": 24,
13498-
\\"attributes\\": {
13499-
\\"crossorigin\\": \\"anonymous\\"
13500-
}
13501-
}
13502-
],
13503-
\\"removes\\": [],
13504-
\\"adds\\": []
13505-
}
13506-
},
13507-
{
13508-
\\"type\\": 3,
13509-
\\"data\\": {
13510-
\\"source\\": 0,
13511-
\\"texts\\": [],
13512-
\\"attributes\\": [
13513-
{
13514-
\\"id\\": 24,
13515-
\\"attributes\\": {
13516-
\\"crossorigin\\": null
13517-
}
13518-
}
13519-
],
13520-
\\"removes\\": [],
13521-
\\"adds\\": []
13522-
}
1352313421
}
1352413422
]"
1352513423
`;

0 commit comments

Comments
 (0)