From 9b6b1fd49cc9cd60eb01aa74971d775c74fb8206 Mon Sep 17 00:00:00 2001
From: Eoghan Murray <eoghan@getthere.ie>
Date: Mon, 17 Jun 2024 14:37:23 +0100
Subject: [PATCH] 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.
---
 .../skip-mask-check-on-leaf-elements.md       |  6 ++++
 packages/rrweb-snapshot/src/snapshot.ts       | 29 +++++++++++--------
 2 files changed, 23 insertions(+), 12 deletions(-)
 create mode 100644 .changeset/skip-mask-check-on-leaf-elements.md

diff --git a/.changeset/skip-mask-check-on-leaf-elements.md b/.changeset/skip-mask-check-on-leaf-elements.md
new file mode 100644
index 0000000000..b54de0a4ba
--- /dev/null
+++ b/.changeset/skip-mask-check-on-leaf-elements.md
@@ -0,0 +1,6 @@
+---
+"rrweb-snapshot": patch
+"rrweb": patch
+---
+
+optimisation: skip mask check on leaf elements
diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts
index 6853415160..4961262c7e 100644
--- a/packages/rrweb-snapshot/src/snapshot.ts
+++ b/packages/rrweb-snapshot/src/snapshot.ts
@@ -328,13 +328,21 @@ export function needMaskingText(
   maskTextSelector: string | null,
   checkAncestors: 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 {
-    const el: HTMLElement | null =
-      node.nodeType === node.ELEMENT_NODE
-        ? (node as HTMLElement)
-        : node.parentElement;
-    if (el === null) return false;
-    if (maskTextSelector === '*') return true;
     if (typeof maskTextClass === 'string') {
       if (checkAncestors) {
         if (el.closest(`.${maskTextClass}`)) return true;
@@ -443,7 +451,7 @@ function serializeNode(
     mirror: Mirror;
     blockClass: string | RegExp;
     blockSelector: string | null;
-    needsMask: boolean | undefined;
+    needsMask: boolean;
     inlineStylesheet: boolean;
     maskInputOptions: MaskInputOptions;
     maskTextFn: MaskTextFn | undefined;
@@ -550,7 +558,7 @@ function serializeTextNode(
   n: Text,
   options: {
     doc: Document;
-    needsMask: boolean | undefined;
+    needsMask: boolean;
     maskTextFn: MaskTextFn | undefined;
     maskInputOptions: MaskInputOptions;
     maskInputFn: MaskInputFn | undefined;
@@ -1026,10 +1034,7 @@ 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) {
     // perf: if needsMask = true, children won't also need to check
     const checkAncestors = needsMask === undefined; // if false, we've already checked ancestors
     needsMask = needMaskingText(