diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.spec.ts index ae5db2d9b6..c7203b3459 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.spec.ts @@ -87,6 +87,7 @@ describe('actionCollection', () => { x: 1, y: 2, }, + pointer_up_delay: undefined, }, }, }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.ts index ac0208f383..7a21b3059f 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.ts @@ -79,6 +79,7 @@ function processAction( action: { target: action.target, position: action.position, + pointer_up_delay: action.pointerUpDelay, }, }, } diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts index 96d9de98f9..bb1497fc53 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts @@ -6,14 +6,14 @@ import { listenActionEvents } from './listenActionEvents' describe('listenActionEvents', () => { let actionEventsHooks: { - onClick: jasmine.Spy['onClick']> + onStartEvent: jasmine.Spy['onStartEvent']> onPointerDown: jasmine.Spy['onPointerDown']> } let stopListenEvents: () => void beforeEach(() => { actionEventsHooks = { - onClick: jasmine.createSpy(), + onStartEvent: jasmine.createSpy(), onPointerDown: jasmine.createSpy().and.returnValue({}), } ;({ stop: stopListenEvents } = listenActionEvents(actionEventsHooks)) @@ -28,11 +28,32 @@ describe('listenActionEvents', () => { expect(actionEventsHooks.onPointerDown).toHaveBeenCalledOnceWith(jasmine.objectContaining({ type: 'pointerdown' })) }) + it('ignore non-primary pointerdown events', () => { + emulateClick({ + beforeMouseUp() { + window.dispatchEvent(createNewEvent('pointerdown', { target: document.body, isPrimary: false })) + }, + }) + expect(actionEventsHooks.onPointerDown).toHaveBeenCalledTimes(1) + }) + it('listen to click events', () => { emulateClick() - expect(actionEventsHooks.onClick).toHaveBeenCalledOnceWith( + expect(actionEventsHooks.onStartEvent).toHaveBeenCalledOnceWith( {}, jasmine.objectContaining({ type: 'click' }), + jasmine.any(Function), + jasmine.any(Function) + ) + }) + + it('listen to non-primary click events', () => { + // This emulates a Chrome behavior where all click events are non-primary + emulateClick({ clickEventIsPrimary: false }) + expect(actionEventsHooks.onStartEvent).toHaveBeenCalledOnceWith( + {}, + jasmine.objectContaining({ type: 'click' }), + jasmine.any(Function), jasmine.any(Function) ) }) @@ -45,23 +66,46 @@ describe('listenActionEvents', () => { it('can abort click lifecycle by returning undefined from the onPointerDown callback', () => { actionEventsHooks.onPointerDown.and.returnValue(undefined) emulateClick() - expect(actionEventsHooks.onClick).not.toHaveBeenCalled() + expect(actionEventsHooks.onStartEvent).not.toHaveBeenCalled() }) - it('passes the context created in onPointerDown to onClick', () => { + it('passes the context created in onPointerDown to onStartEvent', () => { const context = {} actionEventsHooks.onPointerDown.and.returnValue(context) emulateClick() - expect(actionEventsHooks.onClick.calls.mostRecent().args[0]).toBe(context) + expect(actionEventsHooks.onStartEvent.calls.mostRecent().args[0]).toBe(context) }) it('ignore "click" events if no "pointerdown" event happened since the previous "click" event', () => { emulateClick() - actionEventsHooks.onClick.calls.reset() + actionEventsHooks.onStartEvent.calls.reset() window.dispatchEvent(createNewEvent('click', { target: document.body })) - expect(actionEventsHooks.onClick).not.toHaveBeenCalled() + expect(actionEventsHooks.onStartEvent).not.toHaveBeenCalled() + }) + + describe('dead_click_fixes experimental feature', () => { + beforeEach(() => { + stopListenEvents() + + updateExperimentalFeatures(['dead_click_fixes']) + ;({ stop: stopListenEvents } = listenActionEvents(actionEventsHooks)) + }) + + afterEach(() => { + resetExperimentalFeatures() + }) + + it('listen to pointerup events', () => { + emulateClick() + expect(actionEventsHooks.onStartEvent).toHaveBeenCalledOnceWith( + {}, + jasmine.objectContaining({ type: 'pointerup' }), + jasmine.any(Function), + jasmine.any(Function) + ) + }) }) describe('selection change', () => { @@ -133,7 +177,7 @@ describe('listenActionEvents', () => { }) function hasSelectionChanged() { - return actionEventsHooks.onClick.calls.mostRecent().args[2]().selection + return actionEventsHooks.onStartEvent.calls.mostRecent().args[2]().selection } function emulateNodeSelection( @@ -185,11 +229,11 @@ describe('listenActionEvents', () => { expect(hasInputUserActivity()).toBe(true) }) - describe('with fix_dead_clicks_after_input flag', () => { + describe('with dead_click_fixes flag', () => { beforeEach(() => { stopListenEvents() - updateExperimentalFeatures(['fix_dead_clicks_after_input']) + updateExperimentalFeatures(['dead_click_fixes']) ;({ stop: stopListenEvents } = listenActionEvents(actionEventsHooks)) }) @@ -205,6 +249,13 @@ describe('listenActionEvents', () => { }) }) + it('without dead_click_fixes, input events that precede clicks should still be taken into account', () => { + emulateInputEvent() + emulateClick() + clock.tick(1) // run immediate timeouts + expect(hasInputUserActivity()).toBe(true) + }) + it('click and type should report an input user activity', () => { emulateClick({ beforeMouseUp() { @@ -218,16 +269,20 @@ describe('listenActionEvents', () => { window.dispatchEvent(createNewEvent('input')) } function hasInputUserActivity() { - return actionEventsHooks.onClick.calls.mostRecent().args[2]().input + return actionEventsHooks.onStartEvent.calls.mostRecent().args[2]().input } }) - function emulateClick({ beforeMouseUp, target = document.body }: { beforeMouseUp?(): void; target?: Node } = {}) { - window.dispatchEvent(createNewEvent('pointerdown', { target })) + function emulateClick({ + beforeMouseUp, + target = document.body, + clickEventIsPrimary = undefined, + }: { beforeMouseUp?(): void; target?: Node; clickEventIsPrimary?: boolean } = {}) { + window.dispatchEvent(createNewEvent('pointerdown', { target, isPrimary: true })) window.dispatchEvent(createNewEvent('mousedown', { target })) beforeMouseUp?.() - window.dispatchEvent(createNewEvent('pointerup', { target })) + window.dispatchEvent(createNewEvent('pointerup', { target, isPrimary: true })) window.dispatchEvent(createNewEvent('mouseup', { target })) - window.dispatchEvent(createNewEvent('click', { target })) + window.dispatchEvent(createNewEvent('click', { target, isPrimary: clickEventIsPrimary })) } }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts index a6306fc5b8..b247a7a041 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts @@ -1,30 +1,45 @@ -import { addEventListener, DOM_EVENT, isExperimentalFeatureEnabled, monitor } from '@datadog/browser-core' +import type { TimeStamp } from '@datadog/browser-core' +import { addEventListener, DOM_EVENT, isExperimentalFeatureEnabled, timeStampNow } from '@datadog/browser-core' export type MouseEventOnElement = MouseEvent & { target: Element } -export type GetUserActivity = () => { selection: boolean; input: boolean } +export interface UserActivity { + selection: boolean + input: boolean +} export interface ActionEventsHooks { onPointerDown: (event: MouseEventOnElement) => ClickContext | undefined - onClick: (context: ClickContext, event: MouseEventOnElement, getUserActivity: GetUserActivity) => void + onStartEvent: ( + context: ClickContext, + event: MouseEventOnElement, + getUserActivity: () => UserActivity, + getClickEventTimeStamp: () => TimeStamp | undefined + ) => void } -export function listenActionEvents({ onPointerDown, onClick }: ActionEventsHooks) { - let hasSelectionChanged = false +export function listenActionEvents({ onPointerDown, onStartEvent }: ActionEventsHooks) { let selectionEmptyAtPointerDown: boolean - let hasInputChanged = false + let userActivity: UserActivity = { + selection: false, + input: false, + } let clickContext: ClickContext | undefined const listeners = [ addEventListener( window, DOM_EVENT.POINTER_DOWN, - (event) => { - hasSelectionChanged = false - selectionEmptyAtPointerDown = isSelectionEmpty() - if (isExperimentalFeatureEnabled('fix_dead_clicks_after_input')) { - hasInputChanged = false - } - if (isMouseEventOnElement(event)) { + (event: PointerEvent) => { + if (isValidMouseEvent(event)) { + selectionEmptyAtPointerDown = isSelectionEmpty() + userActivity = { + selection: false, + input: isExperimentalFeatureEnabled('dead_click_fixes') + ? false + : // Mimics the issue that was fixed in https://github.com/DataDog/browser-sdk/pull/1968 + // The goal is to release all dead click fixes at the same time + userActivity.input, + } clickContext = onPointerDown(event) } }, @@ -36,7 +51,7 @@ export function listenActionEvents({ onPointerDown, onClick }: Act DOM_EVENT.SELECTION_CHANGE, () => { if (!selectionEmptyAtPointerDown || !isSelectionEmpty()) { - hasSelectionChanged = true + userActivity.selection = true } }, { capture: true } @@ -44,24 +59,29 @@ export function listenActionEvents({ onPointerDown, onClick }: Act addEventListener( window, - DOM_EVENT.CLICK, - (clickEvent: MouseEvent) => { - if (isMouseEventOnElement(clickEvent) && clickContext) { + isExperimentalFeatureEnabled('dead_click_fixes') ? DOM_EVENT.POINTER_UP : DOM_EVENT.CLICK, + (startEvent: MouseEvent) => { + if (isValidMouseEvent(startEvent) && clickContext) { // Use a scoped variable to make sure the value is not changed by other clicks - const userActivity = { - selection: hasSelectionChanged, - input: hasInputChanged, - } - if (!hasInputChanged) { - setTimeout( - monitor(() => { - userActivity.input = hasInputChanged - }) + const localUserActivity = userActivity + let clickEventTimeStamp: TimeStamp | undefined + onStartEvent( + clickContext, + startEvent, + () => localUserActivity, + () => clickEventTimeStamp + ) + clickContext = undefined + if (isExperimentalFeatureEnabled('dead_click_fixes')) { + addEventListener( + window, + DOM_EVENT.CLICK, + () => { + clickEventTimeStamp = timeStampNow() + }, + { capture: true, once: true } ) } - - onClick(clickContext, clickEvent, () => userActivity) - clickContext = undefined } }, { capture: true } @@ -71,7 +91,7 @@ export function listenActionEvents({ onPointerDown, onClick }: Act window, DOM_EVENT.INPUT, () => { - hasInputChanged = true + userActivity.input = true }, { capture: true } ), @@ -89,6 +109,14 @@ function isSelectionEmpty(): boolean { return !selection || selection.isCollapsed } -function isMouseEventOnElement(event: Event): event is MouseEventOnElement { - return event.target instanceof Element +function isValidMouseEvent(event: MouseEvent): event is MouseEventOnElement { + return ( + event.target instanceof Element && + // Only consider 'primary' pointer events for now. Multi-touch support could be implemented in + // the future. + // On Chrome, click events are PointerEvent with `isPrimary = false`, but we should still + // consider them valid. This could be removed when we enable the `click-action-on-pointerup` + // flag, since we won't rely on click events anymore. + (event.type === 'click' || (event as PointerEvent).isPrimary !== false) + ) } diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts index 1665fd7db2..9a9cbd5238 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts @@ -1,4 +1,4 @@ -import type { Context, Observable, Duration } from '@datadog/browser-core' +import type { Context, Duration } from '@datadog/browser-core' import { addDuration, updateExperimentalFeatures, @@ -7,7 +7,6 @@ import { timeStampNow, relativeNow, } from '@datadog/browser-core' -import type { Clock } from '../../../../../core/test/specHelper' import { createNewEvent } from '../../../../../core/test/specHelper' import type { TestSetupBuilder } from '../../../../test/specHelper' import { setup } from '../../../../test/specHelper' @@ -83,9 +82,9 @@ describe('trackClickActions', () => { }) it('starts a click action when clicking on an element', () => { - const { domMutationObservable, clock } = setupBuilder.build() + const { clock } = setupBuilder.build() const pointerDownClocks = clocksNow() - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) expect(findActionId()).not.toBeUndefined() clock.tick(EXPIRE_DELAY) const domEvent = createNewEvent('click', { target: document.createElement('button') }) @@ -109,6 +108,7 @@ describe('trackClickActions', () => { target: undefined, position: undefined, events: [domEvent], + pointerUpDelay: undefined, }, ]) }) @@ -123,8 +123,8 @@ describe('trackClickActions', () => { }) it('should set click position and target', () => { - const { domMutationObservable, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock) + const { clock } = setupBuilder.build() + emulateClick({ activity: {} }) clock.tick(EXPIRE_DELAY) expect(events[0]).toEqual( jasmine.objectContaining({ @@ -140,9 +140,9 @@ describe('trackClickActions', () => { }) it('should keep track of previously validated click actions', () => { - const { domMutationObservable, clock } = setupBuilder.build() + const { clock } = setupBuilder.build() const pointerDownStart = relativeNow() - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) clock.tick(EXPIRE_DELAY) expect(findActionId(addDuration(pointerDownStart, EMULATED_CLICK_DURATION))).not.toBeUndefined() @@ -151,7 +151,7 @@ describe('trackClickActions', () => { it('counts errors occurring during the click action', () => { const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, createFakeErrorEvent()) clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) @@ -171,9 +171,9 @@ describe('trackClickActions', () => { }) it('does not count child events unrelated to the click action', () => { - const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() + const { lifeCycle, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: RumEventType.RESOURCE, @@ -188,12 +188,10 @@ describe('trackClickActions', () => { }) it('should take the name from user-configured attribute', () => { - const { domMutationObservable, clock } = setupBuilder - .withConfiguration({ actionNameAttribute: 'data-my-custom-attribute' }) - .build() + const { clock } = setupBuilder.withConfiguration({ actionNameAttribute: 'data-my-custom-attribute' }).build() button.setAttribute('data-my-custom-attribute', 'test-1') - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) clock.tick(EXPIRE_DELAY) expect(events.length).toBe(1) @@ -202,8 +200,8 @@ describe('trackClickActions', () => { describe('without tracking frustrations', () => { it('discards any click action with a negative duration', () => { - const { domMutationObservable, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock, button, -1) + const { clock } = setupBuilder.build() + emulateClick({ activity: { delay: -1 } }) expect(findActionId()).not.toBeUndefined() clock.tick(EXPIRE_DELAY) @@ -212,8 +210,8 @@ describe('trackClickActions', () => { }) it('discards ongoing click action on view ended', () => { - const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock) + const { lifeCycle, clock } = setupBuilder.build() + emulateClick({ activity: {} }) expect(findActionId()).not.toBeUndefined() lifeCycle.notify(LifeCycleEventType.VIEW_ENDED, { @@ -226,11 +224,11 @@ describe('trackClickActions', () => { }) it('ignores any starting click action while another one is ongoing', () => { - const { domMutationObservable, clock } = setupBuilder.build() + const { clock } = setupBuilder.build() const firstPointerDownTimeStamp = timeStampNow() - emulateClickWithActivity(domMutationObservable, clock) - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) + emulateClick({ activity: {} }) clock.tick(EXPIRE_DELAY) expect(events.length).toBe(1) @@ -239,7 +237,7 @@ describe('trackClickActions', () => { it('discards a click action when nothing happens after a click', () => { const { clock } = setupBuilder.build() - emulateClickWithoutActivity(clock) + emulateClick() clock.tick(EXPIRE_DELAY) expect(events).toEqual([]) @@ -247,8 +245,8 @@ describe('trackClickActions', () => { }) it('ignores a click action if it fails to find a name', () => { - const { domMutationObservable, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock, emptyElement) + const { clock } = setupBuilder.build() + emulateClick({ activity: {}, target: emptyElement }) expect(findActionId()).toBeUndefined() clock.tick(EXPIRE_DELAY) @@ -256,9 +254,9 @@ describe('trackClickActions', () => { }) it('does not populate the frustrationTypes array', () => { - const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() + const { lifeCycle, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, createFakeErrorEvent()) clock.tick(EXPIRE_DELAY) @@ -273,8 +271,8 @@ describe('trackClickActions', () => { }) it('discards any click action with a negative duration', () => { - const { domMutationObservable, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock, button, -1) + const { clock } = setupBuilder.build() + emulateClick({ activity: { delay: -1 } }) expect(findActionId()!.length).toEqual(2) clock.tick(EXPIRE_DELAY) @@ -283,8 +281,8 @@ describe('trackClickActions', () => { }) it('ongoing click action is stopped on view end', () => { - const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock, button, BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + const { lifeCycle, clock } = setupBuilder.build() + emulateClick({ activity: { delay: BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY } }) clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) @@ -297,12 +295,12 @@ describe('trackClickActions', () => { }) it('collect click actions even if another one is ongoing', () => { - const { domMutationObservable, clock } = setupBuilder.build() + const { clock } = setupBuilder.build() const firstPointerDownTimeStamp = timeStampNow() - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) const secondPointerDownTimeStamp = timeStampNow() - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) clock.tick(EXPIRE_DELAY) expect(events.length).toBe(2) @@ -312,7 +310,7 @@ describe('trackClickActions', () => { it('collect click actions even if nothing happens after a click (dead click)', () => { const { clock } = setupBuilder.build() - emulateClickWithoutActivity(clock) + emulateClick() clock.tick(EXPIRE_DELAY) expect(events.length).toBe(1) @@ -322,7 +320,7 @@ describe('trackClickActions', () => { it('does not set a duration for dead clicks', () => { const { clock } = setupBuilder.build() - emulateClickWithoutActivity(clock) + emulateClick() clock.tick(EXPIRE_DELAY) expect(events.length).toBe(1) @@ -330,8 +328,8 @@ describe('trackClickActions', () => { }) it('collect click actions even if it fails to find a name', () => { - const { domMutationObservable, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock, emptyElement) + const { clock } = setupBuilder.build() + emulateClick({ activity: {}, target: emptyElement }) expect(findActionId()!.length).toBeGreaterThan(0) clock.tick(EXPIRE_DELAY) @@ -340,28 +338,28 @@ describe('trackClickActions', () => { describe('rage clicks', () => { it('considers a chain of three clicks or more as a single action with "rage" frustration type', () => { - const { domMutationObservable, clock } = setupBuilder.build() + const { clock } = setupBuilder.build() const firstPointerDownTimeStamp = timeStampNow() - const actionDuration = 5 - emulateClickWithActivity(domMutationObservable, clock, undefined, actionDuration) - emulateClickWithActivity(domMutationObservable, clock, undefined, actionDuration) - emulateClickWithActivity(domMutationObservable, clock, undefined, actionDuration) + const activityDelay = 5 + emulateClick({ activity: { delay: activityDelay } }) + emulateClick({ activity: { delay: activityDelay } }) + emulateClick({ activity: { delay: activityDelay } }) clock.tick(EXPIRE_DELAY) expect(events.length).toBe(1) expect(events[0].startClocks.timeStamp).toBe(addDuration(firstPointerDownTimeStamp, EMULATED_CLICK_DURATION)) expect(events[0].frustrationTypes).toEqual([FrustrationType.RAGE_CLICK]) expect(events[0].duration).toBe( - (MAX_DURATION_BETWEEN_CLICKS + 2 * actionDuration + 2 * EMULATED_CLICK_DURATION) as Duration + (MAX_DURATION_BETWEEN_CLICKS + 2 * activityDelay + 2 * EMULATED_CLICK_DURATION) as Duration ) }) it('should contain original events from of rage sequence', () => { - const { domMutationObservable, clock } = setupBuilder.build() - const actionDuration = 5 - emulateClickWithActivity(domMutationObservable, clock, undefined, actionDuration) - emulateClickWithActivity(domMutationObservable, clock, undefined, actionDuration) - emulateClickWithActivity(domMutationObservable, clock, undefined, actionDuration) + const { clock } = setupBuilder.build() + const activityDelay = 5 + emulateClick({ activity: { delay: activityDelay } }) + emulateClick({ activity: { delay: activityDelay } }) + emulateClick({ activity: { delay: activityDelay } }) clock.tick(EXPIRE_DELAY) expect(events.length).toBe(1) @@ -370,19 +368,19 @@ describe('trackClickActions', () => { }) it('aggregates frustrationTypes from all clicks', () => { - const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() + const { lifeCycle, clock } = setupBuilder.build() // Dead - emulateClickWithoutActivity(clock) + emulateClick() clock.tick(PAGE_ACTIVITY_VALIDATION_DELAY) // Error - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, createFakeErrorEvent()) clock.tick(PAGE_ACTIVITY_VALIDATION_DELAY) // Third click to make a rage click - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) clock.tick(EXPIRE_DELAY) expect(events.length).toBe(1) @@ -398,9 +396,9 @@ describe('trackClickActions', () => { describe('error clicks', () => { it('considers a "click with activity" followed by an error as a click action with "error" frustration type', () => { - const { lifeCycle, domMutationObservable, clock } = setupBuilder.build() + const { lifeCycle, clock } = setupBuilder.build() - emulateClickWithActivity(domMutationObservable, clock) + emulateClick({ activity: {} }) lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, createFakeErrorEvent()) clock.tick(EXPIRE_DELAY) @@ -411,7 +409,7 @@ describe('trackClickActions', () => { it('considers a "click without activity" followed by an error as a click action with "error" (and "dead") frustration type', () => { const { lifeCycle, clock } = setupBuilder.build() - emulateClickWithoutActivity(clock) + emulateClick() lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, createFakeErrorEvent()) clock.tick(EXPIRE_DELAY) @@ -426,33 +424,56 @@ describe('trackClickActions', () => { it('considers a "click without activity" as a dead click', () => { const { clock } = setupBuilder.build() - emulateClickWithoutActivity(clock) + emulateClick() clock.tick(EXPIRE_DELAY) expect(events.length).toBe(1) expect(events[0].frustrationTypes).toEqual([FrustrationType.DEAD_CLICK]) }) + + describe('dead_click_fixes experimental feature', () => { + beforeEach(() => { + updateExperimentalFeatures(['dead_click_fixes']) + }) + + afterEach(() => { + resetExperimentalFeatures() + }) + + it('does not consider a click with activity happening on pointerup as a dead click', () => { + const { clock } = setupBuilder.build() + + emulateClick({ activity: { on: 'pointerup' } }) + + clock.tick(EXPIRE_DELAY) + expect(events.length).toBe(1) + expect(events[0].frustrationTypes).toEqual([]) + }) + + it('reports the delay between pointerup and click event', () => { + const { clock } = setupBuilder.build() + + const pointerUpActivityDelay = 5 as Duration + emulateClick({ activity: { on: 'pointerup', delay: pointerUpActivityDelay } }) + + clock.tick(EXPIRE_DELAY) + expect(events.length).toBe(1) + expect(events[0].pointerUpDelay).toBe(pointerUpActivityDelay) + }) + }) }) }) - function emulateClickWithActivity( - domMutationObservable: Observable, - clock: Clock, - target: HTMLElement = button, - clickActionDuration: number = BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY - ) { - emulateClickWithoutActivity(clock, target) - if (clickActionDuration < 0) { - // Do not use `.tick()` here because negative clock tick does not work since jasmine 4: https://github.com/jasmine/jasmine/pull/1948 - clock.setDate(new Date(Date.now() + clickActionDuration)) - } else { - clock.tick(clickActionDuration) + function emulateClick({ + target = button, + activity, + }: { + target?: HTMLElement + activity?: { + delay?: number + on?: 'pointerup' | 'click' } - // Since we don't collect dom mutations for this test, manually dispatch one - domMutationObservable.notify() - } - - function emulateClickWithoutActivity(clock: Clock, target: HTMLElement = button) { + } = {}) { const targetPosition = target.getBoundingClientRect() const offsetX = targetPosition.width / 2 const offsetY = targetPosition.height / 2 @@ -463,11 +484,28 @@ describe('trackClickActions', () => { offsetX, offsetY, timeStamp: timeStampNow(), + isPrimary: true, } target.dispatchEvent(createNewEvent('pointerdown', eventProperties)) - clock.tick(EMULATED_CLICK_DURATION) + setupBuilder.clock!.tick(EMULATED_CLICK_DURATION) target.dispatchEvent(createNewEvent('pointerup', eventProperties)) + emulateActivityIfNeeded('pointerup') target.dispatchEvent(createNewEvent('click', eventProperties)) + emulateActivityIfNeeded('click') + + function emulateActivityIfNeeded(event: 'pointerup' | 'click') { + if (activity && (activity.on ?? 'click') === event) { + const delay = activity.delay ?? BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY + if (delay < 0) { + // Do not use `.tick()` here because negative clock tick does not work since jasmine 4: https://github.com/jasmine/jasmine/pull/1948 + setupBuilder.clock!.setDate(new Date(Date.now() + delay)) + } else { + setupBuilder.clock!.tick(delay) + } + // Since we don't collect dom mutations for this test, manually dispatch one + setupBuilder.domMutationObservable.notify() + } + } } function createFakeErrorEvent() { diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts index 6d9b48835a..abc9e37305 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts @@ -24,7 +24,7 @@ import type { ClickChain } from './clickChain' import { createClickChain } from './clickChain' import { getActionNameFromElement } from './getActionNameFromElement' import { getSelectorFromElement } from './getSelectorFromElement' -import type { MouseEventOnElement, GetUserActivity } from './listenActionEvents' +import type { MouseEventOnElement, UserActivity } from './listenActionEvents' import { listenActionEvents } from './listenActionEvents' import { computeFrustration } from './computeFrustration' @@ -51,6 +51,7 @@ export interface ClickAction { event: MouseEventOnElement frustrationTypes: FrustrationType[] events: Event[] + pointerUpDelay?: Duration } export interface ActionContexts { @@ -80,8 +81,8 @@ export function trackClickActions( const { stop: stopActionEventsListener } = listenActionEvents({ onPointerDown: (pointerDownEvent) => processPointerDown(configuration, history, pointerDownEvent), - onClick: (clickActionBase, clickEvent, getUserActivity) => - processClick( + onStartEvent: (clickActionBase, startEvent, getUserActivity, getClickEventTimeStamp) => + startClickAction( configuration, lifeCycle, domMutationObservable, @@ -89,8 +90,9 @@ export function trackClickActions( stopObservable, appendClickToClickChain, clickActionBase, - clickEvent, - getUserActivity + startEvent, + getUserActivity, + getClickEventTimeStamp ), }) @@ -145,7 +147,7 @@ function processPointerDown( return clickActionBase } -function processClick( +function startClickAction( configuration: RumConfiguration, lifeCycle: LifeCycle, domMutationObservable: Observable, @@ -153,10 +155,11 @@ function processClick( stopObservable: Observable, appendClickToClickChain: (click: Click) => void, clickActionBase: ClickActionBase, - clickEvent: MouseEventOnElement, - getUserActivity: GetUserActivity + startEvent: MouseEventOnElement, + getUserActivity: () => UserActivity, + getClickEventTimeStamp: () => TimeStamp | undefined ) { - const click = newClick(lifeCycle, history, getUserActivity, clickActionBase, clickEvent) + const click = newClick(lifeCycle, history, getUserActivity, getClickEventTimeStamp, clickActionBase, startEvent) if (configuration.trackFrustrations) { appendClickToClickChain(click) @@ -246,9 +249,10 @@ export type Click = ReturnType function newClick( lifeCycle: LifeCycle, history: ClickActionIdHistory, - getUserActivity: GetUserActivity, + getUserActivity: () => UserActivity, + getClickEventTimeStamp: () => TimeStamp | undefined, clickActionBase: ClickActionBase, - clickEvent: MouseEventOnElement + startEvent: MouseEventOnElement ) { const id = generateUUID() const startClocks = clocksNow() @@ -280,7 +284,7 @@ function newClick( } return { - event: clickEvent, + event: startEvent, stop, stopObservable, @@ -298,7 +302,7 @@ function newClick( isStopped: () => status === ClickStatus.STOPPED || status === ClickStatus.FINALIZED, - clone: () => newClick(lifeCycle, history, getUserActivity, clickActionBase, clickEvent), + clone: () => newClick(lifeCycle, history, getUserActivity, getClickEventTimeStamp, clickActionBase, startEvent), validate: (domEvents?: Event[]) => { stop() @@ -307,6 +311,7 @@ function newClick( } const { resourceCount, errorCount, longTaskCount } = eventCountsSubscription.eventCounts + const clickEventTimeStamp = getClickEventTimeStamp() const clickAction: ClickAction = assign( { type: ActionType.CLICK as const, @@ -319,8 +324,9 @@ function newClick( errorCount, longTaskCount, }, - events: domEvents ?? [clickEvent], - event: clickEvent, + events: domEvents ?? [startEvent], + event: startEvent, + pointerUpDelay: clickEventTimeStamp && elapsed(startClocks.timeStamp, clickEventTimeStamp), }, clickActionBase ) diff --git a/packages/rum-core/src/rawRumEvent.types.ts b/packages/rum-core/src/rawRumEvent.types.ts index 289a6783da..1ea9e1ce51 100644 --- a/packages/rum-core/src/rawRumEvent.types.ts +++ b/packages/rum-core/src/rawRumEvent.types.ts @@ -178,6 +178,7 @@ export interface RawRumActionEvent { x: number y: number } + pointer_up_delay?: Duration } } } diff --git a/packages/rum-core/test/specHelper.ts b/packages/rum-core/test/specHelper.ts index 0ea6140e54..57a1acc5be 100644 --- a/packages/rum-core/test/specHelper.ts +++ b/packages/rum-core/test/specHelper.ts @@ -34,7 +34,8 @@ export interface TestSetupBuilder { withFakeClock: () => TestSetupBuilder beforeBuild: (callback: BeforeBuildCallback) => TestSetupBuilder - clock?: Clock + clock: Clock | undefined + domMutationObservable: Observable cleanup: () => void build: () => TestIO @@ -129,6 +130,11 @@ export function setup(): TestSetupBuilder { } const setupBuilder: TestSetupBuilder = { + domMutationObservable, + get clock() { + return clock + }, + withFakeLocation(initialUrl: string) { fakeLocation = buildLocation(initialUrl) return setupBuilder @@ -159,9 +165,9 @@ export function setup(): TestSetupBuilder { }, withFakeClock() { clock = mockClock() - setupBuilder.clock = clock return setupBuilder }, + beforeBuild(callback: BeforeBuildCallback) { beforeBuildTasks.push(callback) return setupBuilder