⚗🐛 [RUMF-1296] use pointerup to trigger click actions (#1958)

* ♻️ [RUMF-1296] simplify a bit how we compute input user activity

This commit will help implementing click actions triggered by pointerup
behind a FF.

* ♻️✅ simplify trackClickActions tests

* 🐛 [RUMF-1296] ignore non-primary pointer events

In 'listenActionEvents', we expect that a 'click' event is preceded by
a single 'pointerdown' event. It turns out this is not always the case
on multitouch devices.

This issue would be amplified when listening to 'pointerup' events.

Let's just focus on the 'primary' pointer events. This will ignore
secondary touches until we implement proper support for them (if ever).

* ⚗🐛 [RUMF-1296] use pointerup to trigger click actions

This slightly change the action beginning, but takes any activity
produced by 'pointerup' 'mouseup' 'touchend' events into account. If
those actions don't produce any activity, the action start is likely to
be the same as before, since 'pointerup/mouseup/touchend' should be
triggered very close to the 'click' event.

* 🔊⚗ [RUMF-1296] record the delay between pointerup and click

* 🚩 rename feature flag

* 🐛 restore existing behavior for dead clicks

* 👌 rename onActionStart to onStartEvent

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

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

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

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<ActionEventsHooks<object>['onClick']>
+    onStartEvent: jasmine.Spy<ActionEventsHooks<object>['onStartEvent']>
     onPointerDown: jasmine.Spy<ActionEventsHooks<object>['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 }))
   }
 })

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<ClickContext> {
   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<ClickContext>({ onPointerDown, onClick }: ActionEventsHooks<ClickContext>) {
-  let hasSelectionChanged = false
+export function listenActionEvents<ClickContext>({ onPointerDown, onStartEvent }: ActionEventsHooks<ClickContext>) {
   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,32 +51,37 @@ export function listenActionEvents<ClickContext>({ onPointerDown, onClick }: Act
       DOM_EVENT.SELECTION_CHANGE,
       () => {
         if (!selectionEmptyAtPointerDown || !isSelectionEmpty()) {
-          hasSelectionChanged = true
+          userActivity.selection = true
         }
       },
       { capture: true }
     ),
 
     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<ClickContext>({ 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)
+  )
 }