From ba29607748e0fd08a82e4886945f14978e71c049 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Tue, 17 Jan 2023 20:13:11 +1100 Subject: [PATCH 1/6] fix: duplicated elements in shadow doms clean observers before taking new full snapshots --- .../rrweb/src/record/shadow-dom-manager.ts | 44 ++++++++++++------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/packages/rrweb/src/record/shadow-dom-manager.ts b/packages/rrweb/src/record/shadow-dom-manager.ts index fd90d23d90..b196aac5c7 100644 --- a/packages/rrweb/src/record/shadow-dom-manager.ts +++ b/packages/rrweb/src/record/shadow-dom-manager.ts @@ -26,6 +26,7 @@ export class ShadowDomManager { private scrollCb: scrollCallback; private bypassOptions: BypassOptions; private mirror: Mirror; + private observerHandlers: (() => void)[] = []; private restorePatches: (() => void)[] = []; constructor(options: { @@ -66,7 +67,7 @@ export class ShadowDomManager { if (!isNativeShadowDom(shadowRoot)) return; if (this.shadowDoms.has(shadowRoot)) return; this.shadowDoms.add(shadowRoot); - initMutationObserver( + const observer = initMutationObserver( { ...this.bypassOptions, doc, @@ -76,14 +77,17 @@ export class ShadowDomManager { }, shadowRoot, ); - initScrollObserver({ - ...this.bypassOptions, - scrollCb: this.scrollCb, - // https://gist.github.com/praveenpuglia/0832da687ed5a5d7a0907046c9ef1813 - // scroll is not allowed to pass the boundary, so we need to listen the shadow document - doc: (shadowRoot as unknown) as Document, - mirror: this.mirror, - }); + this.observerHandlers.push(() => observer.disconnect()); + this.observerHandlers.push( + initScrollObserver({ + ...this.bypassOptions, + scrollCb: this.scrollCb, + // https://gist.github.com/praveenpuglia/0832da687ed5a5d7a0907046c9ef1813 + // scroll is not allowed to pass the boundary, so we need to listen the shadow document + doc: (shadowRoot as unknown) as Document, + mirror: this.mirror, + }), + ); // Defer this to avoid adoptedStyleSheet events being created before the full snapshot is created or attachShadow action is recorded. setTimeout(() => { if ( @@ -94,12 +98,14 @@ export class ShadowDomManager { shadowRoot.adoptedStyleSheets, this.mirror.getId(shadowRoot.host), ); - initAdoptedStyleSheetObserver( - { - mirror: this.mirror, - stylesheetManager: this.bypassOptions.stylesheetManager, - }, - shadowRoot, + this.observerHandlers.push( + initAdoptedStyleSheetObserver( + { + mirror: this.mirror, + stylesheetManager: this.bypassOptions.stylesheetManager, + }, + shadowRoot, + ), ); }, 0); } @@ -135,6 +141,14 @@ export class ShadowDomManager { public clearCache() { this.shadowDoms = new WeakSet(); + this.observerHandlers.forEach((handler) => { + try { + handler(); + } catch (e) { + // + } + }); + this.observerHandlers = []; } public reset() { From c748b2f5673e31aba742c174eee98f8bac3ebf09 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Tue, 17 Jan 2023 20:44:34 +1100 Subject: [PATCH 2/6] add checker for replayer to make it stable when data has duplicated nodes --- packages/rrweb-snapshot/src/rebuild.ts | 15 ++- packages/rrweb-snapshot/src/utils.ts | 33 +++++ packages/rrweb-snapshot/test/utils.test.ts | 150 +++++++++++++++++++++ 3 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 packages/rrweb-snapshot/test/utils.test.ts diff --git a/packages/rrweb-snapshot/src/rebuild.ts b/packages/rrweb-snapshot/src/rebuild.ts index a491122d6b..8e9fcf7618 100644 --- a/packages/rrweb-snapshot/src/rebuild.ts +++ b/packages/rrweb-snapshot/src/rebuild.ts @@ -8,7 +8,7 @@ import { attributes, legacyAttributes, } from './types'; -import { isElement, Mirror } from './utils'; +import { isElement, Mirror, isNodeMetaEqual } from './utils'; const tagMap: tagMap = { script: 'noscript', @@ -364,6 +364,19 @@ export function buildNodeWithSN( afterAppend, cache, } = options; + /** + * Add a check to see if the node is already in the mirror. If it is, we can skip the whole process. + * This situation (duplicated nodes) can happen when recorder has some unfixed bugs and the same node is recorded twice. Or something goes wrong when saving or transferring event data. + * Duplicated node creation may cause unexpected errors in replayer. This check tries best effort to prevent the errors. + */ + if (mirror.has(n.id)) { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const nodeInMirror = mirror.getNode(n.id)!; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const meta = mirror.getMeta(nodeInMirror)!; + // For safety concern, check if the node in mirror is the same as the node we are trying to build + if (isNodeMetaEqual(meta, n)) return mirror.getNode(n.id); + } let node = buildNode(n, { doc, hackCss, cache }); if (!node) { return null; diff --git a/packages/rrweb-snapshot/src/utils.ts b/packages/rrweb-snapshot/src/utils.ts index 75e9a18008..f5d6ec92ac 100644 --- a/packages/rrweb-snapshot/src/utils.ts +++ b/packages/rrweb-snapshot/src/utils.ts @@ -5,6 +5,12 @@ import { nodeMetaMap, IMirror, serializedNodeWithId, + serializedNode, + NodeType, + documentNode, + documentTypeNode, + textNode, + elementNode, } from './types'; export function isElement(n: Node): n is Element { @@ -213,3 +219,30 @@ export function is2DCanvasBlank(canvas: HTMLCanvasElement): boolean { } return true; } + +export function isNodeMetaEqual(a: serializedNode, b: serializedNode): boolean { + if (!a || !b || a.type !== b.type) return false; + if (a.type === NodeType.Document) + return a.compatMode === (b as documentNode).compatMode; + else if (a.type === NodeType.DocumentType) + return ( + a.name === (b as documentTypeNode).name && + a.publicId === (b as documentTypeNode).publicId && + a.systemId === (b as documentTypeNode).systemId + ); + else if ( + a.type === NodeType.Comment || + a.type === NodeType.Text || + a.type === NodeType.CDATA + ) + return a.textContent === (b as textNode).textContent; + else if (a.type === NodeType.Element) + return ( + a.tagName === (b as elementNode).tagName && + JSON.stringify(a.attributes) === + JSON.stringify((b as elementNode).attributes) && + a.isSVG === (b as elementNode).isSVG && + a.needBlock === (b as elementNode).needBlock + ); + return false; +} diff --git a/packages/rrweb-snapshot/test/utils.test.ts b/packages/rrweb-snapshot/test/utils.test.ts new file mode 100644 index 0000000000..cf6f4b11a6 --- /dev/null +++ b/packages/rrweb-snapshot/test/utils.test.ts @@ -0,0 +1,150 @@ +/** + * @jest-environment jsdom + */ +import { NodeType, serializedNode } from '../src/types'; +import { isNodeMetaEqual } from '../src/utils'; +import { serializedNodeWithId } from 'rrweb-snapshot'; + +describe('utils', () => { + describe('isNodeMetaEqual()', () => { + const document1: serializedNode = { + type: NodeType.Document, + compatMode: 'CSS1Compat', + childNodes: [], + }; + const document2: serializedNode = { + type: NodeType.Document, + compatMode: 'BackCompat', + childNodes: [], + }; + const documentType1: serializedNode = { + type: NodeType.DocumentType, + name: 'html', + publicId: '', + systemId: '', + }; + const documentType2: serializedNode = { + type: NodeType.DocumentType, + name: 'html', + publicId: '', + systemId: 'http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd', + }; + const text1: serializedNode = { + type: NodeType.Text, + textContent: 'Hello World', + }; + const text2: serializedNode = { + type: NodeType.Text, + textContent: 'Hello world', + }; + const comment1: serializedNode = { + type: NodeType.Comment, + textContent: 'Hello World', + }; + const comment2: serializedNode = { + type: NodeType.Comment, + textContent: 'Hello world', + }; + const element1: serializedNode = { + type: NodeType.Element, + tagName: 'div', + attributes: { + className: 'test', + }, + childNodes: [], + }; + const element2: serializedNode = { + type: NodeType.Element, + tagName: 'span', + attributes: { + 'aria-label': 'Hello World', + }, + childNodes: [], + }; + const element3: serializedNode = { + type: NodeType.Element, + tagName: 'div', + attributes: { id: 'test' }, + childNodes: [comment1 as serializedNodeWithId], + }; + + it('should return false if two nodes have different node types', () => { + expect( + isNodeMetaEqual( + (undefined as unknown) as serializedNode, + (null as unknown) as serializedNode, + ), + ).toBeFalsy(); + expect(isNodeMetaEqual(document1, element1)).toBeFalsy(); + expect(isNodeMetaEqual(document1, documentType1)).toBeFalsy(); + expect(isNodeMetaEqual(documentType1, element1)).toBeFalsy(); + expect(isNodeMetaEqual(text1, comment1)).toBeFalsy(); + expect(isNodeMetaEqual(text1, element1)).toBeFalsy(); + expect(isNodeMetaEqual(comment1, element1)).toBeFalsy(); + }); + + it('should compare meta data of two document nodes', () => { + expect( + isNodeMetaEqual(document1, JSON.parse(JSON.stringify(document1))), + ).toBeTruthy(); + expect( + isNodeMetaEqual(JSON.parse(JSON.stringify(document2)), document2), + ).toBeTruthy(); + expect(isNodeMetaEqual(document1, document2)).toBeFalsy(); + }); + + it('should compare meta data of two documentType nodes', () => { + expect( + isNodeMetaEqual( + documentType1, + JSON.parse(JSON.stringify(documentType1)), + ), + ).toBeTruthy(); + expect( + isNodeMetaEqual( + JSON.parse(JSON.stringify(documentType2)), + documentType2, + ), + ).toBeTruthy(); + expect(isNodeMetaEqual(documentType1, documentType2)).toBeFalsy(); + }); + + it('should compare meta data of two text nodes', () => { + expect( + isNodeMetaEqual(text1, JSON.parse(JSON.stringify(text1))), + ).toBeTruthy(); + expect( + isNodeMetaEqual(JSON.parse(JSON.stringify(text2)), text2), + ).toBeTruthy(); + expect(isNodeMetaEqual(text1, text2)).toBeFalsy(); + }); + + it('should compare meta data of two comment nodes', () => { + expect( + isNodeMetaEqual(comment1, JSON.parse(JSON.stringify(comment1))), + ).toBeTruthy(); + expect( + isNodeMetaEqual(JSON.parse(JSON.stringify(comment2)), comment2), + ).toBeTruthy(); + expect(isNodeMetaEqual(comment1, comment2)).toBeFalsy(); + }); + + it('should compare meta data of two HTML elements', () => { + expect( + isNodeMetaEqual(element1, JSON.parse(JSON.stringify(element1))), + ).toBeTruthy(); + expect( + isNodeMetaEqual(JSON.parse(JSON.stringify(element2)), element2), + ).toBeTruthy(); + expect( + isNodeMetaEqual(element1, { + ...element1, + childNodes: [comment2 as serializedNodeWithId], + }), + ).toBeTruthy(); + expect(isNodeMetaEqual(element1, element2)).toBeFalsy(); + expect(isNodeMetaEqual(element1, element3)).toBeFalsy(); + expect(isNodeMetaEqual(element2, element3)).toBeFalsy(); + }); + }); +}); From ae3e8a9134f831c4a9e29c4756571ef84d6bf1a1 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Sat, 21 Jan 2023 13:11:37 +1100 Subject: [PATCH 3/6] apply review suggestions --- packages/rrweb/src/record/index.ts | 4 +- .../rrweb/src/record/shadow-dom-manager.ts | 109 +++++++++--------- 2 files changed, 54 insertions(+), 59 deletions(-) diff --git a/packages/rrweb/src/record/index.ts b/packages/rrweb/src/record/index.ts index c28ec28a2c..71e7d2bd73 100644 --- a/packages/rrweb/src/record/index.ts +++ b/packages/rrweb/src/record/index.ts @@ -345,8 +345,8 @@ function record( // When we take a full snapshot, old tracked StyleSheets need to be removed. stylesheetManager.reset(); - // Old shadow doms cache need to be cleared. - shadowDomManager.clearCache(); + + shadowDomManager.init(); mutationBuffers.forEach((buf) => buf.lock()); // don't allow any mirror modifications during snapshotting const node = snapshot(document, { diff --git a/packages/rrweb/src/record/shadow-dom-manager.ts b/packages/rrweb/src/record/shadow-dom-manager.ts index b196aac5c7..2935a27b24 100644 --- a/packages/rrweb/src/record/shadow-dom-manager.ts +++ b/packages/rrweb/src/record/shadow-dom-manager.ts @@ -26,8 +26,7 @@ export class ShadowDomManager { private scrollCb: scrollCallback; private bypassOptions: BypassOptions; private mirror: Mirror; - private observerHandlers: (() => void)[] = []; - private restorePatches: (() => void)[] = []; + private restoreHandlers: (() => void)[] = []; constructor(options: { mutationCb: mutationCallBack; @@ -40,27 +39,13 @@ export class ShadowDomManager { this.bypassOptions = options.bypassOptions; this.mirror = options.mirror; - // Patch 'attachShadow' to observe newly added shadow doms. - // eslint-disable-next-line @typescript-eslint/no-this-alias - const manager = this; - this.restorePatches.push( - patch( - Element.prototype, - 'attachShadow', - function (original: (init: ShadowRootInit) => ShadowRoot) { - return function (this: HTMLElement, option: ShadowRootInit) { - const shadowRoot = original.call(this, option); + this.init(); + } - // For the shadow dom elements in the document, monitor their dom mutations. - // For shadow dom elements that aren't in the document yet, - // we start monitoring them once their shadow dom host is appended to the document. - if (this.shadowRoot && inDom(this)) - manager.addShadowRoot(this.shadowRoot, this.ownerDocument); - return shadowRoot; - }; - }, - ), - ); + public init() { + this.reset(); + // Patch 'attachShadow' to observe newly added shadow doms. + this.patchAttachShadow(Element, document); } public addShadowRoot(shadowRoot: ShadowRoot, doc: Document) { @@ -77,8 +62,8 @@ export class ShadowDomManager { }, shadowRoot, ); - this.observerHandlers.push(() => observer.disconnect()); - this.observerHandlers.push( + this.restoreHandlers.push(() => observer.disconnect()); + this.restoreHandlers.push( initScrollObserver({ ...this.bypassOptions, scrollCb: this.scrollCb, @@ -98,7 +83,7 @@ export class ShadowDomManager { shadowRoot.adoptedStyleSheets, this.mirror.getId(shadowRoot.host), ); - this.observerHandlers.push( + this.restoreHandlers.push( initAdoptedStyleSheetObserver( { mirror: this.mirror, @@ -114,45 +99,55 @@ export class ShadowDomManager { * Monkey patch 'attachShadow' of an IFrameElement to observe newly added shadow doms. */ public observeAttachShadow(iframeElement: HTMLIFrameElement) { - if (iframeElement.contentWindow) { - // eslint-disable-next-line @typescript-eslint/no-this-alias - const manager = this; - this.restorePatches.push( - patch( - (iframeElement.contentWindow as Window & { - HTMLElement: { prototype: HTMLElement }; - }).HTMLElement.prototype, - 'attachShadow', - function (original: (init: ShadowRootInit) => ShadowRoot) { - return function (this: HTMLElement, option: ShadowRootInit) { - const shadowRoot = original.call(this, option); - if (this.shadowRoot) - manager.addShadowRoot( - this.shadowRoot, - iframeElement.contentDocument as Document, - ); - return shadowRoot; - }; - }, - ), - ); - } + if (!iframeElement.contentWindow || !iframeElement.contentDocument) return; + + this.patchAttachShadow( + (iframeElement.contentWindow as Window & { + Element: { prototype: Element }; + }).Element, + iframeElement.contentDocument, + ); } - public clearCache() { - this.shadowDoms = new WeakSet(); - this.observerHandlers.forEach((handler) => { + /** + * Patch 'attachShadow' to observe newly added shadow doms. + */ + private patchAttachShadow( + element: { + prototype: Element; + }, + doc: Document, + ) { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const manager = this; + this.restoreHandlers.push( + patch( + element.prototype, + 'attachShadow', + function (original: (init: ShadowRootInit) => ShadowRoot) { + return function (this: Element, option: ShadowRootInit) { + const shadowRoot = original.call(this, option); + // For the shadow dom elements in the document, monitor their dom mutations. + // For shadow dom elements that aren't in the document yet, + // we start monitoring them once their shadow dom host is appended to the document. + if (this.shadowRoot && inDom(this)) + manager.addShadowRoot(this.shadowRoot, doc); + return shadowRoot; + }; + }, + ), + ); + } + + public reset() { + this.restoreHandlers.forEach((handler) => { try { handler(); } catch (e) { // } }); - this.observerHandlers = []; - } - - public reset() { - this.restorePatches.forEach((restorePatch) => restorePatch()); - this.clearCache(); + this.restoreHandlers = []; + this.shadowDoms = new WeakSet(); } } From 36d72e370b85fc8bb5e5eb0be4d7bc027a64dabe Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Sat, 11 Feb 2023 21:40:30 +1100 Subject: [PATCH 4/6] add change log --- .changeset/loud-seals-raise.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/loud-seals-raise.md diff --git a/.changeset/loud-seals-raise.md b/.changeset/loud-seals-raise.md new file mode 100644 index 0000000000..fb7309d243 --- /dev/null +++ b/.changeset/loud-seals-raise.md @@ -0,0 +1,6 @@ +--- +'rrweb-snapshot': patch +'rrweb': patch +--- + +Fix duplicated shadow doms From 5aa18224c7f8d6256436454f2c8fcbfbd221415a Mon Sep 17 00:00:00 2001 From: Mark-Fenng Date: Sat, 11 Feb 2023 10:41:42 +0000 Subject: [PATCH 5/6] Apply formatting changes --- packages/rrweb-snapshot/test/utils.test.ts | 4 ++-- packages/rrweb/src/record/shadow-dom-manager.ts | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/rrweb-snapshot/test/utils.test.ts b/packages/rrweb-snapshot/test/utils.test.ts index cf6f4b11a6..09a2bebd4d 100644 --- a/packages/rrweb-snapshot/test/utils.test.ts +++ b/packages/rrweb-snapshot/test/utils.test.ts @@ -71,8 +71,8 @@ describe('utils', () => { it('should return false if two nodes have different node types', () => { expect( isNodeMetaEqual( - (undefined as unknown) as serializedNode, - (null as unknown) as serializedNode, + undefined as unknown as serializedNode, + null as unknown as serializedNode, ), ).toBeFalsy(); expect(isNodeMetaEqual(document1, element1)).toBeFalsy(); diff --git a/packages/rrweb/src/record/shadow-dom-manager.ts b/packages/rrweb/src/record/shadow-dom-manager.ts index 2935a27b24..169c77216a 100644 --- a/packages/rrweb/src/record/shadow-dom-manager.ts +++ b/packages/rrweb/src/record/shadow-dom-manager.ts @@ -69,7 +69,7 @@ export class ShadowDomManager { scrollCb: this.scrollCb, // https://gist.github.com/praveenpuglia/0832da687ed5a5d7a0907046c9ef1813 // scroll is not allowed to pass the boundary, so we need to listen the shadow document - doc: (shadowRoot as unknown) as Document, + doc: shadowRoot as unknown as Document, mirror: this.mirror, }), ); @@ -102,9 +102,11 @@ export class ShadowDomManager { if (!iframeElement.contentWindow || !iframeElement.contentDocument) return; this.patchAttachShadow( - (iframeElement.contentWindow as Window & { - Element: { prototype: Element }; - }).Element, + ( + iframeElement.contentWindow as Window & { + Element: { prototype: Element }; + } + ).Element, iframeElement.contentDocument, ); } From 08e4b1cabf27026cb4bd89035154887829d1908b Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Sat, 11 Feb 2023 22:06:42 +1100 Subject: [PATCH 6/6] update prettier to fit the master branch --- package.json | 1 + packages/rrweb/package.json | 2 -- yarn.lock | 12 +----------- 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index 2245470f48..1e1c77faf8 100644 --- a/package.json +++ b/package.json @@ -31,6 +31,7 @@ "lerna": "^4.0.0", "markdownlint": "^0.25.1", "markdownlint-cli": "^0.31.1", + "prettier": "2.8.4", "turbo": "^1.2.4", "typescript": "^4.7.3" }, diff --git a/packages/rrweb/package.json b/packages/rrweb/package.json index 85117bdb56..f0b9bc4232 100644 --- a/packages/rrweb/package.json +++ b/packages/rrweb/package.json @@ -53,7 +53,6 @@ "@types/jest-image-snapshot": "^5.1.0", "@types/node": "^17.0.21", "@types/offscreencanvas": "^2019.6.4", - "@types/prettier": "^2.3.2", "@types/puppeteer": "^5.4.4", "construct-style-sheets-polyfill": "^3.1.0", "cross-env": "^5.2.0", @@ -65,7 +64,6 @@ "jest": "^27.5.1", "jest-image-snapshot": "^5.2.0", "jest-snapshot": "^23.6.0", - "prettier": "2.2.1", "puppeteer": "^11.0.0", "rollup": "^2.68.0", "rollup-plugin-esbuild": "^4.9.1", diff --git a/yarn.lock b/yarn.lock index 4b0fc28a22..d9f44a60e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2530,11 +2530,6 @@ resolved "https://registry.npmjs.org/@types/prettier/-/prettier-2.4.1.tgz" integrity sha512-Fo79ojj3vdEZOHg3wR9ksAMRz4P3S5fDB5e/YWZiFnyFQI1WY2Vftu9XoXVVtJfxB7Bpce/QTqWSSntkz2Znrw== -"@types/prettier@^2.3.2": - version "2.3.2" - resolved "https://registry.npmjs.org/@types/prettier/-/prettier-2.3.2.tgz" - integrity sha512-eI5Yrz3Qv4KPUa/nSIAi0h+qX0XyewOliug5F2QAtuRg6Kjg6jfmxe1GIwoIRhZspD1A0RP8ANrPwvEXXtRFog== - "@types/pug@^2.0.4": version "2.0.5" resolved "https://registry.npmjs.org/@types/pug/-/pug-2.0.5.tgz" @@ -10411,12 +10406,7 @@ preserve@^0.2.0: resolved "https://registry.npmjs.org/preserve/-/preserve-0.2.0.tgz" integrity sha1-gV7R9uvGWSb4ZbMQwHE7yzMVzks= -prettier@2.2.1: - version "2.2.1" - resolved "https://registry.npmjs.org/prettier/-/prettier-2.2.1.tgz" - integrity sha512-PqyhM2yCjg/oKkFPtTGUojv7gnZAoG80ttl45O6x2Ug/rMJw4wcc9k6aaf2hibP7BGVCCM33gZoGjyvt9mm16Q== - -prettier@^2.7.1: +prettier@2.8.4, prettier@^2.7.1: version "2.8.4" resolved "https://registry.yarnpkg.com/prettier/-/prettier-2.8.4.tgz#34dd2595629bfbb79d344ac4a91ff948694463c3" integrity sha512-vIS4Rlc2FNh0BySk3Wkd6xmwxB0FpOndW5fisM5H8hsZSxU2VWVB5CWIkIjWvrHjIhxk2g3bfMKM87zNTrZddw==