Skip to content

Commit

Permalink
feat(replay): Consider window.open for slow clicks
Browse files Browse the repository at this point in the history
When a click triggers `window.open()`, do not consider it a slow click.
  • Loading branch information
mydea committed Jun 9, 2023
1 parent 8934ccc commit 8f9b822
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
<button id="scrollButton">Trigger scroll</button>
<button id="scrollLateButton">Trigger scroll late</button>
<button id="mutationIgnoreButton" class="ignore-class">Trigger scroll late</button>
<button id="windowOpenButton">Window open</button>

<a href="#" id="link">Link</a>
<a href="#" target="_blank" id="linkExternal">Link external</a>
Expand Down Expand Up @@ -69,6 +70,11 @@ <h1 id="h2">Bottom</h1>
console.log('DONE');
}, 3001);
});
document.getElementById('windowOpenButton').addEventListener('click', () => {
setTimeout(() => {
window.open('.');
}, 3001);
});

// Do nothing on these elements
document
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { expect } from '@playwright/test';

import { sentryTest } from '../../../../utils/fixtures';
import { getCustomRecordingEvents, shouldSkipReplayTest, waitForReplayRequest } from '../../../../utils/replayHelpers';

sentryTest('window.open() is considered for slow click', async ({ getLocalTestUrl, page }) => {
if (shouldSkipReplayTest()) {
sentryTest.skip();
}

const reqPromise0 = waitForReplayRequest(page, 0);

await page.route('https://dsn.ingest.sentry.io/**/*', route => {
return route.fulfill({
status: 200,
contentType: 'application/json',
body: JSON.stringify({ id: 'test-id' }),
});
});

const url = await getLocalTestUrl({ testDir: __dirname });

await page.goto(url);
await reqPromise0;

const reqPromise1 = waitForReplayRequest(page, (event, res) => {
const { breadcrumbs } = getCustomRecordingEvents(res);

return breadcrumbs.some(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');
});

await page.click('#windowOpenButton');

const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);

const slowClickBreadcrumbs = breadcrumbs.filter(breadcrumb => breadcrumb.category === 'ui.slowClickDetected');

expect(slowClickBreadcrumbs).toEqual([
{
category: 'ui.slowClickDetected',
data: {
endReason: 'window.open',
node: {
attributes: {
id: 'windowOpenButton',
},
id: expect.any(Number),
tagName: 'button',
textContent: '****** ****',
},
nodeId: expect.any(Number),
timeAfterClickMs: expect.any(Number),
url: 'http://sentry-test.io/index.html',
},
message: 'body > button#windowOpenButton',
timestamp: expect.any(Number),
},
]);

expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeGreaterThan(3000);
expect(slowClickBreadcrumbs[0]?.data?.timeAfterClickMs).toBeLessThan(3100);
});
7 changes: 7 additions & 0 deletions packages/replay/src/coreHandlers/handleSlowClick.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { Breadcrumb } from '@sentry/types';
import { WINDOW } from '../constants';
import type { ReplayContainer, SlowClickConfig } from '../types';
import { addBreadcrumbEvent } from './util/addBreadcrumbEvent';
import { onWindowOpen } from './util/onWindowOpen';

type ClickBreadcrumb = Breadcrumb & {
timestamp: number;
Expand Down Expand Up @@ -61,6 +62,11 @@ export function detectSlowClick(

WINDOW.addEventListener('scroll', scrollHandler);

const cleanupWindowOpen = onWindowOpen(() => {
maybeHandleSlowClick(replay, clickBreadcrumb, config.threshold, config.timeout, 'window.open');
cleanup();
});

// Stop listening to scroll timeouts early
const scrollTimeout = setTimeout(() => {
WINDOW.removeEventListener('scroll', scrollHandler);
Expand All @@ -71,6 +77,7 @@ export function detectSlowClick(
clearTimeout(scrollTimeout);
obs.disconnect();
WINDOW.removeEventListener('scroll', scrollHandler);
cleanupWindowOpen();
};
}

Expand Down
44 changes: 44 additions & 0 deletions packages/replay/src/coreHandlers/util/onWindowOpen.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { fill } from '@sentry/utils';

import { WINDOW } from '../../constants';

type WindowOpenHandler = () => void;

let handlers: undefined | WindowOpenHandler[];

/**
* Register a handler to be called when `window.open()` is called.
* Returns a cleanup function.
*/
export function onWindowOpen(cb: WindowOpenHandler): () => void {
// Ensure to only register this once
if (!handlers) {
handlers = [];
monkeyPatchWindowOpen();
}

handlers.push(cb);

return () => {
const pos = handlers ? handlers.indexOf(cb) : -1;
if (pos > -1) {
(handlers as WindowOpenHandler[]).splice(pos, 1);
}
};
}

function monkeyPatchWindowOpen(): void {
fill(WINDOW, 'open', function (originalWindowOpen: () => void): () => void {
return function (...args: unknown[]): void {
if (handlers) {
try {
handlers.forEach(handler => handler());
} catch (e) {
// ignore errors in here
}
}

return originalWindowOpen.apply(WINDOW, args);
};
});
}

0 comments on commit 8f9b822

Please sign in to comment.