Skip to content

Commit 0680c3e

Browse files
committed
feat(replay): Consider window.open for slow clicks
When a click triggers `window.open()`, do not consider it a slow click.
1 parent b877c10 commit 0680c3e

File tree

4 files changed

+119
-0
lines changed

4 files changed

+119
-0
lines changed

packages/browser-integration-tests/suites/replay/slowClick/template.html

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
<button id="scrollButton">Trigger scroll</button>
1919
<button id="scrollLateButton">Trigger scroll late</button>
2020
<button id="mutationIgnoreButton" class="ignore-class">Trigger scroll late</button>
21+
<button id="windowOpenButton">Window open</button>
2122

2223
<a href="#" id="link">Link</a>
2324
<a href="#" target="_blank" id="linkExternal">Link external</a>
@@ -69,6 +70,11 @@ <h1 id="h2">Bottom</h1>
6970
console.log('DONE');
7071
}, 3001);
7172
});
73+
document.getElementById('windowOpenButton').addEventListener('click', () => {
74+
setTimeout(() => {
75+
window.open('.');
76+
}, 3001);
77+
});
7278

7379
// Do nothing on these elements
7480
document
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
import { expect } from '@playwright/test';
2+
3+
import { sentryTest } from '../../../../utils/fixtures';
4+
import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';
5+
6+
sentryTest('window.open() is considered for slow click', async ({ getLocalTestUrl, page }) => {
7+
if (shouldSkipReplayTest()) {
8+
sentryTest.skip();
9+
}
10+
11+
const reqPromise0 = waitForReplayRequest(page, 0);
12+
13+
await page.route('https://dsn.ingest.sentry.io/**/*', route => {
14+
return route.fulfill({
15+
status: 200,
16+
contentType: 'application/json',
17+
body: JSON.stringify({ id: 'test-id' }),
18+
});
19+
});
20+
21+
const url = await getLocalTestUrl({ testDir: __dirname });
22+
23+
await page.goto(url);
24+
await reqPromise0;
25+
26+
const reqPromise1 = waitForReplayRequest(page, (event, res) => {
27+
const { breadcrumbs } = getCustomRecordingEvents(res);
28+
29+
return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
30+
});
31+
32+
await page.click('#windowOpenButton');
33+
34+
const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);
35+
36+
const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
37+
38+
expect(slowClickBreadcrumbs).toEqual([
39+
{
40+
category: 'ui.slowClickDetected',
41+
data: {
42+
endReason: 'window.open',
43+
node: {
44+
attributes: {
45+
id: 'windowOpenButton',
46+
},
47+
id: expect.any(Number),
48+
tagName: 'button',
49+
textContent: '****** ****',
50+
},
51+
nodeId: expect.any(Number),
52+
timeAfterClickMs: expect.any(Number),
53+
url: 'http://sentry-test.io/index.html',
54+
},
55+
message: 'body > button#windowOpenButton',
56+
timestamp: expect.any(Number),
57+
},
58+
]);
59+
60+
expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000);
61+
expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
62+
});

packages/replay/src/coreHandlers/handleSlowClick.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { Breadcrumb } from '@sentry/types';
33
import { WINDOW } from '../constants';
44
import type { ReplayContainer, SlowClickConfig } from '../types';
55
import { addBreadcrumbEvent } from './util/addBreadcrumbEvent';
6+
import { onWindowOpen } from './util/onWindowOpen';
67

78
type ClickBreadcrumb = Breadcrumb & {
89
timestamp: number;
@@ -61,6 +62,11 @@ export function detectSlowClick(
6162

6263
WINDOW.addEventListener('scroll', scrollHandler);
6364

65+
const cleanupWindowOpen = onWindowOpen(() => {
66+
maybeHandleSlowClick(replay, clickBreadcrumb, config.threshold, config.timeout, 'window.open');
67+
cleanup();
68+
});
69+
6470
// Stop listening to scroll timeouts early
6571
const scrollTimeout = setTimeout(() => {
6672
WINDOW.removeEventListener('scroll', scrollHandler);
@@ -71,6 +77,7 @@ export function detectSlowClick(
7177
clearTimeout(scrollTimeout);
7278
obs.disconnect();
7379
WINDOW.removeEventListener('scroll', scrollHandler);
80+
cleanupWindowOpen();
7481
};
7582
}
7683

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { fill } from '@sentry/utils';
2+
3+
import { WINDOW } from '../../constants';
4+
5+
type WindowOpenHandler = () => void;
6+
7+
let handlers: undefined | WindowOpenHandler[];
8+
9+
/**
10+
* Register a handler to be called when `window.open()` is called.
11+
* Returns a cleanup function.
12+
*/
13+
export function onWindowOpen(cb: WindowOpenHandler): () => void {
14+
// Ensure to only register this once
15+
if (!handlers) {
16+
handlers = [];
17+
monkeyPatchWindowOpen();
18+
}
19+
20+
handlers.push(cb);
21+
22+
return () => {
23+
const pos = handlers ? handlers.indexOf(cb) : -1;
24+
if (pos > -1) {
25+
(handlers as WindowOpenHandler[]).splice(pos, 1);
26+
}
27+
};
28+
}
29+
30+
function monkeyPatchWindowOpen(): void {
31+
fill(WINDOW, 'open', function (originalWindowOpen: () => void): () => void {
32+
return function (...args: unknown[]): void {
33+
if (handlers) {
34+
try {
35+
handlers.forEach(handler => handler());
36+
} catch (e) {
37+
// ignore errors in here
38+
}
39+
}
40+
41+
return originalWindowOpen.apply(WINDOW, args);
42+
};
43+
});
44+
}

0 commit comments

Comments
 (0)