Skip to content

Commit

Permalink
fix: do not even send heatmap with no x or y (#1621)
Browse files Browse the repository at this point in the history
  • Loading branch information
pauldambra authored Dec 19, 2024
1 parent 142dad1 commit 68f00fc
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 18 deletions.
15 changes: 1 addition & 14 deletions src/__tests__/heatmaps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,20 +94,7 @@ describe('heatmaps', () => {

jest.advanceTimersByTime(posthog.heatmaps!.flushIntervalMilliseconds + 1)

expect(beforeSendMock).toBeCalledTimes(1)
expect(beforeSendMock.mock.lastCall[0]).toMatchObject({
event: '$$heatmap',
properties: {
$heatmap_data: {
'http://replaced/': [
{
target_fixed: false,
type: 'mousemove',
},
],
},
},
})
expect(beforeSendMock).toBeCalledTimes(0)
})

it('should send rageclick events in the same area', async () => {
Expand Down
12 changes: 9 additions & 3 deletions src/heatmaps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { PostHog } from './posthog-core'
import { document, window } from './utils/globals'
import { getEventTarget, getParentElement } from './autocapture-utils'
import { HEATMAPS_ENABLED_SERVER_SIDE } from './constants'
import { isEmptyObject, isObject, isUndefined } from './utils/type-utils'
import { isEmptyObject, isNumber, isObject, isUndefined } from './utils/type-utils'
import { createLogger } from './utils/logger'
import { isElementInToolbar, isElementNode, isTag } from './utils/element-utils'
import { DeadClicksAutocapture, isDeadClicksEnabledForHeatmaps } from './extensions/dead-clicks-autocapture'
Expand Down Expand Up @@ -40,6 +40,10 @@ function elementOrParentPositionMatches(el: Element | null, matches: string[], b
return false
}

function isValidMouseEvent(e: unknown): e is MouseEvent {
return isObject(e) && 'clientX' in e && 'clientY' in e && isNumber(e.clientX) && isNumber(e.clientY)
}

export class Heatmaps {
instance: PostHog
rageclicks = new RageClick()
Expand Down Expand Up @@ -161,9 +165,10 @@ export class Heatmaps {
}

private _onClick(e: MouseEvent, type: string = 'click'): void {
if (isElementInToolbar(e.target as Element)) {
if (isElementInToolbar(e.target) || !isValidMouseEvent(e)) {
return
}

const properties = this._getProperties(e, type)

if (this.rageclicks?.isRageClick(e.clientX, e.clientY, new Date().getTime())) {
Expand All @@ -177,9 +182,10 @@ export class Heatmaps {
}

private _onMouseMove(e: Event): void {
if (isElementInToolbar(e.target)) {
if (isElementInToolbar(e.target) || !isValidMouseEvent(e)) {
return
}

clearTimeout(this._mouseMoveTimeout)

this._mouseMoveTimeout = setTimeout(() => {
Expand Down
2 changes: 1 addition & 1 deletion src/utils/element-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { TOOLBAR_CONTAINER_CLASS, TOOLBAR_ID } from '../constants'

export function isElementInToolbar(el: EventTarget | null): boolean {
if (el instanceof Element) {
// NOTE: .closest is not supported in IE11 hence the operator check
// closest isn't available in IE11, but we'll polyfill when bundling
return el.id === TOOLBAR_ID || !!el.closest?.('.' + TOOLBAR_CONTAINER_CLASS)
}
return false
Expand Down

0 comments on commit 68f00fc

Please sign in to comment.