PR fix: Add test for null attribute (#66)

mydea · web-flow · commit f40f0cf61984 · 2023-02-27T10:19:11.000+01:00
This adds on top of #65 and adds a test for this, as well as streamlining this check in the `transformAttribute` method.
diff --git a/packages/rrweb-snapshot/src/rebuild.ts b/packages/rrweb-snapshot/src/rebuild.ts
@@ -154,7 +154,7 @@ function buildNode(
           continue;
         }
         value =
-          typeof value === 'boolean' || typeof value === 'number' ? '' : value;
+          typeof value === 'boolean' || typeof value === 'number' || value === null ? '' : value;
         // attribute names start with rr_ are internal attributes added by rrweb
         if (!name.startsWith('rr_')) {
           const isTextarea = tagName === 'textarea' && name === 'value';
diff --git a/packages/rrweb-snapshot/src/snapshot.ts b/packages/rrweb-snapshot/src/snapshot.ts
@@ -238,36 +238,39 @@ export function transformAttribute(
   doc: Document,
   tagName: string,
   name: string,
-  value: string,
+  value: string | null,
   maskAllText: boolean,
   maskTextFn: MaskTextFn | undefined,
-): string {
+): string | null {
+  if (!value) {
+    return value;
+  }
+
   // relative path in attribute
-  if (name === 'src' || (name === 'href' && value)) {
+  if (name === 'src' || name === 'href') {
     return absoluteToDoc(doc, value);
-  } else if (name === 'xlink:href' && value && value[0] !== '#') {
+  } else if (name === 'xlink:href' && value[0] !== '#') {
     // xlink:href starts with # is an id pointer
     return absoluteToDoc(doc, value);
   } else if (
     name === 'background' &&
-    value &&
     (tagName === 'table' || tagName === 'td' || tagName === 'th')
   ) {
     return absoluteToDoc(doc, value);
-  } else if (name === 'srcset' && value) {
+  } else if (name === 'srcset') {
     return getAbsoluteSrcsetString(doc, value);
-  } else if (name === 'style' && value) {
+  } else if (name === 'style') {
     return absoluteToStylesheet(value, getHref());
-  } else if (tagName === 'object' && name === 'data' && value) {
+  } else if (tagName === 'object' && name === 'data') {
     return absoluteToDoc(doc, value);
   } else if (
     maskAllText &&
     ['placeholder', 'title', 'aria-label'].indexOf(name) > -1
   ) {
     return maskTextFn ? maskTextFn(value) : defaultMaskFn(value);
-  } else {
-    return value;
   }
+
+  return value;
 }
 
 export function _isBlockedElement(
@@ -770,8 +773,8 @@ function serializeNode(
   }
 }
 
-function lowerIfExists(maybeAttr: string | number | boolean): string {
-  if (maybeAttr === undefined) {
+function lowerIfExists(maybeAttr: string | number | boolean | null | undefined): string {
+  if (maybeAttr === undefined || maybeAttr === null) {
     return '';
   } else {
     return (maybeAttr as string).toLowerCase();
diff --git a/packages/rrweb-snapshot/src/types.ts b/packages/rrweb-snapshot/src/types.ts
@@ -21,7 +21,7 @@ export type documentTypeNode = {
 };
 
 export type attributes = {
-  [key: string]: string | number | boolean;
+  [key: string]: string | number | boolean | null;
 };
 export type elementNode = {
   type: NodeType.Element;
diff --git a/packages/rrweb-snapshot/typings/snapshot.d.ts b/packages/rrweb-snapshot/typings/snapshot.d.ts
@@ -2,7 +2,7 @@ import { serializedNodeWithId, INode, idNodeMap, MaskInputOptions, SlimDOMOption
 export declare const IGNORED_NODE = -2;
 export declare function absoluteToStylesheet(cssText: string | null, href: string): string;
 export declare function absoluteToDoc(doc: Document, attributeValue: string): string;
-export declare function transformAttribute(doc: Document, tagName: string, name: string, value: string, maskAllText: boolean, maskTextFn: MaskTextFn | undefined): string;
+export declare function transformAttribute(doc: Document, tagName: string, name: string, value: string | null, maskAllText: boolean, maskTextFn: MaskTextFn | undefined): string | null;
 export declare function _isBlockedElement(element: HTMLElement, blockClass: string | RegExp, blockSelector: string | null, unblockSelector: string | null): boolean;
 export declare function needMaskingText(node: Node | null, maskTextClass: string | RegExp, maskTextSelector: string | null, unmaskTextSelector: string | null, maskAllText: boolean): boolean;
 export declare function serializeNodeWithId(n: Node | INode, options: {
diff --git a/packages/rrweb-snapshot/typings/types.d.ts b/packages/rrweb-snapshot/typings/types.d.ts
@@ -18,7 +18,7 @@ export type documentTypeNode = {
     systemId: string;
 };
 export type attributes = {
-    [key: string]: string | number | boolean;
+    [key: string]: string | number | boolean | null;
 };
 export type elementNode = {
     type: NodeType.Element;
diff --git a/packages/rrweb/test/__snapshots__/integration.test.ts.snap b/packages/rrweb/test/__snapshots__/integration.test.ts.snap
@@ -3684,6 +3684,205 @@ exports[`record integration tests can use maskInputOptions to configure which ty
 ]"
 `;
 
+exports[`record integration tests handles null attribute values 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": "******** ********",
+                        "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": 20,
+          "attributes": {
+            "aria-label": "*****",
+            "id": "test-li"
+          }
+        }
+      ],
+      "removes": [],
+      "adds": [
+        {
+          "parentId": 10,
+          "nextId": null,
+          "node": {
+            "type": 2,
+            "tagName": "li",
+            "attributes": {
+              "aria-label": "*****",
+              "id": "test-li"
+            },
+            "childNodes": [],
+            "id": 20
+          }
+        }
+      ]
+    }
+  },
+  {
+    "type": 3,
+    "data": {
+      "source": 0,
+      "texts": [],
+      "attributes": [
+        {
+          "id": 20,
+          "attributes": {
+            "aria-label": null
+          }
+        }
+      ],
+      "removes": [],
+      "adds": []
+    }
+  }
+]"
+`;
+
 exports[`record integration tests should mask all text (except unmaskTextSelector), using maskAllText  1`] = `
 "[
   {
diff --git a/packages/rrweb/test/integration.test.ts b/packages/rrweb/test/integration.test.ts
@@ -130,6 +130,37 @@ describe('record integration tests', function (this: ISuite) {
     assertSnapshot(snapshots);
   });
 
+  it('handles null attribute values', async () => {
+    const page: puppeteer.Page = await browser.newPage();
+    await page.goto('about:blank');
+    await page.setContent(
+      getHtml.call(this, 'mutation-observer.html', {
+        maskAllInputs: true,
+        maskAllText: true,
+      }),
+    );
+
+    await page.evaluate(() => {
+      const li = document.createElement('li');
+      const ul = document.querySelector('ul') as HTMLUListElement;
+      ul.appendChild(li);
+
+      li.setAttribute('aria-label', 'label');
+      li.setAttribute('id', 'test-li');
+    });
+
+    await new Promise((resolve) => setTimeout(resolve, 100));
+
+    await page.evaluate(() => {
+      const li = document.querySelector('#test-li') as HTMLLIElement;
+      // This triggers the mutation observer with a `null` attribute value
+      li.removeAttribute('aria-label');
+    });
+
+    const snapshots = await page.evaluate('window.snapshots');
+    assertSnapshot(snapshots);
+  });
+
   it('can record character data muatations', async () => {
     const page: puppeteer.Page = await browser.newPage();
     await page.goto('about:blank');

Original file line number	Diff line number	Diff line change
`@@ -154,7 +154,7 @@ function buildNode(`
`154`	`154`	`continue;`
`155`	`155`	`}`
`156`	`156`	`value =`
`157`		`- typeof value === 'boolean' \|\| typeof value === 'number' ? '' : value;`
	`157`	`+ typeof value === 'boolean' \|\| typeof value === 'number' \|\| value === null ? '' : value;`
`158`	`158`	`// attribute names start with rr_ are internal attributes added by rrweb`
`159`	`159`	`if (!name.startsWith('rr_')) {`
`160`	`160`	`const isTextarea = tagName === 'textarea' && name === 'value';`