fix(screenshots): simplify implementation, allow fullPage + clip, add tests (#1194)

Author: dgozman

docs/api.md

@@ -1298,18 +1298,18 @@ await browser.close();
 #### page.screenshot([options])
 - `options` <[Object]> Options object which might have the following properties:
   - `path` <[string]> The file path to save the image to. The screenshot type will be inferred from file extension. If `path` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd). If no path is provided, the image won't be saved to the disk.
-  - `type` <"png"|"jpeg"> Specify screenshot type, defaults to 'png'.
+  - `type` <"png"|"jpeg"> Specify screenshot type, defaults to `png`.
   - `quality` <[number]> The quality of the image, between 0-100. Not applicable to `png` images.
-  - `fullPage` <[boolean]> When true, takes a screenshot of the full scrollable page. Defaults to `false`.
-  - `clip` <[Object]> An object which specifies clipping region of the page. Should have the following fields:
+  - `fullPage` <[boolean]> When true, takes a screenshot of the full scrollable page, instead of the currently visibvle viewport. Defaults to `false`.
+  - `clip` <[Object]> An object which specifies clipping of the resulting image. Should have the following fields:
     - `x` <[number]> x-coordinate of top-left corner of clip area
     - `y` <[number]> y-coordinate of top-left corner of clip area
     - `width` <[number]> width of clipping area
     - `height` <[number]> height of clipping area
-  - `omitBackground` <[boolean]> Hides default white background and allows capturing screenshots with transparency. Defaults to `false`.
+  - `omitBackground` <[boolean]> Hides default white background and allows capturing screenshots with transparency. Not applicable to `jpeg` images. Defaults to `false`.
 - returns: <[Promise]<[Buffer]>> Promise which resolves to buffer with the captured screenshot.
 
-> **NOTE** Screenshots take at least 1/6 second on OS X. See https://crbug.com/741689 for discussion.
+> **NOTE** Screenshots take at least 1/6 second on Chromium OS X and Chromium Windows. See https://crbug.com/741689 for discussion.
 
 #### page.select(selector, value, options)
 - `selector` <[string]> A selector to query frame for.
@@ -2483,13 +2483,12 @@ If `key` is a single character and no modifier keys besides `Shift` are being he
 #### elementHandle.screenshot([options])
 - `options` <[Object]> Screenshot options.
   - `path` <[string]> The file path to save the image to. The screenshot type will be inferred from file extension. If `path` is a relative path, then it is resolved relative to [current working directory](https://nodejs.org/api/process.html#process_process_cwd). If no path is provided, the image won't be saved to the disk.
-  - `type` <"png"|"jpeg"> Specify screenshot type, defaults to 'png'.
+  - `type` <"png"|"jpeg"> Specify screenshot type, defaults to `png`.
   - `quality` <[number]> The quality of the image, between 0-100. Not applicable to `png` images.
-  - `omitBackground` <[boolean]> Hides default white background and allows capturing screenshots with transparency. Defaults to `false`.
+  - `omitBackground` <[boolean]> Hides default white background and allows capturing screenshots with transparency. Not applicable to `jpeg` images. Defaults to `false`.
 - returns: <[Promise]<|[Buffer]>> Promise which resolves to buffer with the captured screenshot.
 
-This method scrolls element into view if needed, and then uses [page.screenshot](#pagescreenshotoptions) to take a screenshot of the element.
-If the element is detached from DOM, the method throws an error.
+This method scrolls element into view if needed before taking a screenshot. If the element is detached from DOM, the method throws an error.
 
 #### elementHandle.scrollIntoViewIfNeeded()
 - returns: <[Promise]> Resolves after the element has been scrolled into view.

src/chromium/crPage.ts

@@ -417,16 +417,6 @@ export class CRPage implements PageDelegate {
       await this._browser._closePage(this._page);
   }
 
-  async getBoundingBoxForScreenshot(handle: dom.ElementHandle<Node>): Promise<types.Rect | null> {
-    const rect = await handle.boundingBox();
-    if (!rect)
-      return rect;
-    const { layoutViewport: { pageX, pageY } } = await this._client.send('Page.getLayoutMetrics');
-    rect.x += pageX;
-    rect.y += pageY;
-    return rect;
-  }
-
   canScreenshotOutsideViewport(): boolean {
     return false;
   }
@@ -435,10 +425,23 @@ export class CRPage implements PageDelegate {
     await this._client.send('Emulation.setDefaultBackgroundColorOverride', { color });
   }
 
-  async takeScreenshot(format: 'png' | 'jpeg', options: types.ScreenshotOptions, viewportSize: types.Size): Promise<platform.BufferType> {
+  async takeScreenshot(format: 'png' | 'jpeg', documentRect: types.Rect | undefined, viewportRect: types.Rect | undefined, quality: number | undefined): Promise<platform.BufferType> {
+    const { visualViewport } = await this._client.send('Page.getLayoutMetrics');
+    if (!documentRect) {
+      documentRect = {
+        x: visualViewport.pageX + viewportRect!.x,
+        y: visualViewport.pageY + viewportRect!.y,
+        ...helper.enclosingIntSize({
+          width: viewportRect!.width / visualViewport.scale,
+          height: viewportRect!.height / visualViewport.scale,
+        })
+      };
+    }
     await this._client.send('Page.bringToFront', {});
-    const clip = options.clip ? { ...options.clip, scale: 1 } : undefined;
-    const result = await this._client.send('Page.captureScreenshot', { format, quality: options.quality, clip });
+    // When taking screenshots with documentRect (based on the page content, not viewport),
+    // ignore current page scale.
+    const clip = { ...documentRect, scale: viewportRect ? visualViewport.scale : 1 };
+    const result = await this._client.send('Page.captureScreenshot', { format, quality, clip });
     return platform.Buffer.from(result.data, 'base64');
   }
 

src/firefox/ffPage.ts

@@ -317,15 +317,6 @@ export class FFPage implements PageDelegate {
     await this._session.send('Page.close', { runBeforeUnload });
   }
 
-  async getBoundingBoxForScreenshot(handle: dom.ElementHandle<Node>): Promise<types.Rect | null> {
-    const frameId = handle._context.frame._id;
-    const response = await this._session.send('Page.getBoundingBox', {
-      frameId,
-      objectId: handle._remoteObject.objectId,
-    });
-    return response.boundingBox;
-  }
-
   canScreenshotOutsideViewport(): boolean {
     return true;
   }
@@ -335,11 +326,22 @@ export class FFPage implements PageDelegate {
       throw new Error('Not implemented');
   }
 
-  async takeScreenshot(format: 'png' | 'jpeg', options: types.ScreenshotOptions, viewportSize: types.Size): Promise<platform.BufferType> {
+  async takeScreenshot(format: 'png' | 'jpeg', documentRect: types.Rect | undefined, viewportRect: types.Rect | undefined, quality: number | undefined): Promise<platform.BufferType> {
+    if (!documentRect) {
+      const context = await this._page.mainFrame()._utilityContext();
+      const scrollOffset = await context.evaluate(() => ({ x: window.scrollX, y: window.scrollY }));
+      documentRect = {
+        x: viewportRect!.x + scrollOffset.x,
+        y: viewportRect!.y + scrollOffset.y,
+        width: viewportRect!.width,
+        height: viewportRect!.height,
+      };
+    }
+    // TODO: remove fullPage option from Page.screenshot.
+    // TODO: remove Page.getBoundingBox method.
     const { data } = await this._session.send('Page.screenshot', {
       mimeType: ('image/' + format) as ('image/png' | 'image/jpeg'),
-      fullPage: options.fullPage,
-      clip: options.clip,
+      clip: documentRect,
     }).catch(e => {
       if (e instanceof Error && e.message.includes('document.documentElement is null'))
         e.message = kScreenshotDuringNavigationError;
@@ -349,7 +351,7 @@ export class FFPage implements PageDelegate {
   }
 
   async resetViewport(): Promise<void> {
-    await this._session.send('Page.setViewportSize', { viewportSize: null });
+    assert(false, 'Should not be called');
   }
 
   async getContentFrame(handle: dom.ElementHandle): Promise<frames.Frame | null> {

src/helper.ts

@@ -17,6 +17,7 @@
 
 import { TimeoutError } from './errors';
 import * as platform from './platform';
+import * as types from './types';
 
 export const debugError = platform.debug(`pw:error`);
 
@@ -266,6 +267,18 @@ class Helper {
     const rightHalf = maxLength - leftHalf - 1;
     return string.substr(0, leftHalf) + '\u2026' + string.substr(this.length - rightHalf, rightHalf);
   }
+
+  static enclosingIntRect(rect: types.Rect): types.Rect {
+    const x = Math.floor(rect.x + 1e-3);
+    const y = Math.floor(rect.y + 1e-3);
+    const x2 = Math.ceil(rect.x + rect.width - 1e-3);
+    const y2 = Math.ceil(rect.y + rect.height - 1e-3);
+    return { x, y, width: x2 - x, height: y2 - y };
+  }
+
+  static enclosingIntSize(size: types.Size): types.Size {
+    return { width: Math.floor(size.width + 1e-3), height: Math.floor(size.height + 1e-3) };
+  }
 }
 
 export function assert(value: any, message?: string): asserts value {

src/page.ts

@@ -54,11 +54,10 @@ export interface PageDelegate {
   authenticate(credentials: types.Credentials | null): Promise<void>;
   setFileChooserIntercepted(enabled: boolean): Promise<void>;
 
-  getBoundingBoxForScreenshot(handle: dom.ElementHandle<Node>): Promise<types.Rect | null>;
   canScreenshotOutsideViewport(): boolean;
+  resetViewport(): Promise<void>; // Only called if canScreenshotOutsideViewport() returns false.
   setBackgroundColor(color?: { r: number; g: number; b: number; a: number; }): Promise<void>;
-  takeScreenshot(format: string, options: types.ScreenshotOptions, viewportSize: types.Size): Promise<platform.BufferType>;
-  resetViewport(oldSize: types.Size): Promise<void>;
+  takeScreenshot(format: string, documentRect: types.Rect | undefined, viewportRect: types.Rect | undefined, quality: number | undefined): Promise<platform.BufferType>;
 
   isElementHandle(remoteObject: any): boolean;
   adoptElementHandle<T extends Node>(handle: dom.ElementHandle<T>, to: dom.FrameExecutionContext): Promise<dom.ElementHandle<T>>;

src/platform.ts

@@ -234,9 +234,9 @@ export function urlMatches(urlString: string, match: types.URLMatch | undefined)
   return match(url);
 }
 
-export function pngToJpeg(buffer: Buffer): Buffer {
+export function pngToJpeg(buffer: Buffer, quality?: number): Buffer {
   assert(isNode, 'Converting from png to jpeg is only supported in Node.js');
-  return jpeg.encode(png.PNG.sync.read(buffer)).data;
+  return jpeg.encode(png.PNG.sync.read(buffer), quality).data;
 }
 
 function nodeFetch(url: string): Promise<string> {