diff --git a/packages/browser-rum-core/src/domain/action/listenActionEvents.spec.ts b/packages/browser-rum-core/src/domain/action/listenActionEvents.spec.ts index 78a7cecfe8..c0099aced9 100644 --- a/packages/browser-rum-core/src/domain/action/listenActionEvents.spec.ts +++ b/packages/browser-rum-core/src/domain/action/listenActionEvents.spec.ts @@ -1,6 +1,6 @@ import { createNewEvent } from '@datadog/browser-core/test' import type { ActionEventsHooks } from './listenActionEvents' -import { listenActionEvents } from './listenActionEvents' +import { ACTION_SCROLL_DISTANCE_THRESHOLD, listenActionEvents } from './listenActionEvents' describe('listenActionEvents', () => { let actionEventsHooks: { @@ -239,6 +239,98 @@ describe('listenActionEvents', () => { } }) + // The movement guard (see listenActionEvents) only discards a pointerdown -> pointerup pair as a + // scroll/drag for touch/pen pointers that travel farther than ACTION_SCROLL_DISTANCE_THRESHOLD. + // The threshold sits at/above the browser's own synthetic-`click` suppression slop (~8px in + // Chromium), so the guard never discards a gesture the browser itself would have turned into a + // click — protecting against regressions on touch screens (small tap jitter) while still dropping + // scroll gestures that a WebView delivers as pointerdown -> pointerup. + describe('scroll gesture guard', () => { + it('does not trigger onPointerUp when a touch pointer moves beyond the scroll threshold', () => { + emulateGesture({ pointerType: 'touch', from: { x: 100, y: 100 }, to: { x: 100, y: 260 } }) + expect(actionEventsHooks.onPointerUp).not.toHaveBeenCalled() + }) + + it('triggers onPointerUp for a touch pointer that stays within the scroll threshold', () => { + emulateGesture({ pointerType: 'touch', from: { x: 100, y: 100 }, to: { x: 103, y: 104 } }) + expect(actionEventsHooks.onPointerUp).toHaveBeenCalledTimes(1) + }) + + it('triggers onPointerUp for a touch tap with jitter the browser still treats as a click (< slop)', () => { + // Chromium fires its synthetic `click` for touch movement up to ~8px; the guard must keep it. + emulateGesture({ pointerType: 'touch', from: { x: 100, y: 100 }, to: { x: 106, y: 104 } }) // ~7.2px + expect(actionEventsHooks.onPointerUp).toHaveBeenCalledTimes(1) + }) + + it('triggers onPointerUp for a touch pointer at exactly the threshold', () => { + emulateGesture({ pointerType: 'touch', from: { x: 100, y: 100 }, to: { x: 100, y: 100 + ACTION_SCROLL_DISTANCE_THRESHOLD } }) + expect(actionEventsHooks.onPointerUp).toHaveBeenCalledTimes(1) + }) + + it('does not trigger onPointerUp just past the threshold', () => { + emulateGesture({ pointerType: 'touch', from: { x: 100, y: 100 }, to: { x: 100, y: 101 + ACTION_SCROLL_DISTANCE_THRESHOLD } }) + expect(actionEventsHooks.onPointerUp).not.toHaveBeenCalled() + }) + + it('uses euclidean distance for diagonal movement', () => { + // dx=8, dy=8 -> ~11.3px, beyond the 10px threshold even though neither axis alone is. + emulateGesture({ pointerType: 'touch', from: { x: 100, y: 100 }, to: { x: 108, y: 108 } }) + expect(actionEventsHooks.onPointerUp).not.toHaveBeenCalled() + }) + + it('applies the guard to pen pointers (a pen drag is not a click)', () => { + emulateGesture({ pointerType: 'pen', from: { x: 100, y: 100 }, to: { x: 100, y: 260 } }) + expect(actionEventsHooks.onPointerUp).not.toHaveBeenCalled() + }) + + it('records a pen tap within the tolerance', () => { + emulateGesture({ pointerType: 'pen', from: { x: 100, y: 100 }, to: { x: 101, y: 101 } }) + expect(actionEventsHooks.onPointerUp).toHaveBeenCalledTimes(1) + }) + + it('never discards a mouse pointer, no matter how far it moved (desktop click semantics unchanged)', () => { + emulateGesture({ pointerType: 'mouse', from: { x: 100, y: 100 }, to: { x: 400, y: 400 } }) + expect(actionEventsHooks.onPointerUp).toHaveBeenCalledTimes(1) + }) + + it('does not trigger onPointerUp when the pointer is cancelled (pointerup never fires)', () => { + // A scroll on a normal page fires pointercancel instead of pointerup, so onPointerUp must not + // be called at all. The guard is irrelevant here — there is simply no pointerup. + window.dispatchEvent(createNewEvent('pointerdown', { target: document.body, isPrimary: true, clientX: 100, clientY: 100 })) + window.dispatchEvent(createNewEvent('pointercancel', { target: document.body, isPrimary: true, clientX: 100, clientY: 260 })) + expect(actionEventsHooks.onPointerUp).not.toHaveBeenCalled() + }) + + function emulateGesture({ + pointerType, + from, + to, + }: { + pointerType: string + from: { x: number; y: number } + to: { x: number; y: number } + }) { + window.dispatchEvent( + createNewEvent('pointerdown', { + target: document.body, + isPrimary: true, + pointerType, + clientX: from.x, + clientY: from.y, + }) + ) + window.dispatchEvent( + createNewEvent('pointerup', { + target: document.body, + isPrimary: true, + pointerType, + clientX: to.x, + clientY: to.y, + }) + ) + } + }) + function emulateClick({ beforeMouseUp, target = document.body, diff --git a/packages/browser-rum-core/src/domain/action/listenActionEvents.ts b/packages/browser-rum-core/src/domain/action/listenActionEvents.ts index c1c151cf02..ef2ceac458 100644 --- a/packages/browser-rum-core/src/domain/action/listenActionEvents.ts +++ b/packages/browser-rum-core/src/domain/action/listenActionEvents.ts @@ -17,6 +17,18 @@ export interface ActionEventsHooks { onPointerUp: (context: ClickContext, event: MouseEventOnElement, getUserActivity: () => UserActivity) => void } +/** + * Maximum distance (in CSS pixels) a touch/pen pointer may travel between pointerdown and + * pointerup while still being considered a click. Beyond this, the gesture is a scroll/drag and + * is not recorded as a click action. + * + * A touch that scrolls the page moves the finger noticeably (tens to hundreds of pixels), while a + * genuine tap barely moves. This threshold sits just above the platform touch slop (~8px, the + * distance at which the browser itself starts treating a touch as a scroll and stops firing its + * synthetic `click` event), leaving a small margin for the natural jitter of a real tap. + */ +export const ACTION_SCROLL_DISTANCE_THRESHOLD = 10 + export function listenActionEvents({ onPointerDown, onPointerUp }: ActionEventsHooks) { let selectionEmptyAtPointerDown: boolean let userActivity: UserActivity = { @@ -25,6 +37,7 @@ export function listenActionEvents({ onPointerDown, onPointerUp }: scroll: false, } let clickContext: ClickContext | undefined + let pointerDownEvent: MouseEventOnElement | undefined const listeners = [ addEventListener( @@ -38,6 +51,7 @@ export function listenActionEvents({ onPointerDown, onPointerUp }: input: false, scroll: false, } + pointerDownEvent = event clickContext = onPointerDown(event) } }, @@ -69,6 +83,14 @@ export function listenActionEvents({ onPointerDown, onPointerUp }: DOM_EVENT.POINTER_UP, (event: PointerEvent) => { if (isValidPointerEvent(event) && clickContext) { + if (isScrollGesture(pointerDownEvent, event)) { + // The pointer moved like a scroll/drag rather than a tap, so this is not a click. + // This notably happens in Android WebViews: when the native layer handles the scroll, + // the page receives pointerdown -> pointerup (no `pointercancel`, no synthetic `click`) + // even though the finger moved. Recording it would create a spurious click action. + clickContext = undefined + return + } // Use a scoped variable to make sure the value is not changed by other clicks const localUserActivity = userActivity onPointerUp(clickContext, event, () => localUserActivity) @@ -108,3 +130,17 @@ function isValidPointerEvent(event: PointerEvent): event is MouseEventOnElement event.isPrimary !== false ) } + +/** + * Tells whether a pointerdown -> pointerup sequence is a scroll/drag rather than a click, based on + * how far the pointer travelled. Only applies to touch and pen pointers: mouse clicks (which do not + * scroll the page) keep their existing behavior. + */ +function isScrollGesture(pointerDownEvent: MouseEventOnElement | undefined, pointerUpEvent: MouseEventOnElement) { + if (!pointerDownEvent || pointerUpEvent.pointerType === 'mouse') { + return false + } + const deltaX = pointerUpEvent.clientX - pointerDownEvent.clientX + const deltaY = pointerUpEvent.clientY - pointerDownEvent.clientY + return Math.sqrt(deltaX * deltaX + deltaY * deltaY) > ACTION_SCROLL_DISTANCE_THRESHOLD +} diff --git a/packages/browser-rum-core/src/domain/action/trackClickActions.spec.ts b/packages/browser-rum-core/src/domain/action/trackClickActions.spec.ts index af2294b97c..d6976d366e 100644 --- a/packages/browser-rum-core/src/domain/action/trackClickActions.spec.ts +++ b/packages/browser-rum-core/src/domain/action/trackClickActions.spec.ts @@ -561,6 +561,40 @@ describe('trackClickActions', () => { }) }) + // Regression: on Android WebViews, a touch scroll gesture that starts on an element is + // delivered to the page as pointerdown -> pointerup (the finger moves while scrolling, so + // the browser suppresses its own synthetic `click` event, and no `pointercancel` is fired + // as it would be for an in-page scroll). The SDK must not record such a gesture as a click. + describe('scroll gesture (touch drag)', () => { + it('does not create a click action when the pointer moves like a scroll (no `click`, no `pointercancel`)', () => { + startClickActionsTracking() + + emulateScrollGesture() + clock.tick(EXPIRE_DELAY) + + expect(events).toEqual([]) + }) + + it('does not create a click action even if the `scroll` event is delayed until after pointerup', () => { + startClickActionsTracking() + + emulateScrollGesture({ scrollAfterPointerUp: true }) + clock.tick(EXPIRE_DELAY) + + expect(events).toEqual([]) + }) + + it('still records a genuine tap (pointer stays within the tap tolerance)', () => { + startClickActionsTracking() + + emulateScrollGesture({ moveBy: 2, activity: true }) + clock.tick(EXPIRE_DELAY) + + expect(events.length).toBe(1) + expect(events[0].name).toBe('Click me') + }) + }) + function emulateClick({ target = button, activity, @@ -608,6 +642,44 @@ describe('trackClickActions', () => { } } + // Emulates a touch scroll gesture the way an Android WebView delivers it to the page: a + // pointerdown, then a pointerup at a moved location, WITHOUT a `pointercancel` and (by + // default) WITHOUT the browser's synthetic `click` event, which it suppresses because the + // pointer moved. Optionally emits the `scroll` DOM event after pointerup to reproduce the + // "delayed scroll" variant. + function emulateScrollGesture({ + moveBy = 120, + scrollAfterPointerUp = false, + activity = false, + }: { moveBy?: number; scrollAfterPointerUp?: boolean; activity?: boolean } = {}) { + const targetPosition = button.getBoundingClientRect() + const clientX = targetPosition.left + targetPosition.width / 2 + const clientY = targetPosition.top + targetPosition.height / 2 + + button.dispatchEvent( + createNewEvent('pointerdown', { target: button, clientX, clientY, isPrimary: true, timeStamp: relativeNow() }) + ) + clock.tick(EMULATED_CLICK_DURATION) + // Finger has moved down the screen (scrolling), so pointerup lands away from pointerdown. + button.dispatchEvent( + createNewEvent('pointerup', { + target: button, + clientX, + clientY: clientY + moveBy, + isPrimary: true, + timeStamp: relativeNow(), + }) + ) + if (scrollAfterPointerUp) { + window.dispatchEvent(createNewEvent('scroll')) + } + if (activity) { + // A genuine tap on a button typically triggers some page activity. + clock.tick(BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY) + domMutationObservable.notify([createMutationRecord()]) + } + } + function createFakeErrorEvent() { return { type: RumEventType.ERROR, action: { id: findActionId() } } as AssembledRumEvent }