Skip to content

Commit

Permalink
fix: handle newlines in classnames (#919)
Browse files Browse the repository at this point in the history
* fix: handle newlines in classnames

* improve comment

* tidy tests
  • Loading branch information
pauldambra authored Dec 5, 2023
1 parent eb73c2b commit 3b3a344
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 17 deletions.
33 changes: 33 additions & 0 deletions src/__tests__/autocapture-utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
getNestedSpanText,
getDirectAndNestedSpanText,
getElementsChainString,
getClassNames,
} from '../autocapture-utils'
import { document } from '../utils/globals'
import { makeMouseEvent } from './autocapture.test'
Expand Down Expand Up @@ -397,4 +398,36 @@ describe(`Autocapture utility functions`, () => {
expect(elementChain).toEqual('div:text="text"nth-child="1"nth-of-type="2"')
})
})

describe('getClassNames', () => {
it('should cope when there is no classNames attribute', () => {
const el = document!.createElement('div')
const classNames = getClassNames(el)
expect(classNames).toEqual([])
})
it('should cope when there is an empty classNames attribute', () => {
const el = document!.createElement('div')
el.className = ''
const classNames = getClassNames(el)
expect(classNames).toEqual([])
})
it('should cope with a normal class list', () => {
const el = document!.createElement('div')
el.className = 'class1 class2'
const classNames = getClassNames(el)
expect(classNames).toEqual(['class1', 'class2'])
})
it('should cope with a class list with empty strings and tabs', () => {
const el = document!.createElement('div')
el.className = ' class1 class2 '
const classNames = getClassNames(el)
expect(classNames).toEqual(['class1', 'class2'])
})
it('should cope with a class list with unexpected new lines', () => {
const el = document!.createElement('div')
el.className = ' class1\r\n \r\n \n class2 '
const classNames = getClassNames(el)
expect(classNames).toEqual(['class1', 'class2'])
})
})
})
31 changes: 18 additions & 13 deletions src/autocapture-utils.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,32 @@
/*
* Get the className of an element, accounting for edge cases where element.className is an object
* @param {Element} el - element to get the className of
* @returns {string} the element's class
*/

import { AutocaptureConfig, Properties } from 'types'
import { _each, _entries, _includes, _trim } from './utils'

import { _isArray, _isNull, _isString, _isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'
import { window } from './utils/globals'

export function getClassName(el: Element): string {
/*
* Get the className of an element, accounting for edge cases where element.className is an object
*
* Because this is a string it can contain unexpected characters
* So, this method safely splits the className and returns that array.
*/
export function getClassNames(el: Element): string[] {
let className = ''
switch (typeof el.className) {
case 'string':
return el.className
className = el.className
break
// TODO: when is this ever used?
case 'object': // handle cases where className might be SVGAnimatedString or some other type
return ('baseVal' in el.className ? (el.className as any).baseVal : null) || el.getAttribute('class') || ''
className =
('baseVal' in el.className ? (el.className as any).baseVal : null) || el.getAttribute('class') || ''
break
default:
// future proof
return ''
className = ''
}

return className.length ? _trim(className).split(/\s+/) : []
}

/*
Expand Down Expand Up @@ -203,13 +208,13 @@ export function shouldCaptureDomEvent(
*/
export function shouldCaptureElement(el: Element): boolean {
for (let curEl = el; curEl.parentNode && !isTag(curEl, 'body'); curEl = curEl.parentNode as Element) {
const classes = getClassName(curEl).split(' ')
const classes = getClassNames(curEl)
if (_includes(classes, 'ph-sensitive') || _includes(classes, 'ph-no-capture')) {
return false
}
}

if (_includes(getClassName(el).split(' '), 'ph-include')) {
if (_includes(getClassNames(el), 'ph-include')) {
return true
}

Expand Down
8 changes: 4 additions & 4 deletions src/autocapture.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { _bind_instance_methods, _each, _extend, _includes, _register_event, _safewrap_instance_methods } from './utils'
import {
getClassName,
getClassNames,
getSafeText,
isElementNode,
isSensitiveElement,
Expand Down Expand Up @@ -89,9 +89,9 @@ const autocapture = {
}
}

const classes = getClassName(elem)
const classes = getClassNames(elem)
if (classes.length > 0)
props['classes'] = classes.split(' ').filter(function (c) {
props['classes'] = classes.filter(function (c) {
return c !== ''
})

Expand Down Expand Up @@ -219,7 +219,7 @@ const autocapture = {
}

// allow users to programmatically prevent capturing of elements by adding class 'ph-no-capture'
const classes = getClassName(el).split(' ')
const classes = getClassNames(el)
if (_includes(classes, 'ph-no-capture')) {
explicitNoCapture = true
}
Expand Down

0 comments on commit 3b3a344

Please sign in to comment.