From 178f1e6e450e0903e9dadc4dc96dd74236f296ba Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Mon, 8 Jul 2024 14:27:53 -0700 Subject: [PATCH] fix: duplicate textContent for style element cause incremental style mutation invalid (#1417) fix style element corner case - historically we have recorded duplicated css content in certain cases (demonstrated by the attached replayer test). This fix ensures that the replayer doesn't doubly add the content, which can cause problems when further mutations occur --------- Review and further tests contributed by: Eoghan Murray --- .changeset/cuddly-bikes-fail.md | 6 + packages/rrweb-snapshot/src/rebuild.ts | 3 +- packages/rrweb/src/replay/index.ts | 39 ++- .../__snapshots__/integration.test.ts.snap | 218 +++++++++++++++++ packages/rrweb/test/events/bad-style.ts | 231 ++++++++++++++++++ packages/rrweb/test/integration.test.ts | 100 ++++++++ packages/rrweb/test/replayer.test.ts | 26 +- 7 files changed, 610 insertions(+), 13 deletions(-) create mode 100644 .changeset/cuddly-bikes-fail.md create mode 100644 packages/rrweb/test/events/bad-style.ts diff --git a/.changeset/cuddly-bikes-fail.md b/.changeset/cuddly-bikes-fail.md new file mode 100644 index 0000000000..0c99160c9e --- /dev/null +++ b/.changeset/cuddly-bikes-fail.md @@ -0,0 +1,6 @@ +--- +"rrweb-snapshot": patch +"rrweb": patch +--- + +fix: duplicate textContent for style elements cause incremental style mutations to be invalid diff --git a/packages/rrweb-snapshot/src/rebuild.ts b/packages/rrweb-snapshot/src/rebuild.ts index 8c47f08e01..ab1a5d4bcc 100644 --- a/packages/rrweb-snapshot/src/rebuild.ts +++ b/packages/rrweb-snapshot/src/rebuild.ts @@ -160,8 +160,9 @@ function buildNode( value = adaptCssForReplay(value, cache); } if ((isTextarea || isRemoteOrDynamicCss) && typeof value === 'string') { - node.appendChild(doc.createTextNode(value)); // https://github.com/rrweb-io/rrweb/issues/112 + // https://github.com/rrweb-io/rrweb/pull/1351 + node.appendChild(doc.createTextNode(value)); n.childNodes = []; // value overrides childNodes continue; } diff --git a/packages/rrweb/src/replay/index.ts b/packages/rrweb/src/replay/index.ts index c5d03b1c40..ad9855b2fb 100644 --- a/packages/rrweb/src/replay/index.ts +++ b/packages/rrweb/src/replay/index.ts @@ -1558,20 +1558,39 @@ export class Replayer { if ( parentSn && parentSn.type === NodeType.Element && - parentSn.tagName === 'textarea' && mutation.node.type === NodeType.Text ) { - const childNodeArray = Array.isArray(parent.childNodes) + const prospectiveSiblings = Array.isArray(parent.childNodes) ? parent.childNodes : Array.from(parent.childNodes); - // This should be redundant now as we are either recording the value or the childNode, and not both - // keeping around for backwards compatibility with old bad double data, see - - // https://github.com/rrweb-io/rrweb/issues/745 - // parent is textarea, will only keep one child node as the value - for (const c of childNodeArray) { - if (c.nodeType === parent.TEXT_NODE) { - parent.removeChild(c as Node & RRNode); + if (parentSn.tagName === 'textarea') { + // This should be redundant now as we are either recording the value or the childNode, and not both + // keeping around for backwards compatibility with old bad double data, see + + // https://github.com/rrweb-io/rrweb/issues/745 + // parent is textarea, will only keep one child node as the value + for (const c of prospectiveSiblings) { + if (c.nodeType === parent.TEXT_NODE) { + parent.removeChild(c as Node & RRNode); + } + } + } else if ( + parentSn.tagName === 'style' && + prospectiveSiblings.length === 1 + ) { + // https://github.com/rrweb-io/rrweb/pull/1417 + /** + * If both _cssText and textContent are present for a style element due to some existing bugs, the element was ending up with two child text nodes + * We need to remove the textNode created by _cssText as it doesn't have an id in the mirror, and thus cannot be further mutated. + */ + for (const cssText of prospectiveSiblings as (Node & RRNode)[]) { + if ( + cssText.nodeType === parent.TEXT_NODE && + !mirror.hasNode(cssText) + ) { + target.textContent = cssText.textContent; + parent.removeChild(cssText); + } } } } else if (parentSn?.type === NodeType.Document) { diff --git a/packages/rrweb/test/__snapshots__/integration.test.ts.snap b/packages/rrweb/test/__snapshots__/integration.test.ts.snap index 5e15f35ddd..032bdbec63 100644 --- a/packages/rrweb/test/__snapshots__/integration.test.ts.snap +++ b/packages/rrweb/test/__snapshots__/integration.test.ts.snap @@ -15851,6 +15851,224 @@ exports[`record integration tests > should record shadow doms polyfilled by synt ]" `; +exports[`record integration tests > should record style mutations and replay them correctly 1`] = ` +"[ + { + \\"type\\": 4, + \\"data\\": { + \\"href\\": \\"about:blank\\", + \\"width\\": 1920, + \\"height\\": 1080 + } + }, + { + \\"type\\": 2, + \\"data\\": { + \\"node\\": { + \\"type\\": 0, + \\"childNodes\\": [ + { + \\"type\\": 1, + \\"name\\": \\"html\\", + \\"publicId\\": \\"\\", + \\"systemId\\": \\"\\", + \\"id\\": 2 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"html\\", + \\"attributes\\": { + \\"lang\\": \\"en\\" + }, + \\"childNodes\\": [ + { + \\"type\\": 2, + \\"tagName\\": \\"head\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n\\\\t \\", + \\"id\\": 5 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"style\\", + \\"attributes\\": { + \\"_cssText\\": \\"#one { color: rgb(255, 0, 0); }\\" + }, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"#one { color: rgb(255, 0, 0); }\\", + \\"isStyle\\": true, + \\"id\\": 7 + } + ], + \\"id\\": 6 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\", + \\"id\\": 8 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"script\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"SCRIPT_PLACEHOLDER\\", + \\"id\\": 10 + } + ], + \\"id\\": 9 + } + ], + \\"id\\": 4 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\", + \\"id\\": 11 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"body\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n\\\\t \\", + \\"id\\": 13 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"div\\", + \\"attributes\\": { + \\"id\\": \\"one\\" + }, + \\"childNodes\\": [], + \\"id\\": 14 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\", + \\"id\\": 15 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"div\\", + \\"attributes\\": { + \\"id\\": \\"two\\" + }, + \\"childNodes\\": [], + \\"id\\": 16 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n\\\\t \\", + \\"id\\": 17 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"script\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"SCRIPT_PLACEHOLDER\\", + \\"id\\": 19 + } + ], + \\"id\\": 18 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\\\n \\", + \\"id\\": 20 + } + ], + \\"id\\": 12 + } + ], + \\"id\\": 3 + } + ], + \\"id\\": 1 + }, + \\"initialOffset\\": { + \\"left\\": 0, + \\"top\\": 0 + } + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [], + \\"removes\\": [], + \\"adds\\": [ + { + \\"parentId\\": 4, + \\"nextId\\": null, + \\"node\\": { + \\"type\\": 2, + \\"tagName\\": \\"style\\", + \\"attributes\\": { + \\"_cssText\\": \\"#two { color: rgb(255, 0, 0); }\\" + }, + \\"childNodes\\": [], + \\"id\\": 21 + } + }, + { + \\"parentId\\": 21, + \\"nextId\\": null, + \\"node\\": { + \\"type\\": 3, + \\"textContent\\": \\"#two { color: rgb(255, 0, 0); }\\", + \\"isStyle\\": true, + \\"id\\": 22 + } + } + ] + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 13, + \\"id\\": 6, + \\"set\\": { + \\"property\\": \\"color\\", + \\"value\\": \\"rgb(255, 255, 0)\\" + }, + \\"index\\": [ + 0 + ] + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 13, + \\"id\\": 21, + \\"set\\": { + \\"property\\": \\"color\\", + \\"value\\": \\"rgb(255, 255, 0)\\" + }, + \\"index\\": [ + 0 + ] + } + } +]" +`; + exports[`record integration tests > should record webgl canvas mutations 1`] = ` "[ { diff --git a/packages/rrweb/test/events/bad-style.ts b/packages/rrweb/test/events/bad-style.ts new file mode 100644 index 0000000000..67d2a35fb1 --- /dev/null +++ b/packages/rrweb/test/events/bad-style.ts @@ -0,0 +1,231 @@ +import { EventType, IncrementalSource } from '@rrweb/types'; +import type { eventWithTime } from '@rrweb/types'; + +/** + * https://github.com/rrweb-io/rrweb/pull/1417 + * make sure we can replay duplicated content in style elements generated by older versions of rrweb + * test case demonstrates the duplicated content in question in two places; the initial snapshot, and in a style mutation + * (see 'BAD: duplicated content' below) + * these should no longer be generated after https://github.com/rrweb-io/rrweb/pull/1437 + */ +const now = Date.now(); +const events: eventWithTime[] = [ + { + timestamp: now, + type: EventType.Meta, + data: { + href: 'about:blank', + width: 1920, + height: 1080, + }, + }, + { + timestamp: now, + type: EventType.FullSnapshot, + data: { + node: { + type: 0, + childNodes: [ + { + type: 1, + name: 'html', + publicId: '', + systemId: '', + id: 2, + }, + { + type: 2, + tagName: 'html', + attributes: { + lang: 'en', + }, + childNodes: [ + { + type: 2, + tagName: 'head', + attributes: {}, + childNodes: [ + { + type: 3, + textContent: '\\\\n\\\\t ', + id: 5, + }, + { + type: 2, + tagName: 'style', + attributes: { + _cssText: '#one { color: rgb(255, 0, 0); }', + }, + childNodes: [ + { + type: 3, + // BAD: duplicated content (tweaked to different color) + textContent: '#one { color: rgb(155, 0, 0); }', + isStyle: true, + id: 7, + }, + ], + id: 6, + }, + { + type: 3, + textContent: '\\\\n ', + id: 8, + }, + { + type: 2, + tagName: 'script', + attributes: {}, + childNodes: [ + { + type: 3, + textContent: 'SCRIPT_PLACEHOLDER', + id: 10, + }, + ], + id: 9, + }, + ], + id: 4, + }, + { + type: 3, + textContent: '\\\\n ', + id: 11, + }, + { + type: 2, + tagName: 'body', + attributes: {}, + childNodes: [ + { + type: 3, + textContent: '\\\\n\\\\t ', + id: 13, + }, + { + type: 2, + tagName: 'div', + attributes: { + id: 'one', + }, + childNodes: [], + id: 14, + }, + { + type: 3, + textContent: '\\\\n ', + id: 15, + }, + { + type: 2, + tagName: 'div', + attributes: { + id: 'two', + }, + childNodes: [], + id: 16, + }, + { + type: 3, + textContent: '\\\\n\\\\t ', + id: 17, + }, + { + type: 2, + tagName: 'script', + attributes: {}, + childNodes: [ + { + type: 3, + textContent: 'SCRIPT_PLACEHOLDER', + id: 19, + }, + ], + id: 18, + }, + { + type: 3, + textContent: '\\\\n \\\\n ', + id: 20, + }, + ], + id: 12, + }, + ], + id: 3, + }, + ], + id: 1, + }, + initialOffset: { + left: 0, + top: 0, + }, + }, + }, + { + timestamp: now + 1, + type: EventType.IncrementalSnapshot, + data: { + source: IncrementalSource.Mutation, + texts: [], + attributes: [], + removes: [], + adds: [ + { + parentId: 4, + nextId: null, + node: { + type: 2, + tagName: 'style', + attributes: { + _cssText: '#two { color: rgb(255, 0, 0); }', + }, + childNodes: [], + id: 21, + }, + }, + { + parentId: 21, + nextId: null, + node: { + type: 3, + // BAD: duplicated content (tweaked to different color) + textContent: '#two { color: rgb(155, 0, 0); }', + isStyle: true, + id: 22, + }, + }, + ], + }, + }, + { + timestamp: now + 2, + type: EventType.IncrementalSnapshot, + data: { + source: IncrementalSource.StyleDeclaration, + id: 6, + set: { + property: 'color', + value: 'rgb(255, 255, 0)', + }, + index: [0], + }, + }, + { + timestamp: now + 3, + type: EventType.IncrementalSnapshot, + data: { + source: IncrementalSource.StyleDeclaration, + id: 21, + set: { + property: 'color', + value: 'rgb(255, 255, 0)', + }, + index: [0], + }, + }, +]; + +export default events; diff --git a/packages/rrweb/test/integration.test.ts b/packages/rrweb/test/integration.test.ts index 5153728e58..29615d58ef 100644 --- a/packages/rrweb/test/integration.test.ts +++ b/packages/rrweb/test/integration.test.ts @@ -1236,4 +1236,104 @@ describe('record integration tests', function (this: ISuite) { )) as eventWithTime[]; await assertSnapshot(snapshots); }); + + /** + * https://github.com/rrweb-io/rrweb/pull/1417 + * This test is to make sure that this problem doesn't regress + * Test case description: + * 1. Record two style elements. One is recorded as a full snapshot and the other is recorded as an incremental snapshot. + * 2. Change the color of both style elements to yellow as incremental style mutation. + * 3. Replay the recorded events and check if the style mutation is applied correctly. + */ + it('should record style mutations and replay them correctly', async () => { + const page: puppeteer.Page = await browser.newPage(); + const OldColor = 'rgb(255, 0, 0)'; // red color + const NewColor = 'rgb(255, 255, 0)'; // yellow color + + await page.setContent( + ` + + + + + +
+
+ + + `, + ); + // Start rrweb recording + await page.evaluate( + (code, recordSnippet) => { + const script = document.createElement('script'); + script.textContent = `${code}window.Date.now = () => new Date(Date.UTC(2018, 10, 15, 8)).valueOf();${recordSnippet}`; + document.head.appendChild(script); + }, + code, + generateRecordSnippet({}), + ); + + await page.evaluate( + async (OldColor, NewColor) => { + // Create a new style element with the same content as the existing style element and apply it to the #two div element + const incrementalStyle = document.createElement( + 'style', + ) as HTMLStyleElement; + incrementalStyle.textContent = ` \n`; + document.head.appendChild(incrementalStyle); + incrementalStyle.sheet!.insertRule(`#two { color: ${OldColor}; }`, 0); + + await new Promise((resolve) => + requestAnimationFrame(() => { + requestAnimationFrame(resolve); + }), + ); + + // Change the color of the #one div element to yellow as an incremental style mutation + const styleElement = document.querySelector('style')!; + (styleElement.sheet!.cssRules[0] as any).style.setProperty( + 'color', + NewColor, + ); + // Change the color of the #two div element to yellow as an incremental style mutation + (incrementalStyle.sheet!.cssRules[0] as any).style.setProperty( + 'color', + NewColor, + ); + }, + OldColor, + NewColor, + ); + await waitForRAF(page); + + const snapshots = (await page.evaluate( + 'window.snapshots', + )) as eventWithTime[]; + await assertSnapshot(snapshots); + + /** + * Replay the recorded events and check if the style mutation is applied correctly + */ + const changedColors = await page.evaluate(` + const { Replayer } = rrweb; + const replayer = new Replayer(window.snapshots); + replayer.pause(1000); + + // Get the color of the element after applying the style mutation event + [ + window.getComputedStyle( + replayer.iframe.contentDocument.querySelector('#one'), + ).color, + window.getComputedStyle( + replayer.iframe.contentDocument.querySelector('#two'), + ).color, + ]; + `); + expect(changedColors).toEqual([NewColor, NewColor]); + await page.close(); + }); }); diff --git a/packages/rrweb/test/replayer.test.ts b/packages/rrweb/test/replayer.test.ts index be11b62df1..94a9910829 100644 --- a/packages/rrweb/test/replayer.test.ts +++ b/packages/rrweb/test/replayer.test.ts @@ -16,8 +16,9 @@ import scrollEvents from './events/scroll'; import scrollWithParentStylesEvents from './events/scroll-with-parent-styles'; import selectionEvents from './events/selection'; import shadowDomEvents from './events/shadow-dom'; -import textareaEvents from './events/bad-textarea'; import styleSheetRuleEvents from './events/style-sheet-rule-events'; +import badTextareaEvents from './events/bad-textarea'; +import badStyleEvents from './events/bad-style'; import StyleSheetTextMutation from './events/style-sheet-text-mutation'; import { assertDomSnapshot, @@ -1142,7 +1143,7 @@ describe('replayer', function () { }); it('can deal with legacy duplicate/conflicting values on textareas', async () => { - await page.evaluate(`events = ${JSON.stringify(textareaEvents)}`); + await page.evaluate(`events = ${JSON.stringify(badTextareaEvents)}`); const displayValue = await page.evaluate(` const { Replayer } = rrweb; @@ -1155,4 +1156,25 @@ describe('replayer', function () { // If the custom element is defined, the display value will be 'block'. expect(displayValue).toEqual('this value is used for replay'); }); + + it('can deal with duplicate/conflicting values on style elements', async () => { + await page.evaluate(`events = ${JSON.stringify(badStyleEvents)}`); + + const changedColors = await page.evaluate(` + const { Replayer } = rrweb; + const replayer = new Replayer(events); + replayer.pause(1000); + // Get the color of the elements after applying the style mutation event + [ + replayer.iframe.contentWindow.getComputedStyle( + replayer.iframe.contentDocument.querySelector('#one'), + ).color, + replayer.iframe.contentWindow.getComputedStyle( + replayer.iframe.contentDocument.querySelector('#two'), + ).color, + ]; +`); + const newColor = 'rgb(255, 255, 0)'; // yellow + expect(changedColors).toEqual([newColor, newColor]); + }); });