♻️ [PANA-5105] Serialize all DOM attribute values as strings

packages/rum/src/domain/record/serialization/htmlAst.specHelper.ts

@@ -1438,7 +1438,7 @@ export const AST_ALLOW = {
                   type: 2,
                   tagName: 'option',
                   attributes: {
-                    selected: true,
+                    selected: '',
                     'aria-label': 'A',
                     value: 'private option A',
                   },
@@ -1526,7 +1526,7 @@ export const AST_ALLOW = {
                 type: 'checkbox',
                 name: 'inputFoo',
                 value: 'on',
-                checked: true,
+                checked: '',
               },
               childNodes: [],
             },
@@ -1558,7 +1558,6 @@ export const AST_ALLOW = {
                 type: 'radio',
                 name: 'radioGroup',
                 value: 'bar-private',
-                checked: false,
               },
               childNodes: [],
             },

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.
@@ -69,19 +61,9 @@ describe('serializeAttributes for DOM attributes', () => {
       // serialized at all. Used for boolean form element attributes like `<option selected>`.
       // - 'maskable-image': Like 'maskable', except that we expect the masked
       // representation to be a data URL image.
-      // - 'maskable-option-selected': Like 'maskable-boolean', in that when unmasked the
-      // representation is a boolean value. However, when the value is masked, instead of
-      // not being serialized, its underlying string value is used. The effect is a weird
-      // hybrid of 'always-unmasked' and 'maskable-boolean'.
-      // TODO: Reduce the complexity here by aligning 'maskable-form',
-      // 'maskable-form-boolean', and 'maskable-option-selected'.
-      expectedBehavior:
-        | 'always-unmasked'
-        | 'maskable'
-        | 'maskable-form'
-        | 'maskable-form-boolean'
-        | 'maskable-image'
-        | 'maskable-option-selected'
+      // TODO: Reduce the complexity here by aligning 'maskable-form' and
+      // 'maskable-form-boolean'.
+      expectedBehavior: 'always-unmasked' | 'maskable' | 'maskable-form' | 'maskable-form-boolean' | 'maskable-image'
 
       // How to treat the IGNORE privacy level. The default is 'unmasked'.
       // TODO: Eliminate this inconsistency by always masking for IGNORE.
@@ -233,10 +215,7 @@ describe('serializeAttributes for DOM attributes', () => {
       // when masked, the entire attribute should not be serialized.
       {
         html: '<option selected>',
-        // TODO: This is a bug! If the <option> is masked, we don't set the value of
-        // 'selected' based on the property, but we still allow the DOM attribute to be
-        // recorded. We should fix this; this should be 'maskable-boolean'.
-        expectedBehavior: 'maskable-option-selected',
+        expectedBehavior: 'maskable-form-boolean',
         ignoreBehavior: 'masked',
       },
       {
@@ -260,7 +239,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)
       }
@@ -292,13 +272,7 @@ describe('serializeAttributes for DOM attributes', () => {
       attribute: { name: string; value: string | undefined },
       privacyLevel: NodePrivacyLevel
     ): boolean | string | undefined {
-      let unmaskedValue: boolean | string | undefined = testCase.attrValue ?? attribute.value
-      if (
-        testCase.expectedBehavior === 'maskable-form-boolean' ||
-        testCase.expectedBehavior === 'maskable-option-selected'
-      ) {
-        unmaskedValue = attribute.value === ''
-      }
+      const unmaskedValue: boolean | string | undefined = testCase.attrValue ?? attribute.value
 
       let maskedValue: boolean | string | undefined
       if (testCase.expectedBehavior === 'maskable-form') {
@@ -307,8 +281,6 @@ describe('serializeAttributes for DOM attributes', () => {
         maskedValue = undefined
       } else if (testCase.expectedBehavior === 'maskable-image') {
         maskedValue = CENSORED_IMG_MARK
-      } else if (testCase.expectedBehavior === 'maskable-option-selected') {
-        maskedValue = attribute.value
       } else {
         maskedValue = CENSORED_STRING_MARK
       }
@@ -344,7 +316,7 @@ describe('serializeAttributes for DOM attributes', () => {
   })
 })
 
-describe('serializeAttributes for virtual attributes', () => {
+describe('serializeVirtualAttributes', () => {
   let stats: SerializationStats
   let transaction: SerializationTransaction
 
@@ -353,36 +325,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,

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

@@ -1,94 +1,123 @@
 import { NodePrivacyLevel, shouldMaskNode } from '@datadog/browser-rum-core'
 import { isSafari } from '@datadog/browser-core'
-import { getElementInputValue, switchToAbsoluteUrl, getValidTagName } from './serializationUtils'
+import { getElementInputValue, 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, number | string> {
+  return {
+    ...serializeDOMAttributes(element, nodePrivacyLevel, transaction),
+    ...serializeVirtualAttributes(element, nodePrivacyLevel, transaction),
+  }
+}
+
+export function serializeDOMAttributes(
+  element: Element,
+  nodePrivacyLevel: NodePrivacyLevel,
+  transaction: SerializationTransaction
+): Record<string, 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> = {}
+  const tagName = element.tagName
 
   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
     }
   }
 
   if (
     (element as HTMLInputElement).value &&
-    (tagName === 'textarea' || tagName === 'select' || tagName === 'option' || tagName === 'input')
+    (tagName === 'TEXTAREA' || tagName === 'SELECT' || tagName === 'OPTION' || tagName === 'INPUT')
   ) {
     const formValue = getElementInputValue(element, nodePrivacyLevel)
     if (formValue !== undefined) {
-      safeAttrs.value = formValue
+      attrs.value = formValue
     }
   }
 
   /**
    * <Option> can be selected, which occurs if its `value` matches ancestor `<Select>.value`
    */
-  if (tagName === 'option' && nodePrivacyLevel === NodePrivacyLevel.ALLOW) {
-    // For privacy=`MASK`, all the values would be the same, so skip.
+  if (tagName === 'OPTION') {
     const optionElement = element as HTMLOptionElement
-    if (optionElement.selected) {
-      safeAttrs.selected = optionElement.selected
+    if (optionElement.selected && !shouldMaskNode(optionElement, nodePrivacyLevel)) {
+      attrs.selected = ''
+    } else {
+      delete attrs.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 (inputElement.checked && !shouldMaskNode(inputElement, nodePrivacyLevel)) {
+      attrs.checked = ''
+    } else {
+      delete attrs.checked
     }
   }
 
+  return attrs
+}
+
+export function serializeVirtualAttributes(
+  element: Element,
+  nodePrivacyLevel: NodePrivacyLevel,
+  transaction: SerializationTransaction
+): VirtualAttributes {
+  if (nodePrivacyLevel === NodePrivacyLevel.HIDDEN) {
+    return {}
+  }
+
+  const attrs: VirtualAttributes = {}
+  const tagName = element.tagName
+  const doc = element.ownerDocument
+
   // remote css
-  if (tagName === 'link') {
+  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
     }
   }
 
   // dynamic stylesheet
-  if (tagName === 'style' && (element as HTMLStyleElement).sheet) {
+  if (tagName === 'STYLE' && (element as HTMLStyleElement).sheet) {
     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
     }
   }
 
   /**
    * Serialize the media playback state
    */
-  if (tagName === 'audio' || tagName === 'video') {
+  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 +140,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 {