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.
This commit is contained in:
6
.changeset/skip-mask-check-on-leaf-elements.md
Normal file
6
.changeset/skip-mask-check-on-leaf-elements.md
Normal file
@@ -0,0 +1,6 @@
|
|||||||
|
---
|
||||||
|
"rrweb-snapshot": patch
|
||||||
|
"rrweb": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
optimisation: skip mask check on leaf elements
|
||||||
@@ -326,12 +326,21 @@ export function needMaskingText(
|
|||||||
maskTextSelector: string | null,
|
maskTextSelector: string | null,
|
||||||
checkAncestors: boolean,
|
checkAncestors: boolean,
|
||||||
): boolean {
|
): boolean {
|
||||||
|
let el: Element;
|
||||||
|
if (isElement(node)) {
|
||||||
|
el = node;
|
||||||
|
if (!el.childNodes.length) {
|
||||||
|
// optimisation: we can avoid any of the below checks on leaf elements
|
||||||
|
// as masking is applied to child text nodes only
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
} else if (node.parentElement === null) {
|
||||||
|
// should warn? maybe a text node isn't attached to a parent node yet?
|
||||||
|
return false;
|
||||||
|
} else {
|
||||||
|
el = node.parentElement;
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
const el: HTMLElement | null =
|
|
||||||
node.nodeType === node.ELEMENT_NODE
|
|
||||||
? (node as HTMLElement)
|
|
||||||
: node.parentElement;
|
|
||||||
if (el === null) return false;
|
|
||||||
if (typeof maskTextClass === 'string') {
|
if (typeof maskTextClass === 'string') {
|
||||||
if (checkAncestors) {
|
if (checkAncestors) {
|
||||||
if (el.closest(`.${maskTextClass}`)) return true;
|
if (el.closest(`.${maskTextClass}`)) return true;
|
||||||
@@ -440,7 +449,7 @@ function serializeNode(
|
|||||||
mirror: Mirror;
|
mirror: Mirror;
|
||||||
blockClass: string | RegExp;
|
blockClass: string | RegExp;
|
||||||
blockSelector: string | null;
|
blockSelector: string | null;
|
||||||
needsMask: boolean | undefined;
|
needsMask: boolean;
|
||||||
inlineStylesheet: boolean;
|
inlineStylesheet: boolean;
|
||||||
maskInputOptions: MaskInputOptions;
|
maskInputOptions: MaskInputOptions;
|
||||||
maskTextFn: MaskTextFn | undefined;
|
maskTextFn: MaskTextFn | undefined;
|
||||||
@@ -544,7 +553,7 @@ function serializeTextNode(
|
|||||||
n: Text,
|
n: Text,
|
||||||
options: {
|
options: {
|
||||||
doc: Document;
|
doc: Document;
|
||||||
needsMask: boolean | undefined;
|
needsMask: boolean;
|
||||||
maskTextFn: MaskTextFn | undefined;
|
maskTextFn: MaskTextFn | undefined;
|
||||||
rootId: number | undefined;
|
rootId: number | undefined;
|
||||||
},
|
},
|
||||||
@@ -1006,10 +1015,7 @@ export function serializeNodeWithId(
|
|||||||
let { needsMask } = options;
|
let { needsMask } = options;
|
||||||
let { preserveWhiteSpace = true } = options;
|
let { preserveWhiteSpace = true } = options;
|
||||||
|
|
||||||
if (
|
if (!needsMask) {
|
||||||
!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
|
// perf: if needsMask = true, children won't also need to check
|
||||||
const checkAncestors = needsMask === undefined; // if false, we've already checked ancestors
|
const checkAncestors = needsMask === undefined; // if false, we've already checked ancestors
|
||||||
needsMask = needMaskingText(
|
needsMask = needMaskingText(
|
||||||
|
|||||||
Reference in New Issue
Block a user