Skip to content

Commit d08624c

Browse files
authored
Skip mask check on leaf elements (#1512)
* Minor fixup for #1349; the 'we can avoid the check on leaf elements' optimisation wasn't being applied as `n.childNodes` was always truthy even when there were no childNodes. Changing it to `n.childNodes.length` directly there (see #1402) actually caused a bug as during a mutation, we serialize the text node directly, and need to jump to the parentElement to do the check. This is why I've reimplemented this optimisation inside `needMaskingText` where we are already had an `isElement` test Thanks to @Paulhejia (https://github.com/Paulhejia/rrweb/) for spotting that `Boolean(n.childNodes)` is aways true.
1 parent cfd686d commit d08624c

File tree

2 files changed

+23
-11
lines changed

2 files changed

+23
-11
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"rrweb-snapshot": patch
3+
"rrweb": patch
4+
---
5+
6+
optimisation: skip mask check on leaf elements

packages/rrweb-snapshot/src/snapshot.ts

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -326,12 +326,21 @@ export function needMaskingText(
326326
maskTextSelector: string | null,
327327
checkAncestors: boolean,
328328
): boolean {
329+
let el: Element;
330+
if (isElement(node)) {
331+
el = node;
332+
if (!el.childNodes.length) {
333+
// optimisation: we can avoid any of the below checks on leaf elements
334+
// as masking is applied to child text nodes only
335+
return false;
336+
}
337+
} else if (node.parentElement === null) {
338+
// should warn? maybe a text node isn't attached to a parent node yet?
339+
return false;
340+
} else {
341+
el = node.parentElement;
342+
}
329343
try {
330-
const el: HTMLElement | null =
331-
node.nodeType === node.ELEMENT_NODE
332-
? (node as HTMLElement)
333-
: node.parentElement;
334-
if (el === null) return false;
335344
if (typeof maskTextClass === 'string') {
336345
if (checkAncestors) {
337346
if (el.closest(`.${maskTextClass}`)) return true;
@@ -440,7 +449,7 @@ function serializeNode(
440449
mirror: Mirror;
441450
blockClass: string | RegExp;
442451
blockSelector: string | null;
443-
needsMask: boolean | undefined;
452+
needsMask: boolean;
444453
inlineStylesheet: boolean;
445454
maskInputOptions: MaskInputOptions;
446455
maskTextFn: MaskTextFn | undefined;
@@ -544,7 +553,7 @@ function serializeTextNode(
544553
n: Text,
545554
options: {
546555
doc: Document;
547-
needsMask: boolean | undefined;
556+
needsMask: boolean;
548557
maskTextFn: MaskTextFn | undefined;
549558
rootId: number | undefined;
550559
},
@@ -1006,10 +1015,7 @@ export function serializeNodeWithId(
10061015
let { needsMask } = options;
10071016
let { preserveWhiteSpace = true } = options;
10081017

1009-
if (
1010-
!needsMask &&
1011-
n.childNodes // we can avoid the check on leaf elements, as masking is applied to child text nodes only
1012-
) {
1018+
if (!needsMask) {
10131019
// perf: if needsMask = true, children won't also need to check
10141020
const checkAncestors = needsMask === undefined; // if false, we've already checked ancestors
10151021
needsMask = needMaskingText(

0 commit comments

Comments
 (0)