From 5c15e6e7c8c3c1788806f625af5b68f965ee1eed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 6 Feb 2023 10:09:34 +0100 Subject: [PATCH 1/9] =?UTF-8?q?=F0=9F=9A=A9=F0=9F=90=9B=20enable=20fix=20#?= =?UTF-8?q?1986?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../action/computeFrustration.spec.ts | 4 +--- .../action/computeFrustration.ts | 13 +++++-------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/computeFrustration.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/action/computeFrustration.spec.ts index f47d7d085e..a44a52f151 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/computeFrustration.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/computeFrustration.spec.ts @@ -1,4 +1,4 @@ -import { ONE_SECOND, resetExperimentalFeatures, updateExperimentalFeatures } from '@datadog/browser-core' +import { ONE_SECOND } from '@datadog/browser-core' import { FrustrationType } from '../../../rawRumEvent.types' import type { Clock } from '../../../../../core/test/specHelper' import { mockClock } from '../../../../../core/test/specHelper' @@ -137,12 +137,10 @@ describe('isDead', () => { beforeEach(() => { isolatedDom = createIsolatedDom() - updateExperimentalFeatures(['dead_click_fixes']) }) afterEach(() => { isolatedDom.clear() - resetExperimentalFeatures() }) it('considers as dead when the click has no page activity', () => { diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/computeFrustration.ts b/packages/rum-core/src/domain/rumEventsCollection/action/computeFrustration.ts index af17fcca2b..565635abc7 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/computeFrustration.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/computeFrustration.ts @@ -1,4 +1,4 @@ -import { elementMatches, isExperimentalFeatureEnabled, ONE_SECOND } from '@datadog/browser-core' +import { elementMatches, ONE_SECOND } from '@datadog/browser-core' import { FrustrationType } from '../../../rawRumEvent.types' import type { Click } from './trackClickActions' @@ -53,6 +53,9 @@ const DEAD_CLICK_EXCLUDE_SELECTOR = 'input:not([type="checkbox"]):not([type="radio"]):not([type="button"]):not([type="submit"]):not([type="reset"]):not([type="range"]),' + 'textarea,' + 'select,' + + // contenteditable and their descendants don't always trigger meaningful changes when manipulated + '[contenteditable],' + + '[contenteditable] *,' + // canvas, as there is no good way to detect activity occurring on them 'canvas,' + // links that are interactive (have an href attribute) or any of their descendants, as they can @@ -64,11 +67,5 @@ export function isDead(click: Click) { if (click.hasPageActivity || click.getUserActivity().input) { return false } - return !elementMatches( - click.event.target, - isExperimentalFeatureEnabled('dead_click_fixes') - ? // contenteditable and their descendants don't always trigger meaningful changes when manipulated - `${DEAD_CLICK_EXCLUDE_SELECTOR},[contenteditable],[contenteditable] *` - : DEAD_CLICK_EXCLUDE_SELECTOR - ) + return !elementMatches(click.event.target, DEAD_CLICK_EXCLUDE_SELECTOR) } From 73898d42306b6bef5ad7016643ea33d18358004e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 6 Feb 2023 10:12:25 +0100 Subject: [PATCH 2/9] =?UTF-8?q?=F0=9F=9A=A9=F0=9F=90=9B=20enable=20fix=20#?= =?UTF-8?q?1968?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../action/listenActionEvents.spec.ts | 24 ++----------------- .../action/listenActionEvents.ts | 6 +---- 2 files changed, 3 insertions(+), 27 deletions(-) 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 bb1497fc53..95091d6f27 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts @@ -229,31 +229,11 @@ describe('listenActionEvents', () => { expect(hasInputUserActivity()).toBe(true) }) - describe('with dead_click_fixes flag', () => { - beforeEach(() => { - stopListenEvents() - - updateExperimentalFeatures(['dead_click_fixes']) - ;({ stop: stopListenEvents } = listenActionEvents(actionEventsHooks)) - }) - - afterEach(() => { - resetExperimentalFeatures() - }) - - it('input events that precede clicks should not be taken into account', () => { - emulateInputEvent() - emulateClick() - clock.tick(1) // run immediate timeouts - expect(hasInputUserActivity()).toBe(false) - }) - }) - - it('without dead_click_fixes, input events that precede clicks should still be taken into account', () => { + it('input events that precede clicks should not be taken into account', () => { emulateInputEvent() emulateClick() clock.tick(1) // run immediate timeouts - expect(hasInputUserActivity()).toBe(true) + expect(hasInputUserActivity()).toBe(false) }) it('click and type should report an input user activity', () => { diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts index b247a7a041..1966be9e59 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts @@ -34,11 +34,7 @@ export function listenActionEvents({ onPointerDown, onStartEvent } 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, + input: false, } clickContext = onPointerDown(event) } From 54c700a4e99d7f6dead35b916ddee65d9458e266 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 6 Feb 2023 10:14:13 +0100 Subject: [PATCH 3/9] =?UTF-8?q?=F0=9F=9A=A9=F0=9F=90=9B=20enable=20fix=20#?= =?UTF-8?q?1958?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/test/specHelper.ts | 4 ++ .../action/actionCollection.spec.ts | 2 +- .../action/listenActionEvents.spec.ts | 39 +------------------ .../action/listenActionEvents.ts | 17 ++++---- .../action/trackClickActions.spec.ts | 22 +++++------ packages/rum-core/test/createFakeClick.ts | 4 +- 6 files changed, 27 insertions(+), 61 deletions(-) diff --git a/packages/core/test/specHelper.ts b/packages/core/test/specHelper.ts index f31818026e..a5ecc57553 100644 --- a/packages/core/test/specHelper.ts +++ b/packages/core/test/specHelper.ts @@ -329,6 +329,10 @@ class StubXhr extends StubEventEmitter { } export function createNewEvent

>(eventName: 'click', properties?: P): MouseEvent & P +export function createNewEvent

>( + eventName: 'pointerup', + properties?: P +): PointerEvent & P export function createNewEvent(eventName: string, properties?: { [name: string]: unknown }): Event export function createNewEvent(eventName: string, properties: { [name: string]: unknown } = {}) { let event: Event 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 c7203b3459..5c2700d680 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.spec.ts @@ -26,7 +26,7 @@ describe('actionCollection', () => { it('should create action from auto action', () => { const { lifeCycle, rawRumEvents } = setupBuilder.build() - const event = createNewEvent('click', { target: document.createElement('button') }) + const event = createNewEvent('pointerup', { target: document.createElement('button') }) lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_COMPLETED, { counts: { errorCount: 10, 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 95091d6f27..4314a8aa31 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts @@ -1,4 +1,3 @@ -import { resetExperimentalFeatures, updateExperimentalFeatures } from '@datadog/browser-core' import type { Clock } from '../../../../../core/test/specHelper' import { createNewEvent, mockClock } from '../../../../../core/test/specHelper' import type { ActionEventsHooks } from './listenActionEvents' @@ -37,22 +36,11 @@ describe('listenActionEvents', () => { expect(actionEventsHooks.onPointerDown).toHaveBeenCalledTimes(1) }) - it('listen to click events', () => { + it('listen to pointerup events', () => { emulateClick() 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.objectContaining({ type: 'pointerup' }), jasmine.any(Function), jasmine.any(Function) ) @@ -85,29 +73,6 @@ describe('listenActionEvents', () => { 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', () => { let text: Text diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts index 1966be9e59..9223e24231 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts @@ -1,7 +1,7 @@ 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 MouseEventOnElement = PointerEvent & { target: Element } export interface UserActivity { selection: boolean @@ -30,7 +30,7 @@ export function listenActionEvents({ onPointerDown, onStartEvent } window, DOM_EVENT.POINTER_DOWN, (event: PointerEvent) => { - if (isValidMouseEvent(event)) { + if (isValidPointerEvent(event)) { selectionEmptyAtPointerDown = isSelectionEmpty() userActivity = { selection: false, @@ -55,9 +55,9 @@ export function listenActionEvents({ onPointerDown, onStartEvent } addEventListener( window, - isExperimentalFeatureEnabled('dead_click_fixes') ? DOM_EVENT.POINTER_UP : DOM_EVENT.CLICK, - (startEvent: MouseEvent) => { - if (isValidMouseEvent(startEvent) && clickContext) { + DOM_EVENT.POINTER_UP, + (startEvent: PointerEvent) => { + if (isValidPointerEvent(startEvent) && clickContext) { // Use a scoped variable to make sure the value is not changed by other clicks const localUserActivity = userActivity let clickEventTimeStamp: TimeStamp | undefined @@ -105,14 +105,11 @@ function isSelectionEmpty(): boolean { return !selection || selection.isCollapsed } -function isValidMouseEvent(event: MouseEvent): event is MouseEventOnElement { +function isValidPointerEvent(event: PointerEvent): 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) + event.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 3d28aff73e..9030fbc194 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts @@ -87,7 +87,7 @@ describe('trackClickActions', () => { emulateClick({ activity: {} }) expect(findActionId()).not.toBeUndefined() clock.tick(EXPIRE_DELAY) - const domEvent = createNewEvent('click', { target: document.createElement('button') }) + const domEvent = createNewEvent('pointerup', { target: document.createElement('button') }) expect(events).toEqual([ { counts: { @@ -460,16 +460,6 @@ describe('trackClickActions', () => { expect(events[0].duration).toBe(0 as Duration) }) - 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() @@ -481,6 +471,16 @@ describe('trackClickActions', () => { expect(events[0].pointerUpDelay).toBe(pointerUpActivityDelay) }) }) + + 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([]) + }) }) }) diff --git a/packages/rum-core/test/createFakeClick.ts b/packages/rum-core/test/createFakeClick.ts index 2c899a340d..4fc7e4d766 100644 --- a/packages/rum-core/test/createFakeClick.ts +++ b/packages/rum-core/test/createFakeClick.ts @@ -13,7 +13,7 @@ export function createFakeClick({ hasError?: boolean hasPageActivity?: boolean userActivity?: { selection?: boolean; input?: boolean } - event?: Partial + event?: Partial } = {}) { const stopObservable = new Observable() let isStopped = false @@ -42,7 +42,7 @@ export function createFakeClick({ addFrustration: jasmine.createSpy(), clone: jasmine.createSpy().and.callFake(clone), - event: createNewEvent('click', { + event: createNewEvent('pointerup', { clientX: 100, clientY: 100, timeStamp: timeStampNow(), From 6af98d52dcb0024a96ee9a511bc828bfee75d575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 6 Feb 2023 10:23:22 +0100 Subject: [PATCH 4/9] =?UTF-8?q?=F0=9F=94=87=20remove=20pointerup=5Fdelay?= =?UTF-8?q?=20info=20for=20#1958?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../action/actionCollection.spec.ts | 1 - .../action/actionCollection.ts | 1 - .../action/listenActionEvents.spec.ts | 1 - .../action/listenActionEvents.ts | 28 ++----------------- .../action/trackClickActions.spec.ts | 12 -------- .../action/trackClickActions.ts | 21 ++++---------- 6 files changed, 8 insertions(+), 56 deletions(-) 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 5c2700d680..d7707cb8d8 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.spec.ts @@ -87,7 +87,6 @@ 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 7a21b3059f..ac0208f383 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/actionCollection.ts @@ -79,7 +79,6 @@ 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 4314a8aa31..ee4f7532f2 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts @@ -41,7 +41,6 @@ describe('listenActionEvents', () => { expect(actionEventsHooks.onStartEvent).toHaveBeenCalledOnceWith( {}, jasmine.objectContaining({ type: 'pointerup' }), - jasmine.any(Function), jasmine.any(Function) ) }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts index 9223e24231..96217bb03e 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts @@ -1,5 +1,4 @@ -import type { TimeStamp } from '@datadog/browser-core' -import { addEventListener, DOM_EVENT, isExperimentalFeatureEnabled, timeStampNow } from '@datadog/browser-core' +import { addEventListener, DOM_EVENT } from '@datadog/browser-core' export type MouseEventOnElement = PointerEvent & { target: Element } @@ -9,12 +8,7 @@ export interface UserActivity { } export interface ActionEventsHooks { onPointerDown: (event: MouseEventOnElement) => ClickContext | undefined - onStartEvent: ( - context: ClickContext, - event: MouseEventOnElement, - getUserActivity: () => UserActivity, - getClickEventTimeStamp: () => TimeStamp | undefined - ) => void + onStartEvent: (context: ClickContext, event: MouseEventOnElement, getUserActivity: () => UserActivity) => void } export function listenActionEvents({ onPointerDown, onStartEvent }: ActionEventsHooks) { @@ -60,24 +54,8 @@ export function listenActionEvents({ onPointerDown, onStartEvent } if (isValidPointerEvent(startEvent) && clickContext) { // Use a scoped variable to make sure the value is not changed by other clicks const localUserActivity = userActivity - let clickEventTimeStamp: TimeStamp | undefined - onStartEvent( - clickContext, - startEvent, - () => localUserActivity, - () => clickEventTimeStamp - ) + onStartEvent(clickContext, startEvent, () => localUserActivity) clickContext = undefined - if (isExperimentalFeatureEnabled('dead_click_fixes')) { - addEventListener( - window, - DOM_EVENT.CLICK, - () => { - clickEventTimeStamp = timeStampNow() - }, - { capture: true, once: true } - ) - } } }, { capture: true } 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 9030fbc194..b82e452a07 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts @@ -108,7 +108,6 @@ describe('trackClickActions', () => { target: undefined, position: undefined, events: [domEvent], - pointerUpDelay: undefined, }, ]) }) @@ -459,17 +458,6 @@ describe('trackClickActions', () => { expect(events.length).toBe(1) expect(events[0].duration).toBe(0 as Duration) }) - - 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) - }) }) it('does not consider a click with activity happening on pointerup as a dead click', () => { diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts index 56d7f93fee..fa1bbc1747 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts @@ -51,7 +51,6 @@ export interface ClickAction { event: MouseEventOnElement frustrationTypes: FrustrationType[] events: Event[] - pointerUpDelay?: Duration } export interface ActionContexts { @@ -85,12 +84,7 @@ export function trackClickActions( }>({ onPointerDown: (pointerDownEvent) => processPointerDown(configuration, lifeCycle, domMutationObservable, history, pointerDownEvent), - onStartEvent: ( - { clickActionBase, hadActivityOnPointerDown }, - startEvent, - getUserActivity, - getClickEventTimeStamp - ) => + onStartEvent: ({ clickActionBase, hadActivityOnPointerDown }, startEvent, getUserActivity) => startClickAction( configuration, lifeCycle, @@ -101,8 +95,7 @@ export function trackClickActions( clickActionBase, startEvent, getUserActivity, - hadActivityOnPointerDown, - getClickEventTimeStamp + hadActivityOnPointerDown ), }) @@ -185,10 +178,9 @@ function startClickAction( clickActionBase: ClickActionBase, startEvent: MouseEventOnElement, getUserActivity: () => UserActivity, - hadActivityOnPointerDown: () => boolean, - getClickEventTimeStamp: () => TimeStamp | undefined + hadActivityOnPointerDown: () => boolean ) { - const click = newClick(lifeCycle, history, getUserActivity, getClickEventTimeStamp, clickActionBase, startEvent) + const click = newClick(lifeCycle, history, getUserActivity, clickActionBase, startEvent) if (configuration.trackFrustrations) { appendClickToClickChain(click) @@ -289,7 +281,6 @@ function newClick( lifeCycle: LifeCycle, history: ClickActionIdHistory, getUserActivity: () => UserActivity, - getClickEventTimeStamp: () => TimeStamp | undefined, clickActionBase: ClickActionBase, startEvent: MouseEventOnElement ) { @@ -341,7 +332,7 @@ function newClick( isStopped: () => status === ClickStatus.STOPPED || status === ClickStatus.FINALIZED, - clone: () => newClick(lifeCycle, history, getUserActivity, getClickEventTimeStamp, clickActionBase, startEvent), + clone: () => newClick(lifeCycle, history, getUserActivity, clickActionBase, startEvent), validate: (domEvents?: Event[]) => { stop() @@ -350,7 +341,6 @@ function newClick( } const { resourceCount, errorCount, longTaskCount } = eventCountsSubscription.eventCounts - const clickEventTimeStamp = getClickEventTimeStamp() const clickAction: ClickAction = assign( { type: ActionType.CLICK as const, @@ -365,7 +355,6 @@ function newClick( }, events: domEvents ?? [startEvent], event: startEvent, - pointerUpDelay: clickEventTimeStamp && elapsed(startClocks.timeStamp, clickEventTimeStamp), }, clickActionBase ) From 20d77667d841e6f078143506212f7bbd9c5623ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 6 Feb 2023 10:25:36 +0100 Subject: [PATCH 5/9] =?UTF-8?q?=F0=9F=9A=A9=F0=9F=90=9B=20enable=20fix=20#?= =?UTF-8?q?1979?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../action/trackClickActions.spec.ts | 36 +++++++------------ .../action/trackClickActions.ts | 24 ++++++------- 2 files changed, 24 insertions(+), 36 deletions(-) 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 b82e452a07..d644fb126e 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.spec.ts @@ -430,34 +430,24 @@ describe('trackClickActions', () => { 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 pointerdown as a dead click', () => { - const { clock } = setupBuilder.build() + it('does not consider a click with activity happening on pointerdown as a dead click', () => { + const { clock } = setupBuilder.build() - emulateClick({ activity: { on: 'pointerdown' } }) + emulateClick({ activity: { on: 'pointerdown' } }) - clock.tick(EXPIRE_DELAY) - expect(events.length).toBe(1) - expect(events[0].frustrationTypes).toEqual([]) - }) + clock.tick(EXPIRE_DELAY) + expect(events.length).toBe(1) + expect(events[0].frustrationTypes).toEqual([]) + }) - it('activity happening on pointerdown is not taken into account for the action duration', () => { - const { clock } = setupBuilder.build() + it('activity happening on pointerdown is not taken into account for the action duration', () => { + const { clock } = setupBuilder.build() - emulateClick({ activity: { on: 'pointerdown' } }) + emulateClick({ activity: { on: 'pointerdown' } }) - clock.tick(EXPIRE_DELAY) - expect(events.length).toBe(1) - expect(events[0].duration).toBe(0 as Duration) - }) + clock.tick(EXPIRE_DELAY) + expect(events.length).toBe(1) + expect(events[0].duration).toBe(0 as Duration) }) it('does not consider a click with activity happening on pointerup as a dead click', () => { diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts index fa1bbc1747..cc4bc8e54b 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts @@ -151,19 +151,17 @@ function processPointerDown( let hadActivityOnPointerDown = false - if (isExperimentalFeatureEnabled('dead_click_fixes')) { - waitPageActivityEnd( - lifeCycle, - domMutationObservable, - configuration, - (pageActivityEndEvent) => { - hadActivityOnPointerDown = pageActivityEndEvent.hadActivity - }, - // We don't care about the activity duration, we just want to know whether an activity did happen - // within the "validation delay" or not. Limit the duration so the callback is called sooner. - PAGE_ACTIVITY_VALIDATION_DELAY - ) - } + waitPageActivityEnd( + lifeCycle, + domMutationObservable, + configuration, + (pageActivityEndEvent) => { + hadActivityOnPointerDown = pageActivityEndEvent.hadActivity + }, + // We don't care about the activity duration, we just want to know whether an activity did happen + // within the "validation delay" or not. Limit the duration so the callback is called sooner. + PAGE_ACTIVITY_VALIDATION_DELAY + ) return { clickActionBase, hadActivityOnPointerDown: () => hadActivityOnPointerDown } } From f2514ecfbe8f80527ca4ebe760bb161e984c5948 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 6 Feb 2023 10:32:19 +0100 Subject: [PATCH 6/9] =?UTF-8?q?=E2=9C=85=F0=9F=94=A5=20remove=20now=20unne?= =?UTF-8?q?eded=20clock=20mock=20in=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../action/listenActionEvents.spec.ts | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) 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 ee4f7532f2..83e46dced8 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts @@ -1,5 +1,4 @@ -import type { Clock } from '../../../../../core/test/specHelper' -import { createNewEvent, mockClock } from '../../../../../core/test/specHelper' +import { createNewEvent } from '../../../../../core/test/specHelper' import type { ActionEventsHooks } from './listenActionEvents' import { listenActionEvents } from './listenActionEvents' @@ -162,16 +161,6 @@ describe('listenActionEvents', () => { }) describe('input user activity', () => { - let clock: Clock - - beforeEach(() => { - clock = mockClock() - }) - - afterEach(() => { - clock.cleanup() - }) - it('click that do not trigger an input input event should not report input user activity', () => { emulateClick() expect(hasInputUserActivity()).toBe(false) @@ -189,14 +178,12 @@ describe('listenActionEvents', () => { it('click that triggers an input event slightly after the click should report an input user activity', () => { emulateClick() emulateInputEvent() - clock.tick(1) // run immediate timeouts expect(hasInputUserActivity()).toBe(true) }) it('input events that precede clicks should not be taken into account', () => { emulateInputEvent() emulateClick() - clock.tick(1) // run immediate timeouts expect(hasInputUserActivity()).toBe(false) }) From 6a98a9480c800aff92ce4c5d37270534623f7661 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Tue, 7 Feb 2023 15:28:05 +0100 Subject: [PATCH 7/9] =?UTF-8?q?=E2=9C=85=20remove=20now=20unneeded=20exper?= =?UTF-8?q?imental=20flag=20from=20e2e=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test/e2e/scenario/recorder/recorder.scenario.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/scenario/recorder/recorder.scenario.ts b/test/e2e/scenario/recorder/recorder.scenario.ts index c411143cb8..44c196b915 100644 --- a/test/e2e/scenario/recorder/recorder.scenario.ts +++ b/test/e2e/scenario/recorder/recorder.scenario.ts @@ -719,7 +719,7 @@ describe('recorder', () => { describe('frustration records', () => { createTest('should detect a dead click and match it to mouse interaction record') - .withRum({ trackFrustrations: true, enableExperimentalFeatures: ['dead_click_fixes'] }) + .withRum({ trackFrustrations: true }) .withRumInit(initRumAndStartRecording) .withSetup(bundleSetup) .run(async ({ serverEvents }) => { @@ -743,7 +743,7 @@ describe('recorder', () => { }) createTest('should detect a rage click and match it to mouse interaction records') - .withRum({ trackFrustrations: true, enableExperimentalFeatures: ['dead_click_fixes'] }) + .withRum({ trackFrustrations: true }) .withRumInit(initRumAndStartRecording) .withSetup(bundleSetup) .withBody( From 93ab5699f7dd7fffdd3e4c75630d2576f6cd0519 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Fri, 10 Feb 2023 15:12:21 +0100 Subject: [PATCH 8/9] =?UTF-8?q?=F0=9F=91=8C=20rename=20`onStartEvent`=20to?= =?UTF-8?q?=20`onPointerUp`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../action/listenActionEvents.spec.ts | 20 +++++++++---------- .../action/listenActionEvents.ts | 6 +++--- .../action/trackClickActions.ts | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) 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 83e46dced8..5e2a53288b 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts @@ -4,14 +4,14 @@ import { listenActionEvents } from './listenActionEvents' describe('listenActionEvents', () => { let actionEventsHooks: { - onStartEvent: jasmine.Spy['onStartEvent']> + onPointerUp: jasmine.Spy['onPointerUp']> onPointerDown: jasmine.Spy['onPointerDown']> } let stopListenEvents: () => void beforeEach(() => { actionEventsHooks = { - onStartEvent: jasmine.createSpy(), + onPointerUp: jasmine.createSpy(), onPointerDown: jasmine.createSpy().and.returnValue({}), } ;({ stop: stopListenEvents } = listenActionEvents(actionEventsHooks)) @@ -37,7 +37,7 @@ describe('listenActionEvents', () => { it('listen to pointerup events', () => { emulateClick() - expect(actionEventsHooks.onStartEvent).toHaveBeenCalledOnceWith( + expect(actionEventsHooks.onPointerUp).toHaveBeenCalledOnceWith( {}, jasmine.objectContaining({ type: 'pointerup' }), jasmine.any(Function) @@ -52,23 +52,23 @@ describe('listenActionEvents', () => { it('can abort click lifecycle by returning undefined from the onPointerDown callback', () => { actionEventsHooks.onPointerDown.and.returnValue(undefined) emulateClick() - expect(actionEventsHooks.onStartEvent).not.toHaveBeenCalled() + expect(actionEventsHooks.onPointerUp).not.toHaveBeenCalled() }) - it('passes the context created in onPointerDown to onStartEvent', () => { + it('passes the context created in onPointerDown to onPointerUp', () => { const context = {} actionEventsHooks.onPointerDown.and.returnValue(context) emulateClick() - expect(actionEventsHooks.onStartEvent.calls.mostRecent().args[0]).toBe(context) + expect(actionEventsHooks.onPointerUp.calls.mostRecent().args[0]).toBe(context) }) it('ignore "click" events if no "pointerdown" event happened since the previous "click" event', () => { emulateClick() - actionEventsHooks.onStartEvent.calls.reset() + actionEventsHooks.onPointerUp.calls.reset() window.dispatchEvent(createNewEvent('click', { target: document.body })) - expect(actionEventsHooks.onStartEvent).not.toHaveBeenCalled() + expect(actionEventsHooks.onPointerUp).not.toHaveBeenCalled() }) describe('selection change', () => { @@ -140,7 +140,7 @@ describe('listenActionEvents', () => { }) function hasSelectionChanged() { - return actionEventsHooks.onStartEvent.calls.mostRecent().args[2]().selection + return actionEventsHooks.onPointerUp.calls.mostRecent().args[2]().selection } function emulateNodeSelection( @@ -200,7 +200,7 @@ describe('listenActionEvents', () => { window.dispatchEvent(createNewEvent('input')) } function hasInputUserActivity() { - return actionEventsHooks.onStartEvent.calls.mostRecent().args[2]().input + return actionEventsHooks.onPointerUp.calls.mostRecent().args[2]().input } }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts index 96217bb03e..539dde61bc 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts @@ -8,10 +8,10 @@ export interface UserActivity { } export interface ActionEventsHooks { onPointerDown: (event: MouseEventOnElement) => ClickContext | undefined - onStartEvent: (context: ClickContext, event: MouseEventOnElement, getUserActivity: () => UserActivity) => void + onPointerUp: (context: ClickContext, event: MouseEventOnElement, getUserActivity: () => UserActivity) => void } -export function listenActionEvents({ onPointerDown, onStartEvent }: ActionEventsHooks) { +export function listenActionEvents({ onPointerDown, onPointerUp }: ActionEventsHooks) { let selectionEmptyAtPointerDown: boolean let userActivity: UserActivity = { selection: false, @@ -54,7 +54,7 @@ export function listenActionEvents({ onPointerDown, onStartEvent } if (isValidPointerEvent(startEvent) && clickContext) { // Use a scoped variable to make sure the value is not changed by other clicks const localUserActivity = userActivity - onStartEvent(clickContext, startEvent, () => localUserActivity) + onPointerUp(clickContext, startEvent, () => localUserActivity) clickContext = undefined } }, diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts index cc4bc8e54b..1c72a51deb 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/trackClickActions.ts @@ -84,7 +84,7 @@ export function trackClickActions( }>({ onPointerDown: (pointerDownEvent) => processPointerDown(configuration, lifeCycle, domMutationObservable, history, pointerDownEvent), - onStartEvent: ({ clickActionBase, hadActivityOnPointerDown }, startEvent, getUserActivity) => + onPointerUp: ({ clickActionBase, hadActivityOnPointerDown }, startEvent, getUserActivity) => startClickAction( configuration, lifeCycle, From 6684f6ccb1822c172bec6fa42f31211d30149349 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Fri, 10 Feb 2023 15:41:49 +0100 Subject: [PATCH 9/9] =?UTF-8?q?=F0=9F=91=8C=20rename=20startEvent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../domain/rumEventsCollection/action/listenActionEvents.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts index 539dde61bc..bda0fa97d4 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts @@ -50,11 +50,11 @@ export function listenActionEvents({ onPointerDown, onPointerUp }: addEventListener( window, DOM_EVENT.POINTER_UP, - (startEvent: PointerEvent) => { - if (isValidPointerEvent(startEvent) && clickContext) { + (event: PointerEvent) => { + if (isValidPointerEvent(event) && clickContext) { // Use a scoped variable to make sure the value is not changed by other clicks const localUserActivity = userActivity - onPointerUp(clickContext, startEvent, () => localUserActivity) + onPointerUp(clickContext, event, () => localUserActivity) clickContext = undefined } },