From 406a7fb48dcbc8aabe758600d78540b7aad3c045 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Fri, 23 Sep 2022 14:36:31 +1000 Subject: [PATCH 1/3] fix: bug when inlined link elements --- packages/rrdom/src/index.ts | 7 +++- packages/rrweb-snapshot/src/snapshot.ts | 1 - packages/rrweb/src/record/index.ts | 2 +- packages/rrweb/src/record/mutation.ts | 2 +- .../rrweb/src/record/stylesheet-manager.ts | 29 +++++++------- packages/rrweb/src/replay/index.ts | 40 +++++++++++++++++++ 6 files changed, 63 insertions(+), 18 deletions(-) diff --git a/packages/rrdom/src/index.ts b/packages/rrdom/src/index.ts index 776427a11a..c11d7bacbd 100644 --- a/packages/rrdom/src/index.ts +++ b/packages/rrdom/src/index.ts @@ -389,7 +389,12 @@ export class Mirror implements IMirror { } replace(id: number, n: RRNode) { - this.idNodeMap.set(id, n); + const oldNode = this.getNode(id); + if (oldNode) { + const meta = this.nodeMetaMap.get(oldNode); + if (meta) this.nodeMetaMap.set(n, meta); + this.idNodeMap.set(id, n); + } } reset() { diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index 1b42dd5b68..3708001599 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -1165,7 +1165,6 @@ export function serializeNodeWithId( }, stylesheetLoadTimeout, ); - if (isStylesheetLoaded(n as HTMLLinkElement) === false) return null; // add stylesheet in later mutation } return serializedNode; diff --git a/packages/rrweb/src/record/index.ts b/packages/rrweb/src/record/index.ts index 1718a036f4..26e07fc83a 100644 --- a/packages/rrweb/src/record/index.ts +++ b/packages/rrweb/src/record/index.ts @@ -329,7 +329,7 @@ function record( shadowDomManager.observeAttachShadow(iframe); }, onStylesheetLoad: (linkEl, childSn) => { - stylesheetManager.attachLinkElement(linkEl, childSn, mirror); + stylesheetManager.attachLinkElement(linkEl, childSn); }, keepIframeSrcFn, }); diff --git a/packages/rrweb/src/record/mutation.ts b/packages/rrweb/src/record/mutation.ts index c852f3eb27..d7724a16ba 100644 --- a/packages/rrweb/src/record/mutation.ts +++ b/packages/rrweb/src/record/mutation.ts @@ -324,7 +324,7 @@ export default class MutationBuffer { this.shadowDomManager.observeAttachShadow(iframe); }, onStylesheetLoad: (link, childSn) => { - this.stylesheetManager.attachLinkElement(link, childSn, this.mirror); + this.stylesheetManager.attachLinkElement(link, childSn); }, }); if (sn) { diff --git a/packages/rrweb/src/record/stylesheet-manager.ts b/packages/rrweb/src/record/stylesheet-manager.ts index 1a2f6dc1a5..35d8c513db 100644 --- a/packages/rrweb/src/record/stylesheet-manager.ts +++ b/packages/rrweb/src/record/stylesheet-manager.ts @@ -1,8 +1,9 @@ -import type { Mirror, serializedNodeWithId } from 'rrweb-snapshot'; +import type { elementNode, serializedNodeWithId } from 'rrweb-snapshot'; import { getCssRuleString } from 'rrweb-snapshot'; import type { adoptedStyleSheetCallback, adoptedStyleSheetParam, + attributeMutation, mutationCallBack, } from '../types'; import { StyleSheetMirror } from '../utils'; @@ -24,20 +25,20 @@ export class StylesheetManager { public attachLinkElement( linkEl: HTMLLinkElement, childSn: serializedNodeWithId, - mirror: Mirror, ) { - this.mutationCb({ - adds: [ - { - parentId: mirror.getId(linkEl), - nextId: null, - node: childSn, - }, - ], - removes: [], - texts: [], - attributes: [], - }); + if ('_cssText' in (childSn as elementNode).attributes) + this.mutationCb({ + adds: [], + removes: [], + texts: [], + attributes: [ + { + id: childSn.id, + attributes: (childSn as elementNode) + .attributes as attributeMutation['attributes'], + }, + ], + }); this.trackLinkElement(linkEl); } diff --git a/packages/rrweb/src/replay/index.ts b/packages/rrweb/src/replay/index.ts index ae33316b2d..bbabf9b054 100644 --- a/packages/rrweb/src/replay/index.ts +++ b/packages/rrweb/src/replay/index.ts @@ -6,6 +6,8 @@ import { createCache, Mirror, createMirror, + attributes, + serializedElementNodeWithId, } from 'rrweb-snapshot'; import { RRDocument, @@ -1643,6 +1645,44 @@ export class Replayer { (target as Element | RRElement).removeAttribute(attributeName); } else if (typeof value === 'string') { try { + // When building snapshot, some link styles haven't loaded. Then they are loaded, they will be inlined as incremental mutation change of attribute. We need to replace the old elements whose styles aren't inlined. + if ( + attributeName === '_cssText' && + (target.nodeName === 'LINK' || target.nodeName === 'STYLE') + ) { + try { + const newSn = mirror.getMeta( + target as Node & RRNode, + ) as serializedElementNodeWithId; + newSn.attributes = mutation.attributes as attributes; + const newNode = buildNodeWithSN(newSn, { + doc: target.ownerDocument as Document, // can be Document or RRDocument + mirror: mirror as Mirror, + skipChild: true, + hackCss: true, + cache: this.cache, + afterAppend: (node: Node | RRNode, id: number) => { + for (const plugin of this.config.plugins || []) { + if (plugin.onBuild) + plugin.onBuild(node, { id, replayer: this }); + } + }, + }); + const siblingNode = target.nextSibling; + const parentNode = target.parentNode; + if (newNode && parentNode) { + parentNode.removeChild(target as Node & RRNode); + parentNode.insertBefore( + newNode as Node & RRNode, + siblingNode as (Node & RRNode) | null, + ); + mirror.replace(mutation.id, newNode as Node & RRNode); + break; + } + } catch (e) { + // for safe + } + } (target as Element | RRElement).setAttribute( attributeName, value, From 10a750dd9b3ef2d81820eda6eb69e8498b25c5d1 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Fri, 23 Sep 2022 14:36:31 +1000 Subject: [PATCH 2/3] test: update snapshot for test cases --- packages/rrweb/src/replay/index.ts | 6 -- .../test/__snapshots__/record.test.ts.snap | 74 ++++++++++++------- packages/rrweb/test/record.test.ts | 13 +++- 3 files changed, 57 insertions(+), 36 deletions(-) diff --git a/packages/rrweb/src/replay/index.ts b/packages/rrweb/src/replay/index.ts index bbabf9b054..f1eb4a1c9e 100644 --- a/packages/rrweb/src/replay/index.ts +++ b/packages/rrweb/src/replay/index.ts @@ -1661,12 +1661,6 @@ export class Replayer { skipChild: true, hackCss: true, cache: this.cache, - afterAppend: (node: Node | RRNode, id: number) => { - for (const plugin of this.config.plugins || []) { - if (plugin.onBuild) - plugin.onBuild(node, { id, replayer: this }); - } - }, }); const siblingNode = target.nextSibling; const parentNode = target.parentNode; diff --git a/packages/rrweb/test/__snapshots__/record.test.ts.snap b/packages/rrweb/test/__snapshots__/record.test.ts.snap index 4081bbeb5d..7133805635 100644 --- a/packages/rrweb/test/__snapshots__/record.test.ts.snap +++ b/packages/rrweb/test/__snapshots__/record.test.ts.snap @@ -173,9 +173,12 @@ exports[`record captures CORS stylesheets that are still loading 1`] = ` \\"type\\": 3, \\"data\\": { \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [], + \\"removes\\": [], \\"adds\\": [ { - \\"parentId\\": 9, + \\"parentId\\": 4, \\"nextId\\": null, \\"node\\": { \\"type\\": 2, @@ -188,10 +191,7 @@ exports[`record captures CORS stylesheets that are still loading 1`] = ` \\"id\\": 9 } } - ], - \\"removes\\": [], - \\"texts\\": [], - \\"attributes\\": [] + ] } } ]" @@ -2007,7 +2007,19 @@ exports[`record captures stylesheets in iframes that are still loading 1`] = ` \\"type\\": 2, \\"tagName\\": \\"head\\", \\"attributes\\": {}, - \\"childNodes\\": [], + \\"childNodes\\": [ + { + \\"type\\": 2, + \\"tagName\\": \\"link\\", + \\"attributes\\": { + \\"rel\\": \\"stylesheet\\", + \\"href\\": \\"blob:null\\" + }, + \\"childNodes\\": [], + \\"rootId\\": 10, + \\"id\\": 13 + } + ], \\"rootId\\": 10, \\"id\\": 12 }, @@ -2039,25 +2051,17 @@ exports[`record captures stylesheets in iframes that are still loading 1`] = ` \\"type\\": 3, \\"data\\": { \\"source\\": 0, - \\"adds\\": [ + \\"adds\\": [], + \\"removes\\": [], + \\"texts\\": [], + \\"attributes\\": [ { - \\"parentId\\": 13, - \\"nextId\\": null, - \\"node\\": { - \\"type\\": 2, - \\"tagName\\": \\"link\\", - \\"attributes\\": { - \\"_cssText\\": \\"body { color: pink; }\\" - }, - \\"childNodes\\": [], - \\"rootId\\": 10, - \\"id\\": 13 + \\"id\\": 13, + \\"attributes\\": { + \\"_cssText\\": \\"body { color: pink; }\\" } } - ], - \\"removes\\": [], - \\"texts\\": [], - \\"attributes\\": [] + ] } } ]" @@ -2286,24 +2290,42 @@ exports[`record captures stylesheets that are still loading 1`] = ` \\"type\\": 3, \\"data\\": { \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [], + \\"removes\\": [], \\"adds\\": [ { - \\"parentId\\": 9, + \\"parentId\\": 4, \\"nextId\\": null, \\"node\\": { \\"type\\": 2, \\"tagName\\": \\"link\\", \\"attributes\\": { - \\"_cssText\\": \\"body { color: pink; }\\" + \\"rel\\": \\"stylesheet\\", + \\"href\\": \\"blob:null\\" }, \\"childNodes\\": [], \\"id\\": 9 } } - ], + ] + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"adds\\": [], \\"removes\\": [], \\"texts\\": [], - \\"attributes\\": [] + \\"attributes\\": [ + { + \\"id\\": 9, + \\"attributes\\": { + \\"_cssText\\": \\"body { color: pink; }\\" + } + } + ] } } ]" diff --git a/packages/rrweb/test/record.test.ts b/packages/rrweb/test/record.test.ts index db7e0be4d4..e353559a26 100644 --- a/packages/rrweb/test/record.test.ts +++ b/packages/rrweb/test/record.test.ts @@ -626,8 +626,11 @@ describe('record', function (this: ISuite) { // `blob:` URLs are not available immediately, so we need to wait for the browser to load them await waitForRAF(ctx.page); - - assertSnapshot(ctx.events); + // 'blob' URL is different in every execution so we need to remove it from the snapshot. + const filteredEvents = JSON.parse( + JSON.stringify(ctx.events).replace(/blob\:[\w\d-/]+"/, 'blob:null"'), + ); + assertSnapshot(filteredEvents); }); it('captures stylesheets in iframes that are still loading', async () => { @@ -659,8 +662,10 @@ describe('record', function (this: ISuite) { // `blob:` URLs are not available immediately, so we need to wait for the browser to load them await waitForRAF(ctx.page); - - assertSnapshot(ctx.events); + const filteredEvents = JSON.parse( + JSON.stringify(ctx.events).replace(/blob\:[\w\d-/]+"/, 'blob:null"'), + ); + assertSnapshot(filteredEvents); }); it('captures CORS stylesheets that are still loading', async () => { From 8aefe2b7550686bb9ebdb055943570466b73af71 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Fri, 23 Sep 2022 14:36:31 +1000 Subject: [PATCH 3/3] apply Justin's review suggestions 1. make Mirror's replace function act the same with the original one when there's no existed node to get replaced. 2. when replacing with the link/style elements, keep their existing attributes to prevent potential bugs --- packages/rrdom/src/index.ts | 2 +- packages/rrweb/src/replay/index.ts | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/rrdom/src/index.ts b/packages/rrdom/src/index.ts index c11d7bacbd..da96b12db2 100644 --- a/packages/rrdom/src/index.ts +++ b/packages/rrdom/src/index.ts @@ -393,8 +393,8 @@ export class Mirror implements IMirror { if (oldNode) { const meta = this.nodeMetaMap.get(oldNode); if (meta) this.nodeMetaMap.set(n, meta); - this.idNodeMap.set(id, n); } + this.idNodeMap.set(id, n); } reset() { diff --git a/packages/rrweb/src/replay/index.ts b/packages/rrweb/src/replay/index.ts index f1eb4a1c9e..fd9fbf4b19 100644 --- a/packages/rrweb/src/replay/index.ts +++ b/packages/rrweb/src/replay/index.ts @@ -1654,7 +1654,10 @@ export class Replayer { const newSn = mirror.getMeta( target as Node & RRNode, ) as serializedElementNodeWithId; - newSn.attributes = mutation.attributes as attributes; + Object.assign( + newSn.attributes, + mutation.attributes as attributes, + ); const newNode = buildNodeWithSN(newSn, { doc: target.ownerDocument as Document, // can be Document or RRDocument mirror: mirror as Mirror,