Skip to content

Commit

Permalink
Integrate e063320 (#2663) from aymeric/fix-cannot-read-property-lengt…
Browse files Browse the repository at this point in the history
…h-of-undefined-ref into staging-13

Co-authored-by: Aymeric Mortemousque <[email protected]>
  • Loading branch information
dd-mergequeue[bot] and amortemousque authored Mar 25, 2024
2 parents 3bc5f7f + e063320 commit eb11386
Show file tree
Hide file tree
Showing 11 changed files with 158 additions and 72 deletions.
25 changes: 1 addition & 24 deletions packages/core/src/tools/utils/polyfills.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { startsWith, arrayFrom, cssEscape, elementMatches } from './polyfills'
import { startsWith, arrayFrom } from './polyfills'

describe('polyfills', () => {
describe('startWith', () => {
Expand Down Expand Up @@ -26,27 +26,4 @@ describe('polyfills', () => {
expect(arrayFrom(div.classList)).toEqual(['foo'])
})
})

describe('cssEscape', () => {
it('should escape a string', () => {
expect(cssEscape('.foo#bar')).toEqual('\\.foo\\#bar')
expect(cssEscape('()[]{}')).toEqual('\\(\\)\\[\\]\\{\\}')
expect(cssEscape('--a')).toEqual('--a')
expect(cssEscape('\0')).toEqual('\ufffd')
})
})

describe('elementMatches', () => {
it('should return true if the element matches the selector', () => {
const element = document.createElement('div')
element.classList.add('foo')
expect(elementMatches(element, '.foo')).toEqual(true)
})

it('should return false if the element does not match the selector', () => {
const element = document.createElement('div')
element.classList.add('bar')
expect(elementMatches(element, '.foo')).toEqual(false)
})
})
})
32 changes: 0 additions & 32 deletions packages/core/src/tools/utils/polyfills.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,38 +73,6 @@ export function endsWith(candidate: string, search: string) {
return candidate.slice(-search.length) === search
}

export function elementMatches(element: Element & { msMatchesSelector?(selector: string): boolean }, selector: string) {
if (element.matches) {
return element.matches(selector)
}
// IE11 support
if (element.msMatchesSelector) {
return element.msMatchesSelector(selector)
}
return false
}

// https://github.com/jquery/jquery/blob/a684e6ba836f7c553968d7d026ed7941e1a612d8/src/selector/escapeSelector.js
export function cssEscape(str: string) {
if (window.CSS && window.CSS.escape) {
return window.CSS.escape(str)
}

// eslint-disable-next-line no-control-regex
return str.replace(/([\0-\x1f\x7f]|^-?\d)|^-$|[^\x80-\uFFFF\w-]/g, function (ch, asCodePoint) {
if (asCodePoint) {
// U+0000 NULL becomes U+FFFD REPLACEMENT CHARACTER
if (ch === '\0') {
return '\uFFFD'
}
// Control characters and (dependent upon position) numbers get escaped as code points
return `${ch.slice(0, -1)}\\${ch.charCodeAt(ch.length - 1).toString(16)} `
}
// Other potentially-special ASCII characters get backslash-escaped
return `\\${ch}`
})
}

interface Assignable {
[key: string]: any
}
Expand Down
57 changes: 57 additions & 0 deletions packages/rum-core/src/browser/polyfills.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { appendElement } from '../../test'
import { cssEscape, elementMatches, getClassList, getParentElement } from './polyfills'

describe('cssEscape', () => {
it('should escape a string', () => {
expect(cssEscape('.foo#bar')).toEqual('\\.foo\\#bar')
expect(cssEscape('()[]{}')).toEqual('\\(\\)\\[\\]\\{\\}')
expect(cssEscape('--a')).toEqual('--a')
expect(cssEscape('\0')).toEqual('\ufffd')
})
})

describe('elementMatches', () => {
it('should return true if the element matches the selector', () => {
const element = document.createElement('div')
element.classList.add('foo')
expect(elementMatches(element, '.foo')).toEqual(true)
})

it('should return false if the element does not match the selector', () => {
const element = document.createElement('div')
element.classList.add('bar')
expect(elementMatches(element, '.foo')).toEqual(false)
})
})

describe('getParentElement', () => {
it('should return the parentElement of a element that supports the parentElement property', () => {
const divElement = appendElement('<div class="parent"><div target></div></div>')
const parenElement = getParentElement(divElement)
expect(parenElement?.className).toEqual('parent')
})

it('should return the parentElement of a element that does not support the parentElement property (e.g.: svg on IE)', () => {
const svgElement = appendElement('<div class="parent"><svg target></svg></div>')
Object.defineProperty(svgElement, 'parenElement', { get: () => undefined })
const parenElement = getParentElement(svgElement)
expect(parenElement?.className).toEqual('parent')
})
})

describe('getClassList', () => {
it('should return the classList of an element that supports the classList property', () => {
const divElement = appendElement('<div class="foo bar"></div>')
const classList = getClassList(divElement)
expect(classList[0]).toEqual('foo')
expect(classList[1]).toEqual('bar')
})

it('should return the classList of an element that does not support the classList property (e.g.: svg on IE)', () => {
const svgElement = appendElement('<svg class="foo bar"></svg>')
Object.defineProperty(svgElement, 'classList', { get: () => undefined })
const classList = getClassList(svgElement)
expect(classList[0]).toEqual('foo')
expect(classList[1]).toEqual('bar')
})
})
64 changes: 64 additions & 0 deletions packages/rum-core/src/browser/polyfills.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
// https://github.com/jquery/jquery/blob/a684e6ba836f7c553968d7d026ed7941e1a612d8/src/selector/escapeSelector.js
export function cssEscape(str: string) {
if (window.CSS && window.CSS.escape) {
return window.CSS.escape(str)
}

// eslint-disable-next-line no-control-regex
return str.replace(/([\0-\x1f\x7f]|^-?\d)|^-$|[^\x80-\uFFFF\w-]/g, function (ch, asCodePoint) {
if (asCodePoint) {
// U+0000 NULL becomes U+FFFD REPLACEMENT CHARACTER
if (ch === '\0') {
return '\uFFFD'
}
// Control characters and (dependent upon position) numbers get escaped as code points
return `${ch.slice(0, -1)}\\${ch.charCodeAt(ch.length - 1).toString(16)} `
}
// Other potentially-special ASCII characters get backslash-escaped
return `\\${ch}`
})
}

export function elementMatches(element: Element & { msMatchesSelector?(selector: string): boolean }, selector: string) {
if (element.matches) {
return element.matches(selector)
}
// IE11 support
if (element.msMatchesSelector) {
return element.msMatchesSelector(selector)
}
return false
}

/**
* Return the parentElement of an node
*
* In cases where parentElement is not supported, such as in IE11 for SVG nodes, we fallback to parentNode
*/
export function getParentElement(node: Node): HTMLElement | null {
if (node.parentElement) {
return node.parentElement
}

let parentNode = node.parentNode
while (parentNode !== null && parentNode.nodeType !== Node.ELEMENT_NODE) {
parentNode = node.parentNode
}

return parentNode as HTMLElement | null
}

/**
* Return the classList of an element or an array of classes if classList is not supported
*
* In cases where classList is not supported, such as in IE11 for SVG and MathML elements,
* we fallback to using element.getAttribute('class').
* We opt for element.getAttribute('class') over element.className because className returns an SVGAnimatedString for SVG elements.
*/
export function getClassList(element: Element): DOMTokenList | string[] {
if (element.classList) {
return element.classList
}

return (element.getAttribute('class') || '').split(/\s+/)
}
3 changes: 2 additions & 1 deletion packages/rum-core/src/domain/action/computeFrustration.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { elementMatches, ONE_SECOND } from '@datadog/browser-core'
import { ONE_SECOND } from '@datadog/browser-core'
import { FrustrationType } from '../../rawRumEvent.types'
import { elementMatches } from '../../browser/polyfills'
import type { Click } from './trackClickActions'

const MIN_CLICKS_PER_SECOND_TO_CONSIDER_RAGE = 3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,18 @@ describe('getActionNameFromElement', () => {
).toBe('foo')
})

it('computes an action name on SVG elements (IE does not support parentElement property on them)', () => {
expect(
getActionNameFromElement(
appendElement(`
<button>
foo <svg target></svg>
<button>
`)
)
).toBe('foo')
})

describe('programmatically declared action name', () => {
it('extracts the name from the data-dd-action-name attribute', () => {
expect(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { safeTruncate, isIE, find } from '@datadog/browser-core'
import { getParentElement } from '../../browser/polyfills'

/**
* Get the action name from the attribute 'data-dd-action-name' on the element or any of its parent.
Expand Down Expand Up @@ -37,7 +38,7 @@ function getActionNameFromElementProgrammatically(targetElement: Element, progra
elementWithAttribute = element
break
}
element = element.parentElement
element = getParentElement(element)
}
}

Expand Down Expand Up @@ -147,7 +148,7 @@ function getActionNameFromElementForStrategies(
if (element.nodeName === 'FORM') {
break
}
element = element.parentElement
element = getParentElement(element)
recursionCounter += 1
}
}
Expand Down
7 changes: 6 additions & 1 deletion packages/rum-core/src/domain/getSelectorFromElement.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('getSelectorFromElement', () => {

describe('class selector', () => {
it('should use the class selector when the element as classes', () => {
expect(getSelector('<div class="foo"></div>')).toBe('BODY>DIV.foo')
expect(getSelector('<div target class="foo"></div>')).toBe('BODY>DIV.foo')
})

it('should use the class selector when siblings have the same classes but different tags', () => {
Expand Down Expand Up @@ -157,6 +157,11 @@ describe('getSelectorFromElement', () => {
})
})

it('should compute a CSS selector on SVG elements (IE does not support classList nor parentElement properties on them)', () => {
const element = appendElement('<svg class="foo"></svg>')
expect(getSelector(element)).toBe('BODY>svg.foo')
})

function getSelector(htmlOrElement: string | Element, actionNameAttribute?: string): string {
return getSelectorFromElement(
typeof htmlOrElement === 'string' ? appendElement(htmlOrElement) : htmlOrElement,
Expand Down
21 changes: 10 additions & 11 deletions packages/rum-core/src/domain/getSelectorFromElement.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { cssEscape } from '@datadog/browser-core'
import { cssEscape, getClassList, getParentElement } from '../browser/polyfills'
import { DEFAULT_PROGRAMMATIC_ACTION_NAME_ATTRIBUTE } from './action/getActionNameFromElement'

/**
Expand Down Expand Up @@ -64,7 +64,7 @@ export function getSelectorFromElement(targetElement: Element, actionNameAttribu
targetElementSelector =
uniqueSelectorAmongChildren || combineSelector(getPositionSelector(element), targetElementSelector)

element = element.parentElement
element = getParentElement(element)
}

return targetElementSelector
Expand Down Expand Up @@ -92,15 +92,14 @@ function getClassSelector(element: Element): string | undefined {
if (element.tagName === 'BODY') {
return
}
if (element.classList.length > 0) {
for (let i = 0; i < element.classList.length; i += 1) {
const className = element.classList[i]
if (isGeneratedValue(className)) {
continue
}

return `${cssEscape(element.tagName)}.${cssEscape(className)}`
const classList = getClassList(element)
for (let i = 0; i < classList.length; i += 1) {
const className = classList[i]
if (isGeneratedValue(className)) {
continue
}

return `${cssEscape(element.tagName)}.${cssEscape(className)}`
}
}

Expand Down Expand Up @@ -182,7 +181,7 @@ function isSelectorUniqueGlobally(element: Element, selector: string): boolean {
*/
function isSelectorUniqueAmongSiblings(element: Element, selector: string): boolean {
return (
element.parentElement!.querySelectorAll(supportScopeSelector() ? combineSelector(':scope', selector) : selector)
getParentElement(element)!.querySelectorAll(supportScopeSelector() ? combineSelector(':scope', selector) : selector)
.length === 1
)
}
Expand Down
1 change: 1 addition & 0 deletions packages/rum-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,5 @@ export { RumInitConfiguration, RumConfiguration } from './domain/configuration'
export { DEFAULT_PROGRAMMATIC_ACTION_NAME_ATTRIBUTE } from './domain/action/getActionNameFromElement'
export { STABLE_ATTRIBUTES } from './domain/getSelectorFromElement'
export * from './browser/htmlDomUtils'
export * from './browser/polyfills'
export { getSessionReplayUrl } from './domain/getSessionReplayUrl'
3 changes: 2 additions & 1 deletion packages/rum/src/domain/record/observers/inputObserver.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { ListenerHandler } from '@datadog/browser-core'
import { instrumentSetter, assign, DOM_EVENT, addEventListeners, forEach, noop, cssEscape } from '@datadog/browser-core'
import { instrumentSetter, assign, DOM_EVENT, addEventListeners, forEach, noop } from '@datadog/browser-core'
import { cssEscape } from '@datadog/browser-rum-core'
import type { RumConfiguration } from '@datadog/browser-rum-core'
import { NodePrivacyLevel } from '../../../constants'
import type { InputState } from '../../../types'
Expand Down

0 comments on commit eb11386

Please sign in to comment.