👷‍♀️ [RUM-11139] Improve textual content extraction for action names (#3759)

* Use treewalker to replace innerText

* Add support for display and contentVisibility

* Improve code and add test cases

* Clean up

* Update packages/rum-core/src/domain/action/getActionNameFromElement.spec.ts

Co-authored-by: sethfowler-datadog <[email protected]>

* Add feature flag

* Improve code

---------

Co-authored-by: sethfowler-datadog <[email protected]>

Author: cy-moi

packages/core/src/tools/experimentalFeatures.ts

@@ -16,6 +16,8 @@ import { objectHasValue } from './utils/objectUtils'
 export enum ExperimentalFeature {
   TRACK_INTAKE_REQUESTS = 'track_intake_requests',
   WRITABLE_RESOURCE_GRAPHQL = 'writable_resource_graphql',
+  WATCH_COOKIE_WITHOUT_LOCK = 'watch_cookie_without_lock',
+  USE_TREE_WALKER_FOR_ACTION_NAME = 'use_tree_walker_for_action_name',
 }
 
 const enabledExperimentalFeatures: Set<ExperimentalFeature> = new Set()

packages/rum-core/src/domain/action/getActionNameFromElement.spec.ts

@@ -1,3 +1,5 @@
+import { ExperimentalFeature } from '@datadog/browser-core'
+import { mockExperimentalFeatures } from '../../../../core/test'
 import { appendElement, mockRumConfiguration } from '../../../test'
 import { NodePrivacyLevel } from '../privacy'
 import { ActionNameSource, getActionNameFromElement } from './getActionNameFromElement'
@@ -96,6 +98,57 @@ describe('getActionNameFromElement', () => {
     expect(nameSource).toBe('text_content')
   })
 
+  it('should correctly compute whitespace for <br> and <p> elements', () => {
+    const testCases = [
+      { element: appendElement('<div>hello<br/>world</div>'), expected: 'hello world' },
+      { element: appendElement('<div>hello<p>world</p></div>'), expected: 'hello world' },
+      { element: appendElement('<div>hello<p>world<br/>!</p></div>'), expected: 'hello world !' },
+      { element: appendElement('<div>hello world<br/>!<p>!</p></div>'), expected: 'hello world ! !' },
+    ]
+    testCases.forEach(({ element, expected }) => {
+      const { name, nameSource } = getActionNameFromElement(element, defaultConfiguration)
+      expect(name).toBe(expected)
+      expect(nameSource).toBe('text_content')
+    })
+  })
+
+  it('should introduce whitespace for block-level display values', () => {
+    mockExperimentalFeatures([ExperimentalFeature.USE_TREE_WALKER_FOR_ACTION_NAME])
+    const testCases = [
+      { display: 'block', expected: 'space' },
+      { display: 'inline-block', expected: 'no-space' },
+      { display: 'flex', expected: 'space' },
+      { display: 'inline-flex', expected: 'no-space' },
+      { display: 'grid', expected: 'space' },
+      { display: 'inline-grid', expected: 'no-space' },
+      { display: 'list-item', expected: 'space' },
+      { display: 'table', expected: 'space' },
+      { display: 'table-caption', expected: 'space' },
+      { display: 'inline', expected: 'no-space' },
+      { display: 'none', expected: 'nothing' },
+    ]
+    testCases.forEach(({ display, expected }) => {
+      const element = appendElement(
+        `<div><div style="display: ${display}">foo</div><div style="display: ${display}">bar</div></div>`
+      )
+      const { name, nameSource } = getActionNameFromElement(element, defaultConfiguration)
+      switch (expected) {
+        case 'space':
+          expect(name).toBe('foo bar')
+          expect(nameSource).toBe('text_content')
+          break
+        case 'no-space':
+          expect(name).toBe('foobar')
+          expect(nameSource).toBe('text_content')
+          break
+        case 'nothing':
+          expect(name).toBe('')
+          expect(nameSource).toBe('blank')
+          break
+      }
+    })
+  })
+
   it('ignores the inline script textual content', () => {
     const { name, nameSource } = getActionNameFromElement(
       appendElement("<div><script>console.log('toto')</script>b</div>"),
@@ -418,7 +471,7 @@ describe('getActionNameFromElement', () => {
       expect(nameSource).toBe('custom_attribute')
     })
 
-    it('remove children with programmatic action name in textual content', () => {
+    it('removes children with programmatic action name in textual content', () => {
       const { name, nameSource } = getActionNameFromElement(
         appendElement('<div>Foo <div data-dd-action-name="custom action">bar<div></div>'),
         defaultConfiguration
@@ -428,6 +481,16 @@ describe('getActionNameFromElement', () => {
       expect(nameSource).toBe('text_content')
     })
 
+    it('removes only the child with programmatic action name in textual content', () => {
+      mockExperimentalFeatures([ExperimentalFeature.USE_TREE_WALKER_FOR_ACTION_NAME])
+      const { name, nameSource } = getActionNameFromElement(
+        appendElement('<div>Foobar Baz<div data-dd-action-name="custom action">bar<div></div>'),
+        defaultConfiguration
+      )
+      expect(name).toBe('Foobar Baz')
+      expect(nameSource).toBe('text_content')
+    })
+
     it('remove children with programmatic action name in textual content based on the user-configured attribute', () => {
       const { name, nameSource } = getActionNameFromElement(
         appendElement('<div>Foo <div data-test-id="custom action">bar<div></div>'),
@@ -639,6 +702,31 @@ describe('getActionNameFromElement', () => {
           )
         ).toEqual({ name: 'foo', nameSource: ActionNameSource.TEXT_CONTENT })
       })
+
+      it('inherit privacy level and remove only the masked child', () => {
+        mockExperimentalFeatures([ExperimentalFeature.USE_TREE_WALKER_FOR_ACTION_NAME])
+        expect(
+          getActionNameFromElement(
+            appendElement(`
+              <div class="dd-privacy-allow" target>
+                bar secret
+                <div>
+                  foo 
+                  <div class="dd-privacy-mask">
+                    <span>secret</span>
+                  </div>
+                </div>
+              </div>
+        `),
+            {
+              ...defaultConfiguration,
+              enablePrivacyForActionName: true,
+            },
+            NodePrivacyLevel.ALLOW
+          )
+        ).toEqual({ name: 'bar secret foo', nameSource: ActionNameSource.TEXT_CONTENT })
+      })
+
       it('fallback to children but not the masked one with class names', () => {
         expect(
           getActionNameFromElement(

packages/rum-core/src/domain/action/getActionNameFromElement.ts

@@ -1,6 +1,7 @@
-import { safeTruncate } from '@datadog/browser-core'
-import { NodePrivacyLevel, getPrivacySelector } from '../privacy'
+import { ExperimentalFeature, isExperimentalFeatureEnabled, safeTruncate } from '@datadog/browser-core'
+import { getNodeSelfPrivacyLevel, getPrivacySelector, NodePrivacyLevel, shouldMaskNode } from '../privacy'
 import type { RumConfiguration } from '../configuration'
+import { isElementNode } from '../../browser/htmlDomUtils'
 
 /**
  * Get the action name from the attribute 'data-dd-action-name' on the element or any of its parent.
@@ -23,7 +24,7 @@ interface ActionName {
 export function getActionNameFromElement(
   element: Element,
   { enablePrivacyForActionName, actionNameAttribute: userProgrammaticAttribute }: RumConfiguration,
-  nodePrivacyLevel?: NodePrivacyLevel
+  nodePrivacyLevel: NodePrivacyLevel = NodePrivacyLevel.ALLOW
 ): ActionName {
   // Proceed to get the action name in two steps:
   // * first, get the name programmatically, explicitly defined by the user.
@@ -73,14 +74,14 @@ function getActionNameFromElementProgrammatically(targetElement: Element, progra
 type NameStrategy = (
   element: Element | HTMLElement | HTMLInputElement | HTMLSelectElement,
   userProgrammaticAttribute: string | undefined,
-  privacyEnabledActionName?: boolean
+  privacyEnabledActionName: boolean
 ) => ActionName | undefined | null
 
 const priorityStrategies: NameStrategy[] = [
   // associated LABEL text
-  (element, userProgrammaticAttribute) => {
+  (element, userProgrammaticAttribute, privacyEnabledActionName) => {
     if ('labels' in element && element.labels && element.labels.length > 0) {
-      return getActionNameFromTextualContent(element.labels[0], userProgrammaticAttribute)
+      return getActionNameFromTextualContent(element.labels[0], userProgrammaticAttribute, privacyEnabledActionName)
     }
   },
   // INPUT button (and associated) value
@@ -120,9 +121,9 @@ const priorityStrategies: NameStrategy[] = [
   (element) => getActionNameFromStandardAttribute(element, 'title'),
   (element) => getActionNameFromStandardAttribute(element, 'placeholder'),
   // SELECT first OPTION text
-  (element, userProgrammaticAttribute) => {
+  (element, userProgrammaticAttribute, privacyEnabledActionName) => {
     if ('options' in element && element.options.length > 0) {
-      return getActionNameFromTextualContent(element.options[0], userProgrammaticAttribute)
+      return getActionNameFromTextualContent(element.options[0], userProgrammaticAttribute, privacyEnabledActionName)
     }
   },
 ]
@@ -141,7 +142,7 @@ function getActionNameFromElementForStrategies(
   targetElement: Element,
   userProgrammaticAttribute: string | undefined,
   strategies: NameStrategy[],
-  privacyEnabledActionName?: boolean
+  privacyEnabledActionName: boolean
 ) {
   let element: Element | null = targetElement
   let recursionCounter = 0
@@ -196,7 +197,7 @@ function getActionNameFromStandardAttribute(element: Element | HTMLElement, attr
 function getActionNameFromTextualContent(
   element: Element | HTMLElement,
   userProgrammaticAttribute: string | undefined,
-  privacyEnabledActionName?: boolean
+  privacyEnabledActionName: boolean
 ): ActionName {
   return {
     name: getTextualContent(element, userProgrammaticAttribute, privacyEnabledActionName) || '',
@@ -205,16 +206,20 @@ function getActionNameFromTextualContent(
 }
 
 function getTextualContent(
-  element: Element | HTMLElement,
+  element: Element,
   userProgrammaticAttribute: string | undefined,
-  privacyEnabledActionName?: boolean
+  privacyEnabledActionName: boolean
 ) {
   if ((element as HTMLElement).isContentEditable) {
     return
   }
 
+  if (isExperimentalFeatureEnabled(ExperimentalFeature.USE_TREE_WALKER_FOR_ACTION_NAME)) {
+    return getTextualContentWithTreeWalker(element, userProgrammaticAttribute, privacyEnabledActionName)
+  }
+
   if ('innerText' in element) {
-    let text = element.innerText
+    let text = (element as HTMLElement).innerText
 
     const removeTextFromElements = (query: string) => {
       const list = element.querySelectorAll<Element | HTMLElement>(query)
@@ -248,3 +253,60 @@ function getTextualContent(
 
   return element.textContent
 }
+
+function getTextualContentWithTreeWalker(
+  element: Element,
+  userProgrammaticAttribute: string | undefined,
+  privacyEnabledActionName: boolean
+) {
+  const walker = document.createTreeWalker(
+    element,
+    // eslint-disable-next-line no-bitwise
+    NodeFilter.SHOW_ELEMENT | NodeFilter.SHOW_TEXT,
+    rejectInvisibleOrMaskedElementsFilter
+  )
+
+  let text = ''
+
+  while (walker.nextNode()) {
+    const node = walker.currentNode
+    if (isElementNode(node)) {
+      if (
+        // Following InnerText rendering spec https://html.spec.whatwg.org/multipage/dom.html#rendered-text-collection-steps
+        node.nodeName === 'BR' ||
+        node.nodeName === 'P' ||
+        ['block', 'flex', 'grid', 'list-item', 'table', 'table-caption'].includes(getComputedStyle(node).display)
+      ) {
+        text += ' '
+      }
+      continue // skip element nodes
+    }
+
+    text += node.textContent || ''
+  }
+
+  return text.replace(/\s+/g, ' ').trim()
+
+  function rejectInvisibleOrMaskedElementsFilter(node: Node) {
+    if (isElementNode(node)) {
+      const nodeSelfPrivacyLevel = getNodeSelfPrivacyLevel(node)
+      if (
+        node.hasAttribute(DEFAULT_PROGRAMMATIC_ACTION_NAME_ATTRIBUTE) ||
+        (userProgrammaticAttribute && node.hasAttribute(userProgrammaticAttribute)) ||
+        (privacyEnabledActionName && nodeSelfPrivacyLevel && shouldMaskNode(node, nodeSelfPrivacyLevel))
+      ) {
+        return NodeFilter.FILTER_REJECT
+      }
+      const style = getComputedStyle(node)
+      if (
+        style.visibility !== 'visible' ||
+        style.display === 'none' ||
+        (style.contentVisibility && style.contentVisibility !== 'visible')
+        // contentVisibility is not supported in all browsers, so we need to check it
+      ) {
+        return NodeFilter.FILTER_REJECT
+      }
+    }
+    return NodeFilter.FILTER_ACCEPT
+  }
+}