From 4ca36454c3d5ff5df4455bfffd6ef82d29193e66 Mon Sep 17 00:00:00 2001 From: jrandolf <101637635+jrandolf@users.noreply.github.com> Date: Fri, 22 Sep 2023 04:48:01 -0700 Subject: [PATCH] chore: set allowViewportExpansion to captureBeyondViewport (#10992) --- .../puppeteer-core/src/api/ElementHandle.ts | 5 +-- packages/puppeteer-core/src/api/Page.ts | 2 +- packages/puppeteer-core/src/bidi/Page.ts | 13 +++++- packages/puppeteer-core/src/cdp/Page.ts | 41 +++++++++++++++++- test/TestExpectations.json | 6 +++ .../screenshot-offscreen-clip-2.png | Bin 0 -> 188 bytes .../screenshot-offscreen-clip-2.png | Bin 0 -> 204 bytes test/src/screenshot.spec.ts | 15 +++++++ 8 files changed, 75 insertions(+), 7 deletions(-) create mode 100644 test/golden-chrome/screenshot-offscreen-clip-2.png create mode 100644 test/golden-firefox/screenshot-offscreen-clip-2.png diff --git a/packages/puppeteer-core/src/api/ElementHandle.ts b/packages/puppeteer-core/src/api/ElementHandle.ts index 6a303b952c9e9..77c3973630e7b 100644 --- a/packages/puppeteer-core/src/api/ElementHandle.ts +++ b/packages/puppeteer-core/src/api/ElementHandle.ts @@ -1338,7 +1338,7 @@ export abstract class ElementHandle< const { scrollIntoView = true, captureBeyondViewport = true, - allowViewportExpansion = true, + allowViewportExpansion = captureBeyondViewport, } = options; let clip = await this.#nonEmptyVisibleBoundingBox(); @@ -1347,7 +1347,7 @@ export abstract class ElementHandle< // eslint-disable-next-line @typescript-eslint/no-unused-vars await using _ = - (captureBeyondViewport || allowViewportExpansion) && clip + allowViewportExpansion && clip ? await page._createTemporaryViewportContainingBox(clip) : null; @@ -1373,7 +1373,6 @@ export abstract class ElementHandle< return await page.screenshot({ ...options, captureBeyondViewport: false, - allowViewportExpansion: false, clip, }); } diff --git a/packages/puppeteer-core/src/api/Page.ts b/packages/puppeteer-core/src/api/Page.ts index 23f6035a35384..d8f62dc5f4ea0 100644 --- a/packages/puppeteer-core/src/api/Page.ts +++ b/packages/puppeteer-core/src/api/Page.ts @@ -2351,7 +2351,7 @@ export abstract class Page extends EventEmitter { } options.captureBeyondViewport ??= true; - options.allowViewportExpansion ??= true; + options.allowViewportExpansion ??= options.captureBeyondViewport; options.clip = options.clip && roundClip(normalizeClip(options.clip)); await using stack = new AsyncDisposableStack(); diff --git a/packages/puppeteer-core/src/bidi/Page.ts b/packages/puppeteer-core/src/bidi/Page.ts index 3a4aeafe298dc..0ca50ee432179 100644 --- a/packages/puppeteer-core/src/bidi/Page.ts +++ b/packages/puppeteer-core/src/bidi/Page.ts @@ -627,7 +627,12 @@ export class BidiPage extends Page { override async screenshot( options: Readonly = {} ): Promise { - const {clip, type, captureBeyondViewport} = options; + const { + clip, + type, + captureBeyondViewport, + allowViewportExpansion = true, + } = options; if (captureBeyondViewport) { throw new Error(`BiDi does not support 'captureBeyondViewport'.`); } @@ -648,7 +653,11 @@ export class BidiPage extends Page { if (clip?.scale !== undefined) { throw new Error(`BiDi does not support 'scale' in 'clip'.`); } - return await super.screenshot({...options, captureBeyondViewport: false}); + return await super.screenshot({ + ...options, + captureBeyondViewport, + allowViewportExpansion: captureBeyondViewport ?? allowViewportExpansion, + }); } override async _screenshot( diff --git a/packages/puppeteer-core/src/cdp/Page.ts b/packages/puppeteer-core/src/cdp/Page.ts index 0250280925873..f998c0c778fa4 100644 --- a/packages/puppeteer-core/src/cdp/Page.ts +++ b/packages/puppeteer-core/src/cdp/Page.ts @@ -35,6 +35,7 @@ import { type NewDocumentScriptEvaluation, type ScreenshotOptions, type WaitTimeoutOptions, + type ScreenshotClip, } from '../api/Page.js'; import { ConsoleMessage, @@ -1044,7 +1045,7 @@ export class CdpPage extends Page { omitBackground, optimizeForSpeed, quality, - clip, + clip: userClip, type, captureBeyondViewport, } = options; @@ -1057,6 +1058,22 @@ export class CdpPage extends Page { }); } + let clip = userClip; + if (clip && !captureBeyondViewport) { + const viewport = await this.mainFrame() + .isolatedRealm() + .evaluate(() => { + const { + height, + pageLeft: x, + pageTop: y, + width, + } = window.visualViewport!; + return {x, y, height, width}; + }); + clip = getIntersectionRect(clip, viewport); + } + const {data} = await this.#client.send('Page.captureScreenshot', { format: type, optimizeForSpeed, @@ -1204,3 +1221,25 @@ const supportedMetrics = new Set([ 'JSHeapUsedSize', 'JSHeapTotalSize', ]); + +/** @see https://w3c.github.io/webdriver-bidi/#rectangle-intersection */ +function getIntersectionRect( + clip: Readonly, + viewport: Readonly +): ScreenshotClip { + // Note these will already be normalized. + const x = Math.max(clip.x, viewport.x); + const y = Math.max(clip.y, viewport.y); + return { + x, + y, + width: Math.max( + Math.min(clip.x + clip.width, viewport.x + viewport.width) - x, + 0 + ), + height: Math.max( + Math.min(clip.y + clip.height, viewport.y + viewport.height) - y, + 0 + ), + }; +} diff --git a/test/TestExpectations.json b/test/TestExpectations.json index fff2210108611..77d2ef40c2a3a 100644 --- a/test/TestExpectations.json +++ b/test/TestExpectations.json @@ -1265,6 +1265,12 @@ "parameters": ["cdp", "firefox"], "expectations": ["FAIL", "SKIP"] }, + { + "testIdPattern": "[screenshot.spec] Screenshots Page.screenshot should clip clip bigger than the viewport without \"captureBeyondViewport\"", + "platforms": ["darwin", "linux", "win32"], + "parameters": ["firefox"], + "expectations": ["FAIL", "PASS"] + }, { "testIdPattern": "[screenshot.spec] Screenshots Page.screenshot should get screenshot bigger than the viewport", "platforms": ["darwin", "linux", "win32"], diff --git a/test/golden-chrome/screenshot-offscreen-clip-2.png b/test/golden-chrome/screenshot-offscreen-clip-2.png new file mode 100644 index 0000000000000000000000000000000000000000..7ec69d3040c31cfc100b7fd62f22ec45443b9d19 GIT binary patch literal 188 zcmeAS@N?(olHy`uVBq!ia0vp^k|4~%1|*NXY)uAIjKx9jP7LeL$-D$|$~;{hLp(a) zUR=m~R6)eyVsw*>vehO3eJOL+InG$P^3VnrE`KWq=YY`QyXv#{>^>k^K6C52$2Odb z2H8*Fe!FMLkj%5^`+DYbHie}v#eO^+_vbF&>UJ)Y$8f1gQqQ03p*$OfHqVzVbQE+I l33L&3-PdwsZ+YPpCaw(%Arf9!rUG5S;OXk;vd$@?2>`4TL+$_o literal 0 HcmV?d00001 diff --git a/test/golden-firefox/screenshot-offscreen-clip-2.png b/test/golden-firefox/screenshot-offscreen-clip-2.png new file mode 100644 index 0000000000000000000000000000000000000000..f7c0830ba91132be081058cdbdf6f9ae342b19b2 GIT binary patch literal 204 zcmeAS@N?(olHy`uVBq!ia0vp^k|4~%1|*NXY)uAIlRaG=Lp;2b|I|M_^S_i|-iU>r z-I(Df&lR@S@9w20utXX?{*u}D=~Gc#|L<>sxB1w1sNO0p(Je4wPs z;t^ysYnNMd_YwgG&yN|elx%l|t&90NUnMq0gNbP0l+XkK Dw~ { + const {page, server} = await getTestState(); + await page.setViewport({width: 50, height: 50}); + await page.goto(server.PREFIX + '/grid.html'); + const screenshot = await page.screenshot({ + captureBeyondViewport: false, + clip: { + x: 25, + y: 25, + width: 100, + height: 100, + }, + }); + expect(screenshot).toBeGolden('screenshot-offscreen-clip-2.png'); + }); it('should run in parallel', async () => { const {page, server} = await getTestState();