Skip to content

Commit ba08262

Browse files
pauldambrapauldambra
authored
fix(browser): ignore dead clicks when tab visibility changes (#3274)
## Summary - When a user clicks on a page to bring it into focus (e.g. switching tabs or clicking an unfocused window), the dead click detector was incorrectly flagging these as dead clicks since no meaningful DOM mutation/scroll/selection change follows - Adds `visibilitychange` as a fourth activity signal in the dead click detector, following the same pattern as scroll, mutation, and selection change - When visibility changes within the threshold (100ms) around a click, the click is not considered dead 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 parent c59dc90 commit ba08262

File tree

6 files changed

+109
-4
lines changed

6 files changed

+109
-4
lines changed

‎.changeset/slimy-seas-travel.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'posthog-js': patch
3+
'@posthog/types': patch
4+
---
5+
6+
fix: document visibility change shoudln't capture dead click

‎packages/browser/playwright/mocked/dead-clicks.spec.ts‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,45 @@ test.describe('Dead clicks', () => {
120120
expect(deadClicks.length).toBe(1)
121121
})
122122

123+
test('does not capture dead click when visibility changes to visible after click', async ({ page, context }) => {
124+
await start(startOptions, page, context)
125+
126+
await page.resetCapturedEvents()
127+
128+
await page.locator('[data-cy-not-an-order-button]').click()
129+
130+
await page.evaluate(() => {
131+
Object.defineProperty(document, 'visibilityState', { value: 'visible', writable: true })
132+
document.dispatchEvent(new Event('visibilitychange'))
133+
})
134+
135+
await page.waitForTimeout(3500)
136+
137+
const deadClicks = (await page.capturedEvents()).filter((event) => event.event === '$dead_click')
138+
expect(deadClicks.length).toBe(0)
139+
})
140+
141+
test('does not capture dead click when visibility changes to visible just before click', async ({
142+
page,
143+
context,
144+
}) => {
145+
await start(startOptions, page, context)
146+
147+
await page.resetCapturedEvents()
148+
149+
await page.evaluate(() => {
150+
Object.defineProperty(document, 'visibilityState', { value: 'visible', writable: true })
151+
document.dispatchEvent(new Event('visibilitychange'))
152+
})
153+
154+
await page.locator('[data-cy-not-an-order-button]').click()
155+
156+
await page.waitForTimeout(3500)
157+
158+
const deadClicks = (await page.capturedEvents()).filter((event) => event.event === '$dead_click')
159+
expect(deadClicks.length).toBe(0)
160+
})
161+
123162
test('does not capture dead click for selected text', async ({ page, context }) => {
124163
await start(startOptions, page, context)
125164

‎packages/browser/src/__tests__/entrypoints/lazy-loaded-dead-clicks-autocapture.test.ts‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,23 @@ describe('LazyLoadedDeadClicksAutocapture', () => {
208208
expect(fakeInstance.capture).not.toHaveBeenCalled()
209209
})
210210

211+
it.each([
212+
{ scenario: 'visibility change after click', clickTimestamp: 900, visibilityTimestamp: 999 },
213+
{ scenario: 'visibility change just before click', clickTimestamp: 950, visibilityTimestamp: 900 },
214+
])('$scenario, not a dead click', ({ clickTimestamp, visibilityTimestamp }) => {
215+
lazyLoadedDeadClicksAutocapture['_clicks'].push({
216+
node: document.body,
217+
originalEvent: { type: 'click' } as MouseEvent,
218+
timestamp: clickTimestamp,
219+
})
220+
lazyLoadedDeadClicksAutocapture['_lastVisibilityChange'] = visibilityTimestamp
221+
222+
lazyLoadedDeadClicksAutocapture['_checkClicks']()
223+
224+
expect(lazyLoadedDeadClicksAutocapture['_clicks']).toHaveLength(0)
225+
expect(fakeInstance.capture).not.toHaveBeenCalled()
226+
})
227+
211228
it('click followed by a selection change outside of threshold, dead click', () => {
212229
lazyLoadedDeadClicksAutocapture['_clicks'].push({
213230
node: document.body,
@@ -233,6 +250,8 @@ describe('LazyLoadedDeadClicksAutocapture', () => {
233250
$dead_click_scroll_timeout: false,
234251
$dead_click_selection_changed_delay_ms: 100,
235252
$dead_click_selection_changed_timeout: true,
253+
$dead_click_visibility_changed_delay_ms: undefined,
254+
$dead_click_visibility_changed_timeout: false,
236255
$el_text: 'text',
237256
$elements: [
238257
{
@@ -274,6 +293,8 @@ describe('LazyLoadedDeadClicksAutocapture', () => {
274293
$dead_click_scroll_timeout: false,
275294
$dead_click_selection_changed_delay_ms: undefined,
276295
$dead_click_selection_changed_timeout: false,
296+
$dead_click_visibility_changed_delay_ms: undefined,
297+
$dead_click_visibility_changed_timeout: false,
277298
$el_text: 'text',
278299
$elements: [
279300
{
@@ -317,6 +338,8 @@ describe('LazyLoadedDeadClicksAutocapture', () => {
317338
$dead_click_scroll_timeout: true,
318339
$dead_click_selection_changed_delay_ms: undefined,
319340
$dead_click_selection_changed_timeout: false,
341+
$dead_click_visibility_changed_delay_ms: undefined,
342+
$dead_click_visibility_changed_timeout: false,
320343
$el_text: 'text',
321344
$elements: [
322345
{
@@ -359,6 +382,8 @@ describe('LazyLoadedDeadClicksAutocapture', () => {
359382
$dead_click_scroll_timeout: false,
360383
$dead_click_selection_changed_delay_ms: undefined,
361384
$dead_click_selection_changed_timeout: false,
385+
$dead_click_visibility_changed_delay_ms: undefined,
386+
$dead_click_visibility_changed_timeout: false,
362387
$el_text: 'text',
363388
$elements: [
364389
{

‎packages/browser/src/entrypoints/dead-clicks-autocapture.ts‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { assignableWindow, LazyLoadedDeadClicksAutocaptureInterface } from '../utils/globals'
1+
import { assignableWindow, document, LazyLoadedDeadClicksAutocaptureInterface } from '../utils/globals'
22
import { PostHog } from '../posthog-core'
33
import { isNull, isNumber, isUndefined } from '@posthog/core'
44
import { autocaptureCompatibleElements, getEventTarget } from '../autocapture-utils'
@@ -32,6 +32,7 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture
3232
private _mutationObserver: MutationObserver | undefined
3333
private _lastMutation: number | undefined
3434
private _lastSelectionChanged: number | undefined
35+
private _lastVisibilityChange: number | undefined
3536
private _clicks: DeadClickCandidate[] = []
3637
private _checkClickTimer: number | undefined
3738
private _config: Required<DeadClicksAutoCaptureConfig>
@@ -73,6 +74,7 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture
7374
this._startClickObserver()
7475
this._startScrollObserver()
7576
this._startSelectionChangedObserver()
77+
this._startVisibilityChangeObserver()
7678
this._startMutationObserver(observerTarget)
7779
}
7880

@@ -97,6 +99,7 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture
9799
assignableWindow.removeEventListener('click', this._onClick)
98100
assignableWindow.removeEventListener('scroll', this._onScroll, { capture: true })
99101
assignableWindow.removeEventListener('selectionchange', this._onSelectionChange)
102+
document?.removeEventListener('visibilitychange', this._onVisibilityChange)
100103
}
101104

102105
// eslint-disable-next-line @typescript-eslint/no-unused-vars
@@ -156,6 +159,16 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture
156159
this._lastSelectionChanged = Date.now()
157160
}
158161

162+
private _startVisibilityChangeObserver() {
163+
addEventListener(document, 'visibilitychange', this._onVisibilityChange)
164+
}
165+
166+
private _onVisibilityChange = (): void => {
167+
if (document?.visibilityState === 'visible') {
168+
this._lastVisibilityChange = Date.now()
169+
}
170+
}
171+
159172
private _ignoreClick(click: DeadClickCandidate | null): boolean {
160173
if (!click) {
161174
return true
@@ -210,6 +223,9 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture
210223
this._lastSelectionChanged && click.timestamp <= this._lastSelectionChanged
211224
? this._lastSelectionChanged - click.timestamp
212225
: undefined
226+
click.visibilityChangedDelayMs = this._lastVisibilityChange
227+
? Math.abs(click.timestamp - this._lastVisibilityChange)
228+
: undefined
213229

214230
const scrollTimeout = checkTimeout(click.scrollDelayMs, this._config.scroll_threshold_ms)
215231
const selectionChangedTimeout = checkTimeout(
@@ -227,20 +243,34 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture
227243
const hadSelectionChange =
228244
isNumber(click.selectionChangedDelayMs) &&
229245
click.selectionChangedDelayMs < this._config.selection_change_threshold_ms
246+
const hadVisibilityChange =
247+
isNumber(click.visibilityChangedDelayMs) &&
248+
click.visibilityChangedDelayMs < this._config.selection_change_threshold_ms
230249

231-
if (hadScroll || hadMutation || hadSelectionChange) {
232-
// ignore clicks that had a scroll or mutation
250+
if (hadScroll || hadMutation || hadSelectionChange || hadVisibilityChange) {
233251
continue
234252
}
235253

236-
if (scrollTimeout || mutationTimeout || absoluteTimeout || selectionChangedTimeout) {
254+
const visibilityChangedTimeout = checkTimeout(
255+
click.visibilityChangedDelayMs,
256+
this._config.selection_change_threshold_ms
257+
)
258+
259+
if (
260+
scrollTimeout ||
261+
mutationTimeout ||
262+
absoluteTimeout ||
263+
selectionChangedTimeout ||
264+
visibilityChangedTimeout
265+
) {
237266
this._onCapture(click, {
238267
$dead_click_last_mutation_timestamp: this._lastMutation,
239268
$dead_click_event_timestamp: click.timestamp,
240269
$dead_click_scroll_timeout: scrollTimeout,
241270
$dead_click_mutation_timeout: mutationTimeout,
242271
$dead_click_absolute_timeout: absoluteTimeout,
243272
$dead_click_selection_changed_timeout: selectionChangedTimeout,
273+
$dead_click_visibility_changed_timeout: visibilityChangedTimeout,
244274
})
245275
} else if (click.absoluteDelayMs < this._config.mutation_threshold_ms) {
246276
// keep waiting until next check
@@ -274,6 +304,7 @@ class LazyLoadedDeadClicksAutocapture implements LazyLoadedDeadClicksAutocapture
274304
$dead_click_mutation_delay_ms: click.mutationDelayMs,
275305
$dead_click_absolute_delay_ms: click.absoluteDelayMs,
276306
$dead_click_selection_changed_delay_ms: click.selectionChangedDelayMs,
307+
$dead_click_visibility_changed_delay_ms: click.visibilityChangedDelayMs,
277308
},
278309
{
279310
timestamp: new Date(click.timestamp),

‎packages/browser/terser-mangled-names.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,7 @@
280280
"_lastMutation",
281281
"_lastPathname",
282282
"_lastSelectionChanged",
283+
"_lastVisibilityChange",
283284
"_lazyLoadAndStart",
284285
"_lazyLoadedDeadClicksAutocapture",
285286
"_lazyLoadedSessionRecording",
@@ -515,6 +516,7 @@
515516
"_startRefreshInterval",
516517
"_startScrollObserver",
517518
"_startSelectionChangedObserver",
519+
"_startVisibilityChangeObserver",
518520
"_start_queue_if_opted_in",
519521
"_statusMatcher",
520522
"_stopBuffering",

‎packages/types/src/posthog-config.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,8 @@ export interface DeadClickCandidate {
204204
mutationDelayMs?: number
205205
// time between click and the most recent selection changed event
206206
selectionChangedDelayMs?: number
207+
// time between click and the most recent visibility change event
208+
visibilityChangedDelayMs?: number
207209
// if neither scroll nor mutation seen before threshold passed
208210
absoluteDelayMs?: number
209211
}

0 commit comments

Comments
 (0)