Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix fromSurface:false screenshoting #8591

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/api/puppeteer.page.screenshot.md
Expand Up @@ -44,6 +44,6 @@ Options object which might have the following properties:

- `captureBeyondViewport` : When true, captures screenshot [beyond the viewport](https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-captureScreenshot). When false, falls back to old behaviour, and cuts the screenshot by the viewport size. Defaults to `true`.

- `fromSurface` : When true, captures screenshot [from the surface rather than the view](https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-captureScreenshot). When false, works only in headful mode and ignores page viewport (but not browser window's bounds). Defaults to `true`.
- `fromSurface` : When true, captures screenshot [from the surface rather than the view](https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-captureScreenshot). When false, works only in headful mode. Defaults to `true`.

NOTE: Screenshots take at least 1/6 second on OS X. See [https://crbug.com/741689](https://crbug.com/741689) for discussion.
30 changes: 10 additions & 20 deletions src/common/Page.ts
Expand Up @@ -2791,8 +2791,7 @@ export class Page extends EventEmitter {
* - `fromSurface` : When true, captures screenshot
* {@link https://chromedevtools.github.io/devtools-protocol/tot/Page/#method-captureScreenshot
* | from the surface rather than the view}. When false, works only in
* headful mode and ignores page viewport (but not browser window's
* bounds). Defaults to `true`.
* headful mode. Defaults to `true`.
*
* NOTE: Screenshots take at least 1/6 second on OS X. See
* {@link https://crbug.com/741689} for discussion.
Expand Down Expand Up @@ -2909,6 +2908,8 @@ export class Page extends EventEmitter {
? options.fromSurface
: undefined;

const viewport = this.#viewport;

if (options.fullPage) {
const metrics = await this.#client.send('Page.getLayoutMetrics');
// Fallback to `contentSize` in case of using Firefox.
Expand All @@ -2918,22 +2919,7 @@ export class Page extends EventEmitter {
clip = {x: 0, y: 0, width, height, scale: 1};

if (!captureBeyondViewport) {
const {
isMobile = false,
deviceScaleFactor = 1,
isLandscape = false,
} = this.#viewport || {};
const screenOrientation: Protocol.Emulation.ScreenOrientation =
isLandscape
? {angle: 90, type: 'landscapePrimary'}
: {angle: 0, type: 'portraitPrimary'};
await this.#client.send('Emulation.setDeviceMetricsOverride', {
mobile: isMobile,
width,
height,
deviceScaleFactor,
screenOrientation,
});
this.setViewport({...viewport, width, height});
}
}
const shouldSetDefaultBackground =
Expand All @@ -2942,6 +2928,10 @@ export class Page extends EventEmitter {
await this.#setTransparentBackgroundColor();
}

if (fromSurface === false && this.#viewport) {
await this.setViewport(this.#viewport);
}

const result = await this.#client.send('Page.captureScreenshot', {
format,
quality: options.quality,
Expand All @@ -2953,8 +2943,8 @@ export class Page extends EventEmitter {
await this.#resetDefaultBackgroundColor();
}

if (options.fullPage && this.#viewport) {
await this.setViewport(this.#viewport);
if (options.fullPage && !captureBeyondViewport && viewport) {
await this.setViewport(viewport);
}

const buffer =
Expand Down
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 1 addition & 3 deletions test/src/golden-utils.ts
Expand Up @@ -65,9 +65,7 @@ const compareImages = (
? PNG.sync.read(expectedBuffer)
: jpeg.decode(expectedBuffer);
if (expected.width !== actual.width || expected.height !== actual.height) {
throw new Error(
`Sizes differ: expected image ${expected.width}px X ${expected.height}px, but got ${actual.width}px X ${actual.height}px.`
);
return {diff: PNG.sync.write(new PNG(actual))};
}
const diff = new PNG({width: expected.width, height: expected.height});
const count = pixelmatch(
Expand Down
2 changes: 1 addition & 1 deletion test/src/screenshot.spec.ts
Expand Up @@ -197,7 +197,7 @@ describe('Screenshots', function () {
const screenshot = await page.screenshot({
fromSurface: false,
});
expect(screenshot).toBeDefined(); // toBeGolden('screenshot-fromsurface-false.png');
expect(screenshot).toBeGolden('screenshot-fromsurface-false.png');
});
});

Expand Down