Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle newlines in classnames #919

Merged
merged 3 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading