🎨 [PANA-5053] Separate DOM and virtual attribute serialization (#3998)

Author: sethfowler-datadog

packages/rum/src/domain/record/serialization/serializationUtils.ts

@@ -90,6 +90,14 @@ export function getValidTagName(tagName: string): string {
   return processedTagName
 }
 
+/**
+ * Returns the tag name of the given element, normalized to ensure a consistent lowercase
+ * representation regardless of whether the element is HTML, XHTML, or SVG.
+ */
+export function normalizedTagName(element: Element): string {
+  return element.tagName.toLowerCase()
+}
+
 export function censoredImageForSize(width: number, height: number) {
   return `data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='${width}' height='${height}' style='background-color:silver'%3E%3C/svg%3E`
 }

packages/rum/src/domain/record/serialization/serializeAttribute.ts

@@ -17,11 +17,12 @@ export function serializeAttribute(
   nodePrivacyLevel: NodePrivacyLevel,
   attributeName: string,
   configuration: RumConfiguration
-): string | number | boolean | null {
+): string | null {
   if (nodePrivacyLevel === NodePrivacyLevel.HIDDEN) {
     // dup condition for direct access case
     return null
   }
+
   const attributeValue = element.getAttribute(attributeName)
   const tagName = element.tagName
   if (shouldMaskAttribute(tagName, attributeName, attributeValue, nodePrivacyLevel, configuration)) {
@@ -46,7 +47,7 @@ export function serializeAttribute(
     return CENSORED_STRING_MARK
   }
 
-  if (!attributeValue || typeof attributeValue !== 'string') {
+  if (!attributeValue) {
     return attributeValue
   }
 

packages/rum/src/domain/record/serialization/serializeAttributes.spec.ts

@@ -11,7 +11,7 @@ import {
 import { appendElement } from '@datadog/browser-rum-core/test'
 import { createSerializationTransactionForTesting } from '../test/serialization.specHelper'
 import type { ScrollPositions } from '../elementsScrollPositions'
-import { getCssRulesString, serializeAttributes } from './serializeAttributes'
+import { getCssRulesString, serializeDOMAttributes, serializeVirtualAttributes } from './serializeAttributes'
 import { SerializationKind, type SerializationTransaction } from './serializationTransaction'
 import type { VirtualAttributes } from './serialization.types'
 import type { SerializationMetric, SerializationStats } from './serializationStats'
@@ -28,21 +28,13 @@ const PRIVACY_LEVELS = Object.keys({
   [NodePrivacyLevel.MASK_USER_INPUT]: true,
 } satisfies Record<NodePrivacyLevel, true>) as NodePrivacyLevel[]
 
-describe('serializeAttributes for DOM attributes', () => {
+describe('serializeDOMAttributes', () => {
   let transaction: SerializationTransaction
 
   beforeEach(() => {
     transaction = createSerializationTransactionForTesting()
   })
 
-  function serializeDOMAttributes(
-    element: Element,
-    nodePrivacyLevel: NodePrivacyLevel,
-    transaction: SerializationTransaction
-  ): Record<string, string | number | boolean | undefined> {
-    return serializeAttributes(element, nodePrivacyLevel, transaction)
-  }
-
   it('serializes attribute values', () => {
     interface TestCase {
       // A snippet of HTML containing an attribute we should serialize.
@@ -260,7 +252,8 @@ describe('serializeAttributes for DOM attributes', () => {
       element.remove()
 
       for (const privacyLevel of PRIVACY_LEVELS) {
-        const actual = serializeDOMAttributes(element, privacyLevel, transaction)[attribute.name]
+        const attributes = serializeDOMAttributes(element, privacyLevel, transaction)
+        const actual = attributes[attribute.name] as boolean | string | undefined
         const expected = expectedValueForPrivacyLevel(testCase, element, attribute, privacyLevel)
         expect(actual).withContext(`${testCase.html} for ${privacyLevel}`).toEqual(expected)
       }
@@ -342,9 +335,22 @@ describe('serializeAttributes for DOM attributes', () => {
       }
     }
   })
+
+  it('normalizes tag names', () => {
+    // Create an <option> element in the SVG namespace. This makes it an XML element;
+    // among other things, this results in a lowercase Element#tagName.
+    const element = document.createElementNS('http://www.w3.org/2000/svg', 'option')
+    expect(element.tagName).toBe('option')
+
+    // Check that serializeDOMAttributes() still executes <option>-specific serialization
+    // behavior; it shouldn't behave differently because Element#tagName is lowercase.
+    ;(element as any).selected = true
+    const attributes = serializeDOMAttributes(element, NodePrivacyLevel.ALLOW, transaction)
+    expect(attributes['selected']).toBe(true)
+  })
 })
 
-describe('serializeAttributes for virtual attributes', () => {
+describe('serializeVirtualAttributes', () => {
   let stats: SerializationStats
   let transaction: SerializationTransaction
 
@@ -353,36 +359,6 @@ describe('serializeAttributes for virtual attributes', () => {
     transaction = createSerializationTransactionForTesting({ stats })
   })
 
-  function serializeVirtualAttributes(
-    element: Element,
-    nodePrivacyLevel: NodePrivacyLevel,
-    transaction: SerializationTransaction
-  ): VirtualAttributes {
-    const attributes = serializeAttributes(element, nodePrivacyLevel, transaction)
-
-    const virtualAttributes: VirtualAttributes = {}
-    if ('_cssText' in attributes) {
-      virtualAttributes._cssText = attributes._cssText as string
-    }
-    if ('rr_mediaState' in attributes) {
-      virtualAttributes.rr_mediaState = attributes.rr_mediaState as 'paused' | 'played'
-    }
-    if ('rr_scrollLeft' in attributes) {
-      virtualAttributes.rr_scrollLeft = attributes.rr_scrollLeft as number
-    }
-    if ('rr_scrollTop' in attributes) {
-      virtualAttributes.rr_scrollTop = attributes.rr_scrollTop as number
-    }
-    if ('rr_width' in attributes) {
-      virtualAttributes.rr_width = attributes.rr_width as string
-    }
-    if ('rr_height' in attributes) {
-      virtualAttributes.rr_height = attributes.rr_height as string
-    }
-
-    return virtualAttributes
-  }
-
   function expectVirtualAttributes(
     element: Element,
     expectedWhenNotHidden: VirtualAttributes,
@@ -534,6 +510,19 @@ describe('serializeAttributes for virtual attributes', () => {
       expectVirtualAttributes(div, {}, checkElementScrollPositions)
     })
   })
+
+  it('normalizes tag names', () => {
+    // Create an <audio> element in the SVG namespace. This makes it an XML element;
+    // among other things, this results in a lowercase Element#tagName.
+    const element = document.createElementNS('http://www.w3.org/2000/svg', 'audio')
+    expect(element.tagName).toBe('audio')
+
+    // Check that serializeVirtualAttributes() still executes <audio>-specific serialization
+    // behavior; it shouldn't behave differently because Element#tagName is lowercase.
+    ;(element as any).paused = true
+    const attributes = serializeVirtualAttributes(element, NodePrivacyLevel.ALLOW, transaction)
+    expect(attributes.rr_mediaState).toBe('paused')
+  })
 })
 
 describe('getCssRulesString', () => {

packages/rum/src/domain/record/serialization/serializeAttributes.ts

@@ -1,28 +1,40 @@
 import { NodePrivacyLevel, shouldMaskNode } from '@datadog/browser-rum-core'
 import { isSafari } from '@datadog/browser-core'
-import { getElementInputValue, switchToAbsoluteUrl, getValidTagName } from './serializationUtils'
+import { getElementInputValue, normalizedTagName, switchToAbsoluteUrl } from './serializationUtils'
 import { serializeAttribute } from './serializeAttribute'
 import type { SerializationTransaction } from './serializationTransaction'
 import { SerializationKind } from './serializationTransaction'
+import type { VirtualAttributes } from './serialization.types'
 
 export function serializeAttributes(
   element: Element,
   nodePrivacyLevel: NodePrivacyLevel,
   transaction: SerializationTransaction
-): Record<string, string | number | boolean> {
+): Record<string, boolean | number | string> {
+  return {
+    ...serializeDOMAttributes(element, nodePrivacyLevel, transaction),
+    ...serializeVirtualAttributes(element, nodePrivacyLevel, transaction),
+  }
+}
+
+export function serializeDOMAttributes(
+  element: Element,
+  nodePrivacyLevel: NodePrivacyLevel,
+  transaction: SerializationTransaction
+): Record<string, boolean | string> {
   if (nodePrivacyLevel === NodePrivacyLevel.HIDDEN) {
     return {}
   }
-  const safeAttrs: Record<string, string | number | boolean> = {}
-  const tagName = getValidTagName(element.tagName)
-  const doc = element.ownerDocument
+
+  const attrs: Record<string, string | boolean> = {}
+  const tagName = normalizedTagName(element)
 
   for (let i = 0; i < element.attributes.length; i += 1) {
     const attribute = element.attributes.item(i)!
     const attributeName = attribute.name
     const attributeValue = serializeAttribute(element, nodePrivacyLevel, attributeName, transaction.scope.configuration)
     if (attributeValue !== null) {
-      safeAttrs[attributeName] = attributeValue
+      attrs[attributeName] = attributeValue
     }
   }
 
@@ -32,7 +44,7 @@ export function serializeAttributes(
   ) {
     const formValue = getElementInputValue(element, nodePrivacyLevel)
     if (formValue !== undefined) {
-      safeAttrs.value = formValue
+      attrs.value = formValue
     }
   }
 
@@ -43,17 +55,50 @@ export function serializeAttributes(
     // For privacy=`MASK`, all the values would be the same, so skip.
     const optionElement = element as HTMLOptionElement
     if (optionElement.selected) {
-      safeAttrs.selected = optionElement.selected
+      attrs.selected = optionElement.selected
     }
   }
 
+  /**
+   * Forms: input[type=checkbox,radio]
+   * The `checked` property for <input> is a little bit special:
+   * 1. el.checked is a setter that returns if truthy.
+   * 2. getAttribute returns the string value
+   * getAttribute('checked') does not sync with `Element.checked`, so use JS property
+   * NOTE: `checked` property exists on `HTMLInputElement`. For serializer assumptions, we check for type=radio|check.
+   */
+  const inputElement = element as HTMLInputElement
+  if (tagName === 'input' && (inputElement.type === 'radio' || inputElement.type === 'checkbox')) {
+    if (nodePrivacyLevel === NodePrivacyLevel.ALLOW) {
+      attrs.checked = !!inputElement.checked
+    } else if (shouldMaskNode(inputElement, nodePrivacyLevel)) {
+      delete attrs.checked
+    }
+  }
+
+  return attrs
+}
+
+export function serializeVirtualAttributes(
+  element: Element,
+  nodePrivacyLevel: NodePrivacyLevel,
+  transaction: SerializationTransaction
+): VirtualAttributes {
+  if (nodePrivacyLevel === NodePrivacyLevel.HIDDEN) {
+    return {}
+  }
+
+  const attrs: VirtualAttributes = {}
+  const doc = element.ownerDocument
+  const tagName = normalizedTagName(element)
+
   // remote css
   if (tagName === 'link') {
     const stylesheet = Array.from(doc.styleSheets).find((s) => s.href === (element as HTMLLinkElement).href)
     const cssText = getCssRulesString(stylesheet)
     if (cssText && stylesheet) {
       transaction.addMetric('cssText', cssText.length)
-      safeAttrs._cssText = cssText
+      attrs._cssText = cssText
     }
   }
 
@@ -62,24 +107,7 @@ export function serializeAttributes(
     const cssText = getCssRulesString((element as HTMLStyleElement).sheet)
     if (cssText) {
       transaction.addMetric('cssText', cssText.length)
-      safeAttrs._cssText = cssText
-    }
-  }
-
-  /**
-   * Forms: input[type=checkbox,radio]
-   * The `checked` property for <input> is a little bit special:
-   * 1. el.checked is a setter that returns if truthy.
-   * 2. getAttribute returns the string value
-   * getAttribute('checked') does not sync with `Element.checked`, so use JS property
-   * NOTE: `checked` property exists on `HTMLInputElement`. For serializer assumptions, we check for type=radio|check.
-   */
-  const inputElement = element as HTMLInputElement
-  if (tagName === 'input' && (inputElement.type === 'radio' || inputElement.type === 'checkbox')) {
-    if (nodePrivacyLevel === NodePrivacyLevel.ALLOW) {
-      safeAttrs.checked = !!inputElement.checked
-    } else if (shouldMaskNode(inputElement, nodePrivacyLevel)) {
-      delete safeAttrs.checked
+      attrs._cssText = cssText
     }
   }
 
@@ -88,7 +116,7 @@ export function serializeAttributes(
    */
   if (tagName === 'audio' || tagName === 'video') {
     const mediaElement = element as HTMLMediaElement
-    safeAttrs.rr_mediaState = mediaElement.paused ? 'paused' : 'played'
+    attrs.rr_mediaState = mediaElement.paused ? 'paused' : 'played'
   }
 
   /**
@@ -111,13 +139,13 @@ export function serializeAttributes(
       break
   }
   if (scrollLeft) {
-    safeAttrs.rr_scrollLeft = scrollLeft
+    attrs.rr_scrollLeft = scrollLeft
   }
   if (scrollTop) {
-    safeAttrs.rr_scrollTop = scrollTop
+    attrs.rr_scrollTop = scrollTop
   }
 
-  return safeAttrs
+  return attrs
 }
 
 export function getCssRulesString(cssStyleSheet: CSSStyleSheet | undefined | null): string | null {