Skip to content

Commit

Permalink
chore: set allowViewportExpansion to captureBeyondViewport (#10992)
Browse files Browse the repository at this point in the history
  • Loading branch information
jrandolf committed Sep 22, 2023
1 parent 9676da3 commit 4ca3645
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 7 deletions.
5 changes: 2 additions & 3 deletions packages/puppeteer-core/src/api/ElementHandle.ts
Expand Up @@ -1338,7 +1338,7 @@ export abstract class ElementHandle<
const {
scrollIntoView = true,
captureBeyondViewport = true,
allowViewportExpansion = true,
allowViewportExpansion = captureBeyondViewport,
} = options;

let clip = await this.#nonEmptyVisibleBoundingBox();
Expand All @@ -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;

Expand All @@ -1373,7 +1373,6 @@ export abstract class ElementHandle<
return await page.screenshot({
...options,
captureBeyondViewport: false,
allowViewportExpansion: false,
clip,
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/puppeteer-core/src/api/Page.ts
Expand Up @@ -2351,7 +2351,7 @@ export abstract class Page extends EventEmitter<PageEvents> {
}

options.captureBeyondViewport ??= true;
options.allowViewportExpansion ??= true;
options.allowViewportExpansion ??= options.captureBeyondViewport;
options.clip = options.clip && roundClip(normalizeClip(options.clip));

await using stack = new AsyncDisposableStack();
Expand Down
13 changes: 11 additions & 2 deletions packages/puppeteer-core/src/bidi/Page.ts
Expand Up @@ -627,7 +627,12 @@ export class BidiPage extends Page {
override async screenshot(
options: Readonly<ScreenshotOptions> = {}
): Promise<Buffer | string> {
const {clip, type, captureBeyondViewport} = options;
const {
clip,
type,
captureBeyondViewport,
allowViewportExpansion = true,
} = options;
if (captureBeyondViewport) {
throw new Error(`BiDi does not support 'captureBeyondViewport'.`);
}
Expand All @@ -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(
Expand Down
41 changes: 40 additions & 1 deletion packages/puppeteer-core/src/cdp/Page.ts
Expand Up @@ -35,6 +35,7 @@ import {
type NewDocumentScriptEvaluation,
type ScreenshotOptions,
type WaitTimeoutOptions,
type ScreenshotClip,
} from '../api/Page.js';
import {
ConsoleMessage,
Expand Down Expand Up @@ -1044,7 +1045,7 @@ export class CdpPage extends Page {
omitBackground,
optimizeForSpeed,
quality,
clip,
clip: userClip,
type,
captureBeyondViewport,
} = options;
Expand All @@ -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,
Expand Down Expand Up @@ -1204,3 +1221,25 @@ const supportedMetrics = new Set<string>([
'JSHeapUsedSize',
'JSHeapTotalSize',
]);

/** @see https://w3c.github.io/webdriver-bidi/#rectangle-intersection */
function getIntersectionRect(
clip: Readonly<ScreenshotClip>,
viewport: Readonly<Protocol.DOM.Rect>
): 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
),
};
}
6 changes: 6 additions & 0 deletions test/TestExpectations.json
Expand Up @@ -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"],
Expand Down
Binary file added test/golden-chrome/screenshot-offscreen-clip-2.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added test/golden-firefox/screenshot-offscreen-clip-2.png
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 15 additions & 0 deletions test/src/screenshot.spec.ts
Expand Up @@ -77,6 +77,21 @@ describe('Screenshots', function () {
});
expect(screenshot).toBeGolden('screenshot-offscreen-clip.png');
});
it('should clip clip bigger than the viewport without "captureBeyondViewport"', async () => {
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();

Expand Down

0 comments on commit 4ca3645

Please sign in to comment.