From bd984aca6eb47227281c3e90eaa4e44bab42bd86 Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 15 Aug 2024 11:48:20 -0400 Subject: [PATCH 1/2] fix(rrdom): Ignore invalid DOM attributes when diffing We encountered an issue where replays with invalid attributes (e.g. `@click`) would break rendering the replay after seeking. The exception bubbles up to [here](https://github.com/rrweb-io/rrweb/blob/62093d4385a09eb0980c2ac02d97eea5ce2882be/packages/rrweb/src/replay/index.ts#L270-L279), which means the replay will continue to play, but the replay mirror will be incomplete. --- packages/rrdom/src/diff.ts | 10 +++++++++- packages/rrdom/test/diff.test.ts | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index 14c9278542..28ca9ee8df 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -328,7 +328,15 @@ function diffProps( } }; } else if (newTree.tagName === 'IFRAME' && name === 'srcdoc') continue; - else oldTree.setAttribute(name, newValue); + else { + try { + oldTree.setAttribute(name, newValue); + } catch (err) { + // We want to continue diffing so we quietly catch + // this exception. Otherwise, this can throw and bubble up to + // the `ReplayerEvents.Flush` listener and break rendering + } + } } for (const { name } of Array.from(oldAttributes)) diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index 7fbf3c6027..9c84568a49 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -336,6 +336,30 @@ describe('diff algorithm for rrdom', () => { expect((node as Node as HTMLElement).className).toBe('node'); }); + it('ignores invalid attributes', () => { + const tagName = 'DIV'; + const node = document.createElement(tagName); + const sn = Object.assign({}, elementSn, { + attributes: { '@click': 'foo' }, + tagName, + }); + mirror.add(node, sn); + + const rrDocument = new RRDocument(); + const rrNode = rrDocument.createElement(tagName); + const sn2 = Object.assign({}, elementSn, { + attributes: { '@click': 'foo' }, + tagName, + }); + rrDocument.mirror.add(rrNode, sn2); + + rrNode.attributes = { id: 'node1', class: 'node', '@click': 'foo' }; + diff(node, rrNode, replayer); + expect((node as Node as HTMLElement).id).toBe('node1'); + expect((node as Node as HTMLElement).className).toBe('node'); + expect('@click' in (node as Node as HTMLElement)).toBe(false); + }); + it('can update exist properties', () => { const tagName = 'DIV'; const node = document.createElement(tagName); From 058ed695089c14da3cf7b6631e3d5e9e0c78ac8f Mon Sep 17 00:00:00 2001 From: Billy Vong Date: Thu, 15 Aug 2024 11:50:02 -0400 Subject: [PATCH 2/2] add warning --- packages/rrdom/src/diff.ts | 1 + packages/rrdom/test/diff.test.ts | 2 ++ 2 files changed, 3 insertions(+) diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index 28ca9ee8df..c9e99e914f 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -335,6 +335,7 @@ function diffProps( // We want to continue diffing so we quietly catch // this exception. Otherwise, this can throw and bubble up to // the `ReplayerEvents.Flush` listener and break rendering + console.warn(err); } } } diff --git a/packages/rrdom/test/diff.test.ts b/packages/rrdom/test/diff.test.ts index 9c84568a49..fa5a8ccb8a 100644 --- a/packages/rrdom/test/diff.test.ts +++ b/packages/rrdom/test/diff.test.ts @@ -358,6 +358,8 @@ describe('diff algorithm for rrdom', () => { expect((node as Node as HTMLElement).id).toBe('node1'); expect((node as Node as HTMLElement).className).toBe('node'); expect('@click' in (node as Node as HTMLElement)).toBe(false); + expect(warn).toHaveBeenCalledTimes(1); + warn.mockClear(); }); it('can update exist properties', () => {