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.

Sign up for GitHub

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: dead click fixes from watching in prod #1508

Merged
merged 2 commits into from
Nov 1, 2024
Merged

fix: dead click fixes from watching in prod #1508

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
afaa990
fix: dead click fixes from watching in prod
pauldambra Oct 31, 2024
01154fd
don't ignore disabled
pauldambra Nov 1, 2024
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
228 changes: 125 additions & 103 deletions src/__tests__/entrypoints/lazy-loaded-dead-clicks-autocapture.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,16 @@ describe('LazyLoadedDeadClicksAutocapture', () => {
expect(lazyLoadedDeadClicksAutocapture['_clicks'].length).toBe(0)
})

it('ignores clicks on disabled nodes', () => {
const div = document.createElement('div')
div.setAttribute('disabled', 'true')
document.body.append(div)

triggerMouseEvent(div, 'click')

expect(lazyLoadedDeadClicksAutocapture['_clicks'].length).toBe(0)
})

it.each(autocaptureCompatibleElements)('click on %s node is never a deadclick', (element) => {
const el = document.createElement(element)
document.body.append(el)
Expand Down Expand Up @@ -211,32 +221,35 @@ describe('LazyLoadedDeadClicksAutocapture', () => {
lazyLoadedDeadClicksAutocapture['_checkClicks']()

expect(lazyLoadedDeadClicksAutocapture['_clicks']).toHaveLength(0)
expect(fakeInstance.capture).toHaveBeenCalledWith('$dead_click', {
// faked system timestamp isn't moving so this is negative
$dead_click_absolute_delay_ms: -900,
$dead_click_absolute_timeout: false,
$dead_click_event_timestamp: 900,
$dead_click_last_mutation_timestamp: undefined,
$dead_click_last_scroll_timestamp: undefined,
$dead_click_mutation_delay_ms: undefined,
$dead_click_mutation_timeout: false,
$dead_click_scroll_delay_ms: undefined,
$dead_click_scroll_timeout: false,
$dead_click_selection_changed_delay_ms: 100,
$dead_click_selection_changed_timeout: true,
timestamp: 900,
$ce_version: 1,
$el_text: 'text',
$elements: [
{
$el_text: 'text',
nth_child: 2,
nth_of_type: 1,
tag_name: 'body',
},
],
$event_type: 'click',
})
expect(fakeInstance.capture).toHaveBeenCalledWith(
'$dead_click',
{
// faked system timestamp isn't moving so this is negative
$dead_click_absolute_delay_ms: -900,
$dead_click_absolute_timeout: false,
$dead_click_event_timestamp: 900,
$dead_click_last_mutation_timestamp: undefined,
$dead_click_last_scroll_timestamp: undefined,
$dead_click_mutation_delay_ms: undefined,
$dead_click_mutation_timeout: false,
$dead_click_scroll_delay_ms: undefined,
$dead_click_scroll_timeout: false,
$dead_click_selection_changed_delay_ms: 100,
$dead_click_selection_changed_timeout: true,
$ce_version: 1,
$el_text: 'text',
$elements: [
{
$el_text: 'text',
nth_child: 2,
nth_of_type: 1,
tag_name: 'body',
},
],
$event_type: 'click',
},
{ timestamp: new Date(900) }
)
})

it('click followed by a mutation after threshold, dead click', () => {
Expand All @@ -250,32 +263,35 @@ describe('LazyLoadedDeadClicksAutocapture', () => {
lazyLoadedDeadClicksAutocapture['_checkClicks']()

expect(lazyLoadedDeadClicksAutocapture['_clicks']).toHaveLength(0)
expect(fakeInstance.capture).toHaveBeenCalledWith('$dead_click', {
// faked system timestamp isn't moving so this is negative
$dead_click_absolute_delay_ms: -900,
$dead_click_absolute_timeout: false,
$dead_click_event_timestamp: 900,
$dead_click_last_mutation_timestamp: 3401,
$dead_click_last_scroll_timestamp: undefined,
$dead_click_mutation_delay_ms: 2501,
$dead_click_mutation_timeout: true,
$dead_click_scroll_delay_ms: undefined,
$dead_click_scroll_timeout: false,
$dead_click_selection_changed_delay_ms: undefined,
$dead_click_selection_changed_timeout: false,
timestamp: 900,
$ce_version: 1,
$el_text: 'text',
$elements: [
{
$el_text: 'text',
nth_child: 2,
nth_of_type: 1,
tag_name: 'body',
},
],
$event_type: 'click',
})
expect(fakeInstance.capture).toHaveBeenCalledWith(
'$dead_click',
{
// faked system timestamp isn't moving so this is negative
$dead_click_absolute_delay_ms: -900,
$dead_click_absolute_timeout: false,
$dead_click_event_timestamp: 900,
$dead_click_last_mutation_timestamp: 3401,
$dead_click_last_scroll_timestamp: undefined,
$dead_click_mutation_delay_ms: 2501,
$dead_click_mutation_timeout: true,
$dead_click_scroll_delay_ms: undefined,
$dead_click_scroll_timeout: false,
$dead_click_selection_changed_delay_ms: undefined,
$dead_click_selection_changed_timeout: false,
$ce_version: 1,
$el_text: 'text',
$elements: [
{
$el_text: 'text',
nth_child: 2,
nth_of_type: 1,
tag_name: 'body',
},
],
$event_type: 'click',
},
{ timestamp: new Date(900) }
)
})

it('click followed by a scroll after threshold, dead click', () => {
Expand All @@ -290,31 +306,34 @@ describe('LazyLoadedDeadClicksAutocapture', () => {
lazyLoadedDeadClicksAutocapture['_checkClicks']()

expect(lazyLoadedDeadClicksAutocapture['_clicks']).toHaveLength(0)
expect(fakeInstance.capture).toHaveBeenCalledWith('$dead_click', {
// faked system timestamp isn't moving so this is negative
$dead_click_absolute_delay_ms: -900,
$dead_click_absolute_timeout: false,
$dead_click_event_timestamp: 900,
$dead_click_last_mutation_timestamp: undefined,
$dead_click_mutation_delay_ms: undefined,
$dead_click_mutation_timeout: false,
$dead_click_scroll_delay_ms: 2501,
$dead_click_scroll_timeout: true,
$dead_click_selection_changed_delay_ms: undefined,
$dead_click_selection_changed_timeout: false,
$ce_version: 1,
$el_text: 'text',
$elements: [
{
$el_text: 'text',
nth_child: 2,
nth_of_type: 1,
tag_name: 'body',
},
],
$event_type: 'click',
timestamp: 900,
})
expect(fakeInstance.capture).toHaveBeenCalledWith(
'$dead_click',
{
// faked system timestamp isn't moving so this is negative
$dead_click_absolute_delay_ms: -900,
$dead_click_absolute_timeout: false,
$dead_click_event_timestamp: 900,
$dead_click_last_mutation_timestamp: undefined,
$dead_click_mutation_delay_ms: undefined,
$dead_click_mutation_timeout: false,
$dead_click_scroll_delay_ms: 2501,
$dead_click_scroll_timeout: true,
$dead_click_selection_changed_delay_ms: undefined,
$dead_click_selection_changed_timeout: false,
$ce_version: 1,
$el_text: 'text',
$elements: [
{
$el_text: 'text',
nth_child: 2,
nth_of_type: 1,
tag_name: 'body',
},
],
$event_type: 'click',
},
{ timestamp: new Date(900) }
)
})

it('click followed by nothing for too long, dead click', () => {
Expand All @@ -325,35 +344,38 @@ describe('LazyLoadedDeadClicksAutocapture', () => {
})
lazyLoadedDeadClicksAutocapture['_lastMutation'] = undefined

jest.setSystemTime(2501 + 900)
jest.setSystemTime(3001 + 900)
lazyLoadedDeadClicksAutocapture['_checkClicks']()

expect(lazyLoadedDeadClicksAutocapture['_clicks']).toHaveLength(0)
expect(fakeInstance.capture).toHaveBeenCalledWith('$dead_click', {
$dead_click_absolute_delay_ms: 2501,
$dead_click_absolute_timeout: true,
$dead_click_event_timestamp: 900,
$dead_click_last_mutation_timestamp: undefined,
$dead_click_last_scroll_timestamp: undefined,
$dead_click_mutation_delay_ms: undefined,
$dead_click_mutation_timeout: false,
$dead_click_scroll_delay_ms: undefined,
$dead_click_scroll_timeout: false,
$dead_click_selection_changed_delay_ms: undefined,
$dead_click_selection_changed_timeout: false,
$ce_version: 1,
$el_text: 'text',
$elements: [
{
$el_text: 'text',
nth_child: 2,
nth_of_type: 1,
tag_name: 'body',
},
],
$event_type: 'click',
timestamp: 900,
})
expect(fakeInstance.capture).toHaveBeenCalledWith(
'$dead_click',
{
$dead_click_absolute_delay_ms: 3001,
$dead_click_absolute_timeout: true,
$dead_click_event_timestamp: 900,
$dead_click_last_mutation_timestamp: undefined,
$dead_click_last_scroll_timestamp: undefined,
$dead_click_mutation_delay_ms: undefined,
$dead_click_mutation_timeout: false,
$dead_click_scroll_delay_ms: undefined,
$dead_click_scroll_timeout: false,
$dead_click_selection_changed_delay_ms: undefined,
$dead_click_selection_changed_timeout: false,
$ce_version: 1,
$el_text: 'text',
$elements: [
{
$el_text: 'text',
nth_child: 2,
nth_of_type: 1,
tag_name: 'body',
},
],
$event_type: 'click',
},
{ timestamp: new Date(900) }
)
})

it('click not followed by anything within threshold, rescheduled for next check', () => {
Expand Down
42 changes: 1 addition & 41 deletions src/autocapture-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { each, entries, includes, trim } from './utils'
import { isArray, isNullish, isString, isUndefined } from './utils/type-utils'
import { logger } from './utils/logger'
import { window } from './utils/globals'
import { isDocumentFragment, isElementNode, isTag, isTextNode } from './utils/element-utils'

export function splitClassString(s: string): string[] {
return s ? trim(s).split(/\s+/) : []
Expand Down Expand Up @@ -94,47 +95,6 @@ export function getEventTarget(e: Event): Element | null {
}
}

/*
* Check whether an element has nodeType Node.ELEMENT_NODE
* @param {Element} el - element to check
* @returns {boolean} whether el is of the correct nodeType
*/
export function isElementNode(el: Node | Element | undefined | null): el is Element {
return !!el && el.nodeType === 1 // Node.ELEMENT_NODE - use integer constant for browser portability
}

/*
* Check whether an element is of a given tag type.
* Due to potential reference discrepancies (such as the webcomponents.js polyfill),
* we want to match tagNames instead of specific references because something like
* element === document.body won't always work because element might not be a native
* element.
* @param {Element} el - element to check
* @param {string} tag - tag name (e.g., "div")
* @returns {boolean} whether el is of the given tag type
*/
export function isTag(el: Element | undefined | null, tag: string): el is HTMLElement {
return !!el && !!el.tagName && el.tagName.toLowerCase() === tag.toLowerCase()
}

/*
* Check whether an element has nodeType Node.TEXT_NODE
* @param {Element} el - element to check
* @returns {boolean} whether el is of the correct nodeType
*/
export function isTextNode(el: Element | undefined | null): el is HTMLElement {
return !!el && el.nodeType === 3 // Node.TEXT_NODE - use integer constant for browser portability
}

/*
* Check whether an element has nodeType Node.DOCUMENT_FRAGMENT_NODE
* @param {Element} el - element to check
* @returns {boolean} whether el is of the correct nodeType
*/
export function isDocumentFragment(el: Element | ParentNode | undefined | null): el is DocumentFragment {
return !!el && el.nodeType === 11 // Node.DOCUMENT_FRAGMENT_NODE - use integer constant for browser portability
}

export const autocaptureCompatibleElements = ['a', 'button', 'form', 'input', 'select', 'textarea', 'label']

/*
Expand Down
5 changes: 1 addition & 4 deletions src/autocapture.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,7 @@ import {
getEventTarget,
getSafeText,
isAngularStyleAttr,
isDocumentFragment,
isElementNode,
isSensitiveElement,
isTag,
isTextNode,
makeSafeText,
shouldCaptureDomEvent,
shouldCaptureElement,
Expand All @@ -27,6 +23,7 @@ import { isBoolean, isFunction, isNull, isObject } from './utils/type-utils'
import { logger } from './utils/logger'
import { document, window } from './utils/globals'
import { convertToURL } from './utils/request-utils'
import { isDocumentFragment, isElementNode, isTag, isTextNode } from './utils/element-utils'

const COPY_AUTOCAPTURE_EVENT = '$copy_autocapture'

Expand Down
Loading
Loading