diff --git a/dev-packages/browser-integration-tests/suites/feedback/screenshotCapture/init.js b/dev-packages/browser-integration-tests/suites/feedback/screenshotCapture/init.js new file mode 100644 index 000000000000..94453fb8e47a --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/feedback/screenshotCapture/init.js @@ -0,0 +1,7 @@ +import * as Sentry from '@sentry/browser'; +import { feedbackIntegration } from '@sentry/browser'; + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + integrations: [feedbackIntegration({ colorScheme: 'light' })], +}); diff --git a/dev-packages/browser-integration-tests/suites/feedback/screenshotCapture/test.ts b/dev-packages/browser-integration-tests/suites/feedback/screenshotCapture/test.ts new file mode 100644 index 000000000000..2a4a25f8ca75 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/feedback/screenshotCapture/test.ts @@ -0,0 +1,56 @@ +import { expect } from '@playwright/test'; +import { sentryTest } from '../../../utils/fixtures'; +import { shouldSkipFeedbackTest } from '../../../utils/helpers'; + +type CaptureWindow = Window & { framesWithoutFormAtCapture?: number }; + +// Whether the capture still shows the form depends on how busy the machine is, so this counts the frames the page +// drew with the form hidden by the time the capture is requested instead. +sentryTest( + 'draws two frames without the feedback form before it asks for the screen capture', + async ({ getLocalTestUrl, page, browserName }) => { + sentryTest.skip(shouldSkipFeedbackTest()); + sentryTest.skip(browserName !== 'chromium', 'The canvas screen-capture fixture requires Chromium.'); + + await page.addInitScript(() => { + // The fixture runs over HTTP, but screenshot capture is stubbed below. + Object.defineProperty(window, 'isSecureContext', { value: true }); + + let framesWithoutForm = 0; + const countFrame = (): void => { + const form = document + .getElementById('sentry-feedback') + ?.shadowRoot?.querySelector('dialog.dialog')?.parentElement; + if (form?.style.display === 'none') { + framesWithoutForm += 1; + } + requestAnimationFrame(countFrame); + }; + requestAnimationFrame(countFrame); + + Object.defineProperty(navigator, 'mediaDevices', { + value: { + getDisplayMedia: async () => { + (window as CaptureWindow).framesWithoutFormAtCapture = framesWithoutForm; + const canvas = document.createElement('canvas'); + canvas.width = 1600; + canvas.height = 1000; + const ctx = canvas.getContext('2d')!; + ctx.fillStyle = 'white'; + ctx.fillRect(0, 0, canvas.width, canvas.height); + return canvas.captureStream(); + }, + }, + configurable: true, + }); + }); + + await page.goto(await getLocalTestUrl({ testDir: __dirname, handleLazyLoadedFeedback: true })); + await page.getByRole('button', { name: 'Report a Bug', exact: true }).click(); + await page.getByRole('button', { name: 'Add a screenshot', exact: true }).click(); + await expect(page.locator('#foreground')).toBeVisible(); + + const framesWithoutForm = await page.evaluate(() => (window as CaptureWindow).framesWithoutFormAtCapture); + expect(framesWithoutForm).toBeGreaterThanOrEqual(2); + }, +); diff --git a/packages/feedback/src/screenshot/components/useTakeScreenshot.tsx b/packages/feedback/src/screenshot/components/useTakeScreenshot.tsx index 220c2fdaa4b0..19ef97edd32e 100644 --- a/packages/feedback/src/screenshot/components/useTakeScreenshot.tsx +++ b/packages/feedback/src/screenshot/components/useTakeScreenshot.tsx @@ -14,6 +14,28 @@ interface Props { type UseTakeScreenshot = ({ onBeforeScreenshot, onScreenshot, onAfterScreenshot, onError }: Props) => void; +const FRAMES_BEFORE_CAPTURE = 2; +// Safari drops the user gesture that `getDisplayMedia` needs 1s after the click, so stay well below that. +const MAX_WAIT_BEFORE_CAPTURE_MS = 300; + +/** Resolves once `frames` animation frames have been drawn, or after `timeout` ms if the page draws none. */ +function waitForFrames(frames: number, timeout: number): Promise { + return new Promise(resolve => { + const timeoutId = setTimeout(resolve, timeout); + let remaining = frames; + const onFrame = (): void => { + remaining -= 1; + if (remaining > 0) { + WINDOW.requestAnimationFrame(onFrame); + return; + } + clearTimeout(timeoutId); + resolve(); + }; + WINDOW.requestAnimationFrame(onFrame); + }); +} + export function useTakeScreenshotFactory({ hooks }: FactoryParams): UseTakeScreenshot { function useDpi(): number { const [dpi, setDpi] = hooks.useState(WINDOW.devicePixelRatio ?? 1); @@ -38,6 +60,10 @@ export function useTakeScreenshotFactory({ hooks }: FactoryParams): UseTakeScree const takeScreenshot = async (): Promise => { onBeforeScreenshot(); + // The capture starts from the last frame the browser painted, which can still show the form that + // `onBeforeScreenshot` just hid. A tab that draws no frames (e.g. a background tab) must not hold it forever. + await waitForFrames(FRAMES_BEFORE_CAPTURE, MAX_WAIT_BEFORE_CAPTURE_MS); + // Chrome will animate a top-bar which can shrink the window height by a // few pixels. The exact amount depends on how fast it takes to exec // the onloadedmetadata callback. diff --git a/packages/feedback/test/screenshot/components/useTakeScreenshot.test.ts b/packages/feedback/test/screenshot/components/useTakeScreenshot.test.ts new file mode 100644 index 000000000000..308deeead7b3 --- /dev/null +++ b/packages/feedback/test/screenshot/components/useTakeScreenshot.test.ts @@ -0,0 +1,71 @@ +/** + * @vitest-environment jsdom + */ +import { h, render } from 'preact'; +import * as hooks from 'preact/hooks'; +import { act } from 'preact/test-utils'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { useTakeScreenshotFactory } from '../../../src/screenshot/components/useTakeScreenshot'; + +describe('useTakeScreenshot', () => { + const getDisplayMedia = vi.fn(); + const onBeforeScreenshot = vi.fn(); + const onError = vi.fn(); + let pendingFrames: FrameRequestCallback[] = []; + + /** Runs the callbacks queued so far, like the browser does at the start of one frame. */ + async function drawFrame(): Promise { + pendingFrames.splice(0).forEach(callback => callback(0)); + await vi.advanceTimersByTimeAsync(0); + } + + function mountHook(): void { + const useTakeScreenshot = useTakeScreenshotFactory({ hooks }); + const Probe = (): null => { + useTakeScreenshot({ onBeforeScreenshot, onScreenshot: vi.fn(), onAfterScreenshot: vi.fn(), onError }); + return null; + }; + act(() => { + render(h(Probe, null), document.body); + }); + } + + beforeEach(() => { + pendingFrames = []; + getDisplayMedia.mockRejectedValue(new Error('NotAllowedError')); + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }); + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => pendingFrames.push(callback)); + vi.stubGlobal('matchMedia', () => ({ addEventListener: vi.fn(), removeEventListener: vi.fn() })); + Object.defineProperty(navigator, 'mediaDevices', { value: { getDisplayMedia }, configurable: true }); + }); + + afterEach(() => { + render(null, document.body); + vi.unstubAllGlobals(); + vi.useRealTimers(); + vi.clearAllMocks(); + }); + + it('requests the capture only after the page has drawn two frames without the form', async () => { + mountHook(); + + expect(onBeforeScreenshot).toHaveBeenCalledTimes(1); + expect(getDisplayMedia).not.toHaveBeenCalled(); + + await drawFrame(); + expect(getDisplayMedia).not.toHaveBeenCalled(); + + await drawFrame(); + expect(getDisplayMedia).toHaveBeenCalledTimes(1); + }); + + it('requests the capture after 300ms when the page draws no frames', async () => { + mountHook(); + + await vi.advanceTimersByTimeAsync(299); + expect(getDisplayMedia).not.toHaveBeenCalled(); + + await vi.advanceTimersByTimeAsync(1); + expect(getDisplayMedia).toHaveBeenCalledTimes(1); + }); +});