From b9a75075be78ec739f4903493e7b9b672ecbeaba Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Fri, 9 Jun 2023 10:20:42 +0200 Subject: [PATCH] feat(replay): Consider `window.open` for slow clicks When a click triggers `window.open()`, do not consider it a slow click. --- .../suites/replay/slowClick/template.html | 6 ++ .../replay/slowClick/windowOpen/test.ts | 62 +++++++++++++++++++ .../src/coreHandlers/handleSlowClick.ts | 7 +++ .../src/coreHandlers/util/onWindowOpen.ts | 44 +++++++++++++ 4 files changed, 119 insertions(+) create mode 100644 packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts create mode 100644 packages/replay/src/coreHandlers/util/onWindowOpen.ts diff --git a/packages/browser-integration-tests/suites/replay/slowClick/template.html b/packages/browser-integration-tests/suites/replay/slowClick/template.html index 1cf757f7b974..763a24430b71 100644 --- a/packages/browser-integration-tests/suites/replay/slowClick/template.html +++ b/packages/browser-integration-tests/suites/replay/slowClick/template.html @@ -18,6 +18,7 @@ + Link Link external @@ -69,6 +70,11 @@

Bottom

console.log('DONE'); }, 3001); }); + document.getElementById('windowOpenButton').addEventListener('click', () => { + setTimeout(() => { + window.open('.'); + }, 3001); + }); // Do nothing on these elements document diff --git a/packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts b/packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts new file mode 100644 index 000000000000..d4cc0ae7de5a --- /dev/null +++ b/packages/browser-integration-tests/suites/replay/slowClick/windowOpen/test.ts @@ -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); +}); diff --git a/packages/replay/src/coreHandlers/handleSlowClick.ts b/packages/replay/src/coreHandlers/handleSlowClick.ts index c939a990f87a..9e486a060706 100644 --- a/packages/replay/src/coreHandlers/handleSlowClick.ts +++ b/packages/replay/src/coreHandlers/handleSlowClick.ts @@ -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; @@ -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); @@ -71,6 +77,7 @@ export function detectSlowClick( clearTimeout(scrollTimeout); obs.disconnect(); WINDOW.removeEventListener('scroll', scrollHandler); + cleanupWindowOpen(); }; } diff --git a/packages/replay/src/coreHandlers/util/onWindowOpen.ts b/packages/replay/src/coreHandlers/util/onWindowOpen.ts new file mode 100644 index 000000000000..e3b6b7ac92ed --- /dev/null +++ b/packages/replay/src/coreHandlers/util/onWindowOpen.ts @@ -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); + }; + }); +}