Skip to content

Commit

Permalink
feat(replay): Consider window.open for slow clicks (#8308)
Browse files Browse the repository at this point in the history
When a click triggers `window.open()`, do not consider it a slow click.

Closes #8301
  • Loading branch information
mydea committed Jun 19, 2023
1 parent 34bb403 commit 1bdf2d9
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
<button id="scrollLateButton">Trigger scroll late</button>
<button id="mutationIgnoreButton" class="ignore-class">Trigger scroll late</button>
<button id="mouseDownButton">Trigger mutation on mouse down</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 @@ -73,6 +74,9 @@ <h1 id="h2">Bottom</h1>
document.getElementById('mouseDownButton').addEventListener('mousedown', () => {
document.getElementById('out').innerHTML += 'mutationButton clicked<br>';
});
document.getElementById('windowOpenButton').addEventListener('click', () => {
window.open('https://example.com/', '_blank');
});

// Do nothing on these elements
document
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
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, browser }) => {
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.click');
});

// Ensure window.open() still works as expected
const context = browser.contexts()[0];
const waitForNewPage = context.waitForEvent('page');

await page.click('#windowOpenButton');

const { breadcrumbs } = getCustomRecordingEvents(await reqPromise1);

// Filter out potential blur breadcrumb, as otherwise this can be flaky
const filteredBreadcrumb = breadcrumbs.filter(breadcrumb => breadcrumb.category !== 'ui.blur');

expect(filteredBreadcrumb).toEqual([
{
category: 'ui.click',
data: {
node: {
attributes: {
id: 'windowOpenButton',
},
id: expect.any(Number),
tagName: 'button',
textContent: '****** ****',
},
nodeId: expect.any(Number),
},
message: 'body > button#windowOpenButton',
timestamp: expect.any(Number),
type: 'default',
},
]);

await waitForNewPage;

const pages = context.pages();

expect(pages.length).toBe(2);
expect(pages[1].url()).toBe('https://example.com/');
});
7 changes: 7 additions & 0 deletions packages/replay/src/coreHandlers/handleClick.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { WINDOW } from '../constants';
import type { MultiClickFrame, ReplayClickDetector, ReplayContainer, SlowClickConfig, SlowClickFrame } from '../types';
import { addBreadcrumbEvent } from './util/addBreadcrumbEvent';
import { getClickTargetNode } from './util/domUtils';
import { onWindowOpen } from './util/onWindowOpen';

type ClickBreadcrumb = Breadcrumb & {
timestamp: number;
Expand Down Expand Up @@ -68,6 +69,11 @@ export class ClickDetector implements ReplayClickDetector {
this._lastScroll = nowInSeconds();
};

const cleanupWindowOpen = onWindowOpen(() => {
// Treat window.open as mutation
this._lastMutation = nowInSeconds();
});

const clickHandler = (event: MouseEvent): void => {
if (!event.target) {
return;
Expand All @@ -94,6 +100,7 @@ export class ClickDetector implements ReplayClickDetector {
this._teardown = () => {
WINDOW.removeEventListener('scroll', scrollHandler);
WINDOW.removeEventListener('click', clickHandler);
cleanupWindowOpen();

obs.disconnect();
this._clicks = [];
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 1bdf2d9

Please sign in to comment.