🐛 [RUMF-1296][RUMF-1293] fix dead click computation

packages/core/test/specHelper.ts

@@ -329,6 +329,10 @@ class StubXhr extends StubEventEmitter {
 }
 
 export function createNewEvent<P extends Record<string, unknown>>(eventName: 'click', properties?: P): MouseEvent & P
+export function createNewEvent<P extends Record<string, unknown>>(
+  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

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,
@@ -87,7 +87,6 @@ describe('actionCollection', () => {
             x: 1,
             y: 2,
           },
-          pointer_up_delay: undefined,
         },
       },
     })

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,
           },
         },
       }

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', () => {

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)
 }

packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.spec.ts

@@ -1,19 +1,17 @@
-import { resetExperimentalFeatures, updateExperimentalFeatures } from '@datadog/browser-core'
-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'
 
 describe('listenActionEvents', () => {
   let actionEventsHooks: {
-    onStartEvent: jasmine.Spy<ActionEventsHooks<object>['onStartEvent']>
+    onPointerUp: jasmine.Spy<ActionEventsHooks<object>['onPointerUp']>
     onPointerDown: jasmine.Spy<ActionEventsHooks<object>['onPointerDown']>
   }
   let stopListenEvents: () => void
 
   beforeEach(() => {
     actionEventsHooks = {
-      onStartEvent: jasmine.createSpy(),
+      onPointerUp: jasmine.createSpy(),
       onPointerDown: jasmine.createSpy().and.returnValue({}),
     }
     ;({ stop: stopListenEvents } = listenActionEvents(actionEventsHooks))
@@ -37,23 +35,11 @@ describe('listenActionEvents', () => {
     expect(actionEventsHooks.onPointerDown).toHaveBeenCalledTimes(1)
   })
 
-  it('listen to click events', () => {
+  it('listen to pointerup events', () => {
     emulateClick()
-    expect(actionEventsHooks.onStartEvent).toHaveBeenCalledOnceWith(
+    expect(actionEventsHooks.onPointerUp).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.objectContaining({ type: 'pointerup' }),
       jasmine.any(Function)
     )
   })
@@ -66,46 +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()
-  })
-
-  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)
-      )
-    })
+    expect(actionEventsHooks.onPointerUp).not.toHaveBeenCalled()
   })
 
   describe('selection change', () => {
@@ -177,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(
@@ -198,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)
@@ -225,35 +178,13 @@ 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)
     })
 
-    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', () => {
@@ -269,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
     }
   })
 

packages/rum-core/src/domain/rumEventsCollection/action/listenActionEvents.ts

@@ -1,23 +1,17 @@
-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 = MouseEvent & { target: Element }
+export type MouseEventOnElement = PointerEvent & { target: Element }
 
 export interface UserActivity {
   selection: boolean
   input: boolean
 }
 export interface ActionEventsHooks<ClickContext> {
   onPointerDown: (event: MouseEventOnElement) => ClickContext | undefined
-  onStartEvent: (
-    context: ClickContext,
-    event: MouseEventOnElement,
-    getUserActivity: () => UserActivity,
-    getClickEventTimeStamp: () => TimeStamp | undefined
-  ) => void
+  onPointerUp: (context: ClickContext, event: MouseEventOnElement, getUserActivity: () => UserActivity) => void
 }
 
-export function listenActionEvents<ClickContext>({ onPointerDown, onStartEvent }: ActionEventsHooks<ClickContext>) {
+export function listenActionEvents<ClickContext>({ onPointerDown, onPointerUp }: ActionEventsHooks<ClickContext>) {
   let selectionEmptyAtPointerDown: boolean
   let userActivity: UserActivity = {
     selection: false,
@@ -30,15 +24,11 @@ export function listenActionEvents<ClickContext>({ onPointerDown, onStartEvent }
       window,
       DOM_EVENT.POINTER_DOWN,
       (event: PointerEvent) => {
-        if (isValidMouseEvent(event)) {
+        if (isValidPointerEvent(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,
+            input: false,
           }
           clickContext = onPointerDown(event)
         }
@@ -59,29 +49,13 @@ export function listenActionEvents<ClickContext>({ 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,
+      (event: PointerEvent) => {
+        if (isValidPointerEvent(event) && 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
-          )
+          onPointerUp(clickContext, event, () => localUserActivity)
           clickContext = undefined
-          if (isExperimentalFeatureEnabled('dead_click_fixes')) {
-            addEventListener(
-              window,
-              DOM_EVENT.CLICK,
-              () => {
-                clickEventTimeStamp = timeStampNow()
-              },
-              { capture: true, once: true }
-            )
-          }
         }
       },
       { capture: true }
@@ -109,14 +83,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
   )
 }