From 0dda2191a08f6627d66c9be8538df74ba3e91816 Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Wed, 8 Nov 2023 10:43:06 +0000 Subject: [PATCH 01/10] masking: avoid the repeated calls to `closest` when recursing through the DOM --- packages/rrweb-snapshot/src/snapshot.ts | 72 ++++++++++++++++--------- packages/rrweb/src/record/mutation.ts | 3 +- 2 files changed, 49 insertions(+), 26 deletions(-) diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index e6b25dc92b..d525f3e684 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -306,28 +306,44 @@ export function classMatchesRegex( return classMatchesRegex(node.parentNode, regex, checkAncestors); } +// used on newly added mutations export function needMaskingText( - node: Node, + node: Text, maskTextClass: string | RegExp, maskTextSelector: string | null, ): boolean { try { - const el: HTMLElement | null = - node.nodeType === node.ELEMENT_NODE - ? (node as HTMLElement) - : node.parentElement; + const el = node.parentElement; if (el === null) return false; - if (typeof maskTextClass === 'string') { - if (el.classList.contains(maskTextClass)) return true; if (el.closest(`.${maskTextClass}`)) return true; } else { + // TODO: this doesn't check ancestors for mutations if (classMatchesRegex(el, maskTextClass, true)) return true; } + if (maskTextSelector) { + if (el.closest(maskTextSelector)) return true; + } + } catch (e) { + // + } + return false; +} +// used upon first serialization +function elementNeedsTextMasked( + el: Element, + maskTextClass: string | RegExp, + maskTextSelector: string | null, +): boolean { + try { + if (typeof maskTextClass === 'string') { + if (el.classList.contains(maskTextClass)) return true; + } else { + if (classMatchesRegex(el, maskTextClass, true)) return true; + } if (maskTextSelector) { if (el.matches(maskTextSelector)) return true; - if (el.closest(maskTextSelector)) return true; } } catch (e) { // @@ -426,8 +442,7 @@ function serializeNode( mirror: Mirror; blockClass: string | RegExp; blockSelector: string | null; - maskTextClass: string | RegExp; - maskTextSelector: string | null; + needsMask: boolean; inlineStylesheet: boolean; maskInputOptions: MaskInputOptions; maskTextFn: MaskTextFn | undefined; @@ -447,8 +462,7 @@ function serializeNode( mirror, blockClass, blockSelector, - maskTextClass, - maskTextSelector, + needsMask, inlineStylesheet, maskInputOptions = {}, maskTextFn, @@ -500,8 +514,7 @@ function serializeNode( }); case n.TEXT_NODE: return serializeTextNode(n as Text, { - maskTextClass, - maskTextSelector, + needsMask, maskTextFn, rootId, }); @@ -531,13 +544,12 @@ function getRootId(doc: Document, mirror: Mirror): number | undefined { function serializeTextNode( n: Text, options: { - maskTextClass: string | RegExp; - maskTextSelector: string | null; + needsMask: boolean; maskTextFn: MaskTextFn | undefined; rootId: number | undefined; }, ): serializedNode { - const { maskTextClass, maskTextSelector, maskTextFn, rootId } = options; + const { needsMask, maskTextFn, rootId } = options; // The parent node may not be a html element which has a tagName attribute. // So just let it be undefined which is ok in this use case. const parentTagName = n.parentNode && (n.parentNode as HTMLElement).tagName; @@ -568,12 +580,7 @@ function serializeTextNode( if (isScript) { textContent = 'SCRIPT_PLACEHOLDER'; } - if ( - !isStyle && - !isScript && - textContent && - needMaskingText(n, maskTextClass, maskTextSelector) - ) { + if (!isStyle && !isScript && textContent && needsMask) { textContent = maskTextFn ? maskTextFn(textContent, n.parentElement) : textContent.replace(/[\S]/g, '*'); @@ -935,6 +942,7 @@ export function serializeNodeWithId( inlineStylesheet: boolean; newlyAddedElement?: boolean; maskInputOptions?: MaskInputOptions; + needsMask: boolean; maskTextFn: MaskTextFn | undefined; maskInputFn: MaskInputFn | undefined; slimDOMOptions: SlimDOMOptions; @@ -980,14 +988,14 @@ export function serializeNodeWithId( keepIframeSrcFn = () => false, newlyAddedElement = false, } = options; + let { needsMask } = options; let { preserveWhiteSpace = true } = options; const _serializedNode = serializeNode(n, { doc, mirror, blockClass, blockSelector, - maskTextClass, - maskTextSelector, + needsMask, inlineStylesheet, maskInputOptions, maskTextFn, @@ -1039,6 +1047,16 @@ export function serializeNodeWithId( const shadowRoot = (n as HTMLElement).shadowRoot; if (shadowRoot && isNativeShadowDom(shadowRoot)) serializedNode.isShadowHost = true; + if (!needsMask) { + // we've already serialized this Element, but that's okay as masking + // is only relevant for child Text nodes. + // perf: if true, children won't also need to check + needsMask = elementNeedsTextMasked( + n as Element, + maskTextClass, + maskTextSelector, + ); + } } if ( (serializedNode.type === NodeType.Document || @@ -1058,6 +1076,7 @@ export function serializeNodeWithId( mirror, blockClass, blockSelector, + needsMask, maskTextClass, maskTextSelector, skipChild, @@ -1118,6 +1137,7 @@ export function serializeNodeWithId( mirror, blockClass, blockSelector, + needsMask, maskTextClass, maskTextSelector, skipChild: false, @@ -1165,6 +1185,7 @@ export function serializeNodeWithId( mirror, blockClass, blockSelector, + needsMask, maskTextClass, maskTextSelector, skipChild: false, @@ -1306,6 +1327,7 @@ function snapshot( skipChild: false, inlineStylesheet, maskInputOptions, + needsMask: false, maskTextFn, maskInputFn, slimDOMOptions, diff --git a/packages/rrweb/src/record/mutation.ts b/packages/rrweb/src/record/mutation.ts index 7c209605d0..5ce4f2dc34 100644 --- a/packages/rrweb/src/record/mutation.ts +++ b/packages/rrweb/src/record/mutation.ts @@ -300,6 +300,7 @@ export default class MutationBuffer { mirror: this.mirror, blockClass: this.blockClass, blockSelector: this.blockSelector, + needsMask: false, maskTextClass: this.maskTextClass, maskTextSelector: this.maskTextSelector, skipChild: true, @@ -512,7 +513,7 @@ export default class MutationBuffer { this.texts.push({ value: needMaskingText( - m.target, + m.target as Text, this.maskTextClass, this.maskTextSelector, ) && value From dac89a1530c4f2cac060e498d4f3e5f31199f4fa Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Wed, 8 Nov 2023 14:11:13 +0000 Subject: [PATCH 02/10] The split into needMaskingText/elementNeedsTextMasked was ugly and incorrect as a newly added text node needs to check all ancestors as well as just the parentNode. This problem suggested a new approach which facilitated the removal of the ugly `needsMask: false` initialization: - needsMask===true means that an ancestor has tested positively for masking, and so this node and all descendends should be masked - needsMask===false means that no ancestors have tested positively for masking, we should check each node encountered - needsMask===undefined means that we don't know whether ancestors are masked or not (e.g. after a mutation) and should look up the tree --- packages/rrweb-snapshot/src/snapshot.ts | 72 +++---- packages/rrweb/src/record/mutation.ts | 3 +- .../__snapshots__/integration.test.ts.snap | 203 ++++++++++++++++++ packages/rrweb/test/integration.test.ts | 27 +++ packages/rrweb/test/utils.ts | 1 + 5 files changed, 264 insertions(+), 42 deletions(-) diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index d525f3e684..0e7be75fa0 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -306,44 +306,35 @@ export function classMatchesRegex( return classMatchesRegex(node.parentNode, regex, checkAncestors); } -// used on newly added mutations export function needMaskingText( - node: Text, + node: Node, maskTextClass: string | RegExp, maskTextSelector: string | null, + checkAncestors: boolean, ): boolean { try { - const el = node.parentElement; + const el: HTMLElement | null = + node.nodeType === node.ELEMENT_NODE + ? (node as HTMLElement) + : node.parentElement; if (el === null) return false; if (typeof maskTextClass === 'string') { - if (el.closest(`.${maskTextClass}`)) return true; - } else { - // TODO: this doesn't check ancestors for mutations - if (classMatchesRegex(el, maskTextClass, true)) return true; - } - if (maskTextSelector) { - if (el.closest(maskTextSelector)) return true; - } - } catch (e) { - // - } - return false; -} - -// used upon first serialization -function elementNeedsTextMasked( - el: Element, - maskTextClass: string | RegExp, - maskTextSelector: string | null, -): boolean { - try { - if (typeof maskTextClass === 'string') { - if (el.classList.contains(maskTextClass)) return true; + if (checkAncestors) { + // we haven't already checked parents + if (el.closest(`.${maskTextClass}`)) return true; + } else { + if (el.classList.contains(maskTextClass)) return true; + } } else { - if (classMatchesRegex(el, maskTextClass, true)) return true; + if (classMatchesRegex(el, maskTextClass, checkAncestors)) return true; } if (maskTextSelector) { - if (el.matches(maskTextSelector)) return true; + if (checkAncestors) { + // we haven't already checked parents + if (el.closest(maskTextSelector)) return true; + } else { + if (el.matches(maskTextSelector)) return true; + } } } catch (e) { // @@ -942,7 +933,7 @@ export function serializeNodeWithId( inlineStylesheet: boolean; newlyAddedElement?: boolean; maskInputOptions?: MaskInputOptions; - needsMask: boolean; + needsMask?: boolean; maskTextFn: MaskTextFn | undefined; maskInputFn: MaskInputFn | undefined; slimDOMOptions: SlimDOMOptions; @@ -990,6 +981,18 @@ export function serializeNodeWithId( } = options; let { needsMask } = options; let { preserveWhiteSpace = true } = options; + + if (!needsMask) { + // perf: if needsMask = true, children won't also need to check + let checkAncestors = needsMask === undefined; // if false, we've already checked ancestors + needsMask = needMaskingText( + n as Element, + maskTextClass, + maskTextSelector, + checkAncestors, + ); + } + const _serializedNode = serializeNode(n, { doc, mirror, @@ -1047,16 +1050,6 @@ export function serializeNodeWithId( const shadowRoot = (n as HTMLElement).shadowRoot; if (shadowRoot && isNativeShadowDom(shadowRoot)) serializedNode.isShadowHost = true; - if (!needsMask) { - // we've already serialized this Element, but that's okay as masking - // is only relevant for child Text nodes. - // perf: if true, children won't also need to check - needsMask = elementNeedsTextMasked( - n as Element, - maskTextClass, - maskTextSelector, - ); - } } if ( (serializedNode.type === NodeType.Document || @@ -1327,7 +1320,6 @@ function snapshot( skipChild: false, inlineStylesheet, maskInputOptions, - needsMask: false, maskTextFn, maskInputFn, slimDOMOptions, diff --git a/packages/rrweb/src/record/mutation.ts b/packages/rrweb/src/record/mutation.ts index 5ce4f2dc34..7c209605d0 100644 --- a/packages/rrweb/src/record/mutation.ts +++ b/packages/rrweb/src/record/mutation.ts @@ -300,7 +300,6 @@ export default class MutationBuffer { mirror: this.mirror, blockClass: this.blockClass, blockSelector: this.blockSelector, - needsMask: false, maskTextClass: this.maskTextClass, maskTextSelector: this.maskTextSelector, skipChild: true, @@ -513,7 +512,7 @@ export default class MutationBuffer { this.texts.push({ value: needMaskingText( - m.target as Text, + m.target, this.maskTextClass, this.maskTextSelector, ) && value diff --git a/packages/rrweb/test/__snapshots__/integration.test.ts.snap b/packages/rrweb/test/__snapshots__/integration.test.ts.snap index e320254111..390eb616d5 100644 --- a/packages/rrweb/test/__snapshots__/integration.test.ts.snap +++ b/packages/rrweb/test/__snapshots__/integration.test.ts.snap @@ -797,6 +797,209 @@ exports[`record integration tests can mask character data mutations 1`] = ` ]" `; +exports[`record integration tests can mask character data mutations with regexp 1`] = ` +"[ + { + \\"type\\": 0, + \\"data\\": {} + }, + { + \\"type\\": 1, + \\"data\\": {} + }, + { + \\"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\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 2, + \\"tagName\\": \\"head\\", + \\"attributes\\": {}, + \\"childNodes\\": [], + \\"id\\": 4 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"body\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\", + \\"id\\": 6 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"p\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"mutation observer\\", + \\"id\\": 8 + } + ], + \\"id\\": 7 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\", + \\"id\\": 9 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"ul\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\", + \\"id\\": 11 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"li\\", + \\"attributes\\": {}, + \\"childNodes\\": [], + \\"id\\": 12 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\", + \\"id\\": 13 + } + ], + \\"id\\": 10 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\", + \\"id\\": 14 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"canvas\\", + \\"attributes\\": {}, + \\"childNodes\\": [], + \\"id\\": 15 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n\\\\n \\", + \\"id\\": 16 + }, + { + \\"type\\": 2, + \\"tagName\\": \\"script\\", + \\"attributes\\": {}, + \\"childNodes\\": [ + { + \\"type\\": 3, + \\"textContent\\": \\"SCRIPT_PLACEHOLDER\\", + \\"id\\": 18 + } + ], + \\"id\\": 17 + }, + { + \\"type\\": 3, + \\"textContent\\": \\"\\\\n \\\\n \\\\n\\", + \\"id\\": 19 + } + ], + \\"id\\": 5 + } + ], + \\"id\\": 3 + } + ], + \\"id\\": 1 + }, + \\"initialOffset\\": { + \\"left\\": 0, + \\"top\\": 0 + } + } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [], + \\"attributes\\": [ + { + \\"id\\": 7, + \\"attributes\\": { + \\"class\\": \\"custom-mask\\" + } + } + ], + \\"removes\\": [ + { + \\"parentId\\": 7, + \\"id\\": 8 + } + ], + \\"adds\\": [ + { + \\"parentId\\": 10, + \\"nextId\\": null, + \\"node\\": { + \\"type\\": 2, + \\"tagName\\": \\"li\\", + \\"attributes\\": { + \\"class\\": \\"custom-mask\\" + }, + \\"childNodes\\": [], + \\"id\\": 20 + } + }, + { + \\"parentId\\": 20, + \\"nextId\\": null, + \\"node\\": { + \\"type\\": 3, + \\"textContent\\": \\"*** **** ****\\", + \\"id\\": 21 + } + }, + { + \\"parentId\\": 7, + \\"nextId\\": null, + \\"node\\": { + \\"type\\": 3, + \\"textContent\\": \\"*******\\", + \\"id\\": 22 + } + } + ] + } + } +]" +`; + exports[`record integration tests can record attribute mutation 1`] = ` "[ { diff --git a/packages/rrweb/test/integration.test.ts b/packages/rrweb/test/integration.test.ts index c627be84cb..79069a7d5d 100644 --- a/packages/rrweb/test/integration.test.ts +++ b/packages/rrweb/test/integration.test.ts @@ -1213,6 +1213,33 @@ describe('record integration tests', function (this: ISuite) { assertSnapshot(snapshots); }); + it('can mask character data mutations with regexp', async () => { + const page: puppeteer.Page = await browser.newPage(); + await page.goto('about:blank'); + await page.setContent( + getHtml.call(this, 'mutation-observer.html', { + maskTextClass: /custom/, + }), + ); + + await page.evaluate(() => { + const li = document.createElement('li'); + const ul = document.querySelector('ul') as HTMLUListElement; + const p = document.querySelector('p') as HTMLParagraphElement; + [li, p].forEach((element) => { + element.className = 'custom-mask'; + }); + ul.appendChild(li); + li.innerText = 'new list item'; + p.innerText = 'mutated'; + }); + + const snapshots = (await page.evaluate( + 'window.snapshots', + )) as eventWithTime[]; + assertSnapshot(snapshots); + }); + it('should record after DOMContentLoaded event', async () => { const page: puppeteer.Page = await browser.newPage(); await page.goto('about:blank'); diff --git a/packages/rrweb/test/utils.ts b/packages/rrweb/test/utils.ts index 5a90f62031..6cd93281f9 100644 --- a/packages/rrweb/test/utils.ts +++ b/packages/rrweb/test/utils.ts @@ -693,6 +693,7 @@ export function generateRecordSnippet(options: recordOptions) { maskAllInputs: ${options.maskAllInputs}, maskInputOptions: ${JSON.stringify(options.maskAllInputs)}, userTriggeredOnInput: ${options.userTriggeredOnInput}, + maskTextClass: ${options.maskTextClass}, maskTextFn: ${options.maskTextFn}, maskInputFn: ${options.maskInputFn}, recordCanvas: ${options.recordCanvas}, From bae073996680432843a8a7a972abedf5412bffec Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Wed, 8 Nov 2023 15:13:38 +0000 Subject: [PATCH 03/10] Remove comments added before the explicit `checkAncestors` argument which is now self evident --- packages/rrweb-snapshot/src/snapshot.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index 0e7be75fa0..169901e743 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -320,7 +320,6 @@ export function needMaskingText( if (el === null) return false; if (typeof maskTextClass === 'string') { if (checkAncestors) { - // we haven't already checked parents if (el.closest(`.${maskTextClass}`)) return true; } else { if (el.classList.contains(maskTextClass)) return true; @@ -330,7 +329,6 @@ export function needMaskingText( } if (maskTextSelector) { if (checkAncestors) { - // we haven't already checked parents if (el.closest(maskTextSelector)) return true; } else { if (el.matches(maskTextSelector)) return true; From b5843a79c1c2ad41286592f03063f3778bb96b62 Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Wed, 8 Nov 2023 15:14:11 +0000 Subject: [PATCH 04/10] We didn't have any explicit characterData mutation tests - add one --- .../__snapshots__/integration.test.ts.snap | 30 +++++++++++++++++++ packages/rrweb/test/integration.test.ts | 12 ++++++++ 2 files changed, 42 insertions(+) diff --git a/packages/rrweb/test/__snapshots__/integration.test.ts.snap b/packages/rrweb/test/__snapshots__/integration.test.ts.snap index 390eb616d5..05e093633e 100644 --- a/packages/rrweb/test/__snapshots__/integration.test.ts.snap +++ b/packages/rrweb/test/__snapshots__/integration.test.ts.snap @@ -793,6 +793,21 @@ exports[`record integration tests can mask character data mutations 1`] = ` } ] } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [ + { + \\"id\\": 22, + \\"value\\": \\"****** *******\\" + } + ], + \\"attributes\\": [], + \\"removes\\": [], + \\"adds\\": [] + } } ]" `; @@ -996,6 +1011,21 @@ exports[`record integration tests can mask character data mutations with regexp } ] } + }, + { + \\"type\\": 3, + \\"data\\": { + \\"source\\": 0, + \\"texts\\": [ + { + \\"id\\": 22, + \\"value\\": \\"****** *******\\" + } + ], + \\"attributes\\": [], + \\"removes\\": [], + \\"adds\\": [] + } } ]" `; diff --git a/packages/rrweb/test/integration.test.ts b/packages/rrweb/test/integration.test.ts index 79069a7d5d..344f8e81b9 100644 --- a/packages/rrweb/test/integration.test.ts +++ b/packages/rrweb/test/integration.test.ts @@ -1207,6 +1207,12 @@ describe('record integration tests', function (this: ISuite) { p.innerText = 'mutated'; }); + await page.evaluate(() => { + // generate a characterData mutation; innerText doesn't do that + const p = document.querySelector('p') as HTMLParagraphElement; + (p.childNodes[0] as Text).insertData(0, 'doubly '); + }); + const snapshots = (await page.evaluate( 'window.snapshots', )) as eventWithTime[]; @@ -1234,6 +1240,12 @@ describe('record integration tests', function (this: ISuite) { p.innerText = 'mutated'; }); + await page.evaluate(() => { + // generate a characterData mutation; innerText doesn't do that + const p = document.querySelector('p') as HTMLParagraphElement; + (p.childNodes[0] as Text).insertData(0, 'doubly '); + }); + const snapshots = (await page.evaluate( 'window.snapshots', )) as eventWithTime[]; From aa046c45f3dd48f9d1758a7ba0c80a4f80a79eab Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Wed, 8 Nov 2023 15:19:12 +0000 Subject: [PATCH 05/10] I missed the checkAncestors:true argument on needMaskingText with the text mutation - modify the test to show where this would have failed --- packages/rrweb/src/record/mutation.ts | 1 + .../test/__snapshots__/integration.test.ts.snap | 14 +++++++++----- packages/rrweb/test/integration.test.ts | 6 +++--- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/rrweb/src/record/mutation.ts b/packages/rrweb/src/record/mutation.ts index 7c209605d0..2f6b6550ff 100644 --- a/packages/rrweb/src/record/mutation.ts +++ b/packages/rrweb/src/record/mutation.ts @@ -515,6 +515,7 @@ export default class MutationBuffer { m.target, this.maskTextClass, this.maskTextSelector, + true, // checkAncestors ) && value ? this.maskTextFn ? this.maskTextFn(value, closestElementOfNode(m.target)) diff --git a/packages/rrweb/test/__snapshots__/integration.test.ts.snap b/packages/rrweb/test/__snapshots__/integration.test.ts.snap index 05e093633e..e3fb552c7e 100644 --- a/packages/rrweb/test/__snapshots__/integration.test.ts.snap +++ b/packages/rrweb/test/__snapshots__/integration.test.ts.snap @@ -964,6 +964,12 @@ exports[`record integration tests can mask character data mutations with regexp \\"source\\": 0, \\"texts\\": [], \\"attributes\\": [ + { + \\"id\\": 10, + \\"attributes\\": { + \\"class\\": \\"custom-mask\\" + } + }, { \\"id\\": 7, \\"attributes\\": { @@ -984,9 +990,7 @@ exports[`record integration tests can mask character data mutations with regexp \\"node\\": { \\"type\\": 2, \\"tagName\\": \\"li\\", - \\"attributes\\": { - \\"class\\": \\"custom-mask\\" - }, + \\"attributes\\": {}, \\"childNodes\\": [], \\"id\\": 20 } @@ -1018,8 +1022,8 @@ exports[`record integration tests can mask character data mutations with regexp \\"source\\": 0, \\"texts\\": [ { - \\"id\\": 22, - \\"value\\": \\"****** *******\\" + \\"id\\": 21, + \\"value\\": \\"********** ****** ** ****** *** **** ****\\" } ], \\"attributes\\": [], diff --git a/packages/rrweb/test/integration.test.ts b/packages/rrweb/test/integration.test.ts index 344f8e81b9..0d4c871854 100644 --- a/packages/rrweb/test/integration.test.ts +++ b/packages/rrweb/test/integration.test.ts @@ -1232,7 +1232,7 @@ describe('record integration tests', function (this: ISuite) { const li = document.createElement('li'); const ul = document.querySelector('ul') as HTMLUListElement; const p = document.querySelector('p') as HTMLParagraphElement; - [li, p].forEach((element) => { + [ul, p].forEach((element) => { element.className = 'custom-mask'; }); ul.appendChild(li); @@ -1242,8 +1242,8 @@ describe('record integration tests', function (this: ISuite) { await page.evaluate(() => { // generate a characterData mutation; innerText doesn't do that - const p = document.querySelector('p') as HTMLParagraphElement; - (p.childNodes[0] as Text).insertData(0, 'doubly '); + const li = document.querySelector('li:not(:empty)') as HTMLLIElement; + (li.childNodes[0] as Text).insertData(0, 'descendent should be masked '); }); const snapshots = (await page.evaluate( From 5ce0f4e019b42dd083da299ed1d5bdc9f8f5ec11 Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Wed, 8 Nov 2023 15:21:31 +0000 Subject: [PATCH 06/10] Create thin-vans-applaud.md --- .changeset/thin-vans-applaud.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/thin-vans-applaud.md diff --git a/.changeset/thin-vans-applaud.md b/.changeset/thin-vans-applaud.md new file mode 100644 index 0000000000..e5d8d32a63 --- /dev/null +++ b/.changeset/thin-vans-applaud.md @@ -0,0 +1,6 @@ +--- +'rrweb-snapshot': patch +'rrweb': patch +--- + +Snapshot performance when masking text: Avoid the repeated calls to `closest` when recursing through the DOM From d19e922a16fa9f1d6d02e0866a40ecb0342b550b Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Mon, 13 Nov 2023 11:53:36 +0000 Subject: [PATCH 07/10] Fix eslint --- packages/rrweb-snapshot/src/snapshot.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index 169901e743..0b34135ea5 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -982,7 +982,7 @@ export function serializeNodeWithId( if (!needsMask) { // perf: if needsMask = true, children won't also need to check - let checkAncestors = needsMask === undefined; // if false, we've already checked ancestors + const checkAncestors = needsMask === undefined; // if false, we've already checked ancestors needsMask = needMaskingText( n as Element, maskTextClass, From 1b37e33afdf38e3c37afc0f09181d99783eb04e2 Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Wed, 15 Nov 2023 13:35:42 +0000 Subject: [PATCH 08/10] Avoid calls to `el.matches` when on a leaf node, e.g. a `
` --- packages/rrweb-snapshot/src/snapshot.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index 0b34135ea5..003d2f3e9b 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -980,7 +980,9 @@ export function serializeNodeWithId( let { needsMask } = options; let { preserveWhiteSpace = true } = options; - if (!needsMask) { + if (!needsMask && + n.childNodes // we can avoid the check on leaf elements, as masking is applied to child text nodes only + ) { // perf: if needsMask = true, children won't also need to check const checkAncestors = needsMask === undefined; // if false, we've already checked ancestors needsMask = needMaskingText( From 632075179f49d88026973f543278911c3e6d7d3f Mon Sep 17 00:00:00 2001 From: eoghanmurray Date: Wed, 15 Nov 2023 13:37:50 +0000 Subject: [PATCH 09/10] Apply formatting changes --- packages/rrweb-snapshot/src/snapshot.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index 003d2f3e9b..701c268c4b 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -980,9 +980,10 @@ export function serializeNodeWithId( let { needsMask } = options; let { preserveWhiteSpace = true } = options; - if (!needsMask && - n.childNodes // we can avoid the check on leaf elements, as masking is applied to child text nodes only - ) { + if ( + !needsMask && + n.childNodes // we can avoid the check on leaf elements, as masking is applied to child text nodes only + ) { // perf: if needsMask = true, children won't also need to check const checkAncestors = needsMask === undefined; // if false, we've already checked ancestors needsMask = needMaskingText( From 36618ab26233b7c3611b3d6b0c16ffd483a4de1f Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Wed, 15 Nov 2023 13:40:41 +0000 Subject: [PATCH 10/10] Fixup eslint --- packages/rrweb-snapshot/src/snapshot.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts index 701c268c4b..6d2f2ed2e8 100644 --- a/packages/rrweb-snapshot/src/snapshot.ts +++ b/packages/rrweb-snapshot/src/snapshot.ts @@ -431,7 +431,7 @@ function serializeNode( mirror: Mirror; blockClass: string | RegExp; blockSelector: string | null; - needsMask: boolean; + needsMask: boolean | undefined; inlineStylesheet: boolean; maskInputOptions: MaskInputOptions; maskTextFn: MaskTextFn | undefined; @@ -533,7 +533,7 @@ function getRootId(doc: Document, mirror: Mirror): number | undefined { function serializeTextNode( n: Text, options: { - needsMask: boolean; + needsMask: boolean | undefined; maskTextFn: MaskTextFn | undefined; rootId: number | undefined; },