chore: remove screenshot path from the server side (#3519)

Also fixes auto-detection of mime type based on path and adds tests.

src/frames.ts

@@ -15,8 +15,6 @@
  * limitations under the License.
  */
 
-import * as fs from 'fs';
-import * as util from 'util';
 import { ConsoleMessage } from './console';
 import * as dom from './dom';
 import { Events } from './events';
@@ -640,31 +638,23 @@ export class Frame {
   }
 
   async addScriptTag(options: {
-      url?: string; path?: string;
+      url?: string,
-      content?: string;
+      content?: string,
-      type?: string;
+      type?: string,
     }): Promise<dom.ElementHandle> {
     const {
       url = null,
-      path = null,
       content = null,
       type = ''
     } = options;
-    if (!url && !path && !content)
+    if (!url && !content)
       throw new Error('Provide an object with a `url`, `path` or `content` property');
 
     const context = await this._mainContext();
     return this._raceWithCSPError(async () => {
       if (url !== null)
         return (await context.evaluateHandleInternal(addScriptUrl, { url, type })).asElement()!;
-      let result;
+      const result = (await context.evaluateHandleInternal(addScriptContent, { content: content!, type })).asElement()!;
-      if (path !== null) {
-        let contents = await util.promisify(fs.readFile)(path, 'utf8');
-        contents += '\n//# sourceURL=' + path.replace(/\n/g, '');
-        result = (await context.evaluateHandleInternal(addScriptContent, { content: contents, type })).asElement()!;
-      } else {
-        result = (await context.evaluateHandleInternal(addScriptContent, { content: content!, type })).asElement()!;
-      }
       // Another round trip to the browser to ensure that we receive CSP error messages
       // (if any) logged asynchronously in a separate task on the content main thread.
       if (this._page._delegate.cspErrorsAsynchronousForInlineScipts)
@@ -699,26 +689,18 @@ export class Frame {
     }
   }
 
-  async addStyleTag(options: { url?: string; path?: string; content?: string; }): Promise<dom.ElementHandle> {
+  async addStyleTag(options: { url?: string, content?: string }): Promise<dom.ElementHandle> {
     const {
       url = null,
-      path = null,
       content = null
     } = options;
-    if (!url && !path && !content)
+    if (!url && !content)
       throw new Error('Provide an object with a `url`, `path` or `content` property');
 
     const context = await this._mainContext();
     return this._raceWithCSPError(async () => {
       if (url !== null)
         return (await context.evaluateHandleInternal(addStyleUrl, url)).asElement()!;
-
-      if (path !== null) {
-        let contents = await util.promisify(fs.readFile)(path, 'utf8');
-        contents += '\n/*# sourceURL=' + path.replace(/\n/g, '') + '*/';
-        return (await context.evaluateHandleInternal(addStyleContent, contents)).asElement()!;
-      }
-
       return (await context.evaluateHandleInternal(addStyleContent, content!)).asElement()!;
     });
 

src/rpc/channels.ts

@@ -1822,14 +1822,12 @@ export type ElementHandleQuerySelectorAllResult = {
 export type ElementHandleScreenshotParams = {
   timeout?: number,
   type?: 'png' | 'jpeg',
-  path?: string,
   quality?: number,
   omitBackground?: boolean,
 };
 export type ElementHandleScreenshotOptions = {
   timeout?: number,
   type?: 'png' | 'jpeg',
-  path?: string,
   quality?: number,
   omitBackground?: boolean,
 };

src/rpc/client/elementHandle.ts

@@ -18,13 +18,15 @@ import { ElementHandleChannel, JSHandleInitializer, ElementHandleScrollIntoViewI
 import { Frame } from './frame';
 import { FuncOn, JSHandle, serializeArgument, parseResult } from './jsHandle';
 import { ChannelOwner } from './channelOwner';
-import { helper, assert } from '../../helper';
+import { helper, assert, mkdirIfNeeded } from '../../helper';
 import { SelectOption, FilePayload, Rect, SelectOptionOptions } from './types';
 import * as fs from 'fs';
 import * as mime from 'mime';
 import * as path from 'path';
 import * as util from 'util';
 
+const fsWriteFileAsync = util.promisify(fs.writeFile.bind(fs));
+
 export class ElementHandle<T extends Node = Node> extends JSHandle<T> {
   readonly _elementChannel: ElementHandleChannel;
 
@@ -175,9 +177,16 @@ export class ElementHandle<T extends Node = Node> extends JSHandle<T> {
     });
   }
 
-  async screenshot(options: ElementHandleScreenshotOptions = {}): Promise<Buffer> {
+  async screenshot(options: ElementHandleScreenshotOptions & { path?: string } = {}): Promise<Buffer> {
     return this._wrapApiCall('elementHandle.screenshot', async () => {
-      return Buffer.from((await this._elementChannel.screenshot(options)).binary, 'base64');
+      const type = determineScreenshotType(options);
+      const result = await this._elementChannel.screenshot({ ...options, type });
+      const buffer = Buffer.from(result.binary, 'base64');
+      if (options.path) {
+        await mkdirIfNeeded(options.path);
+        await fsWriteFileAsync(options.path, buffer);
+      }
+      return buffer;
     });
   }
 
@@ -262,3 +271,15 @@ export async function convertInputFiles(files: string | FilePayload | string[] |
   }));
   return filePayloads;
 }
+
+export function determineScreenshotType(options: { path?: string, type?: 'png' | 'jpeg' }): 'png' | 'jpeg' | undefined {
+  if (options.path) {
+    const mimeType = mime.getType(options.path);
+    if (mimeType === 'image/png')
+      return 'png';
+    else if (mimeType === 'image/jpeg')
+      return 'jpeg';
+    throw new Error(`path: unsupported mime type "${mimeType}"`);
+  }
+  return options.type;
+}

src/rpc/client/page.ts

@@ -27,7 +27,7 @@ import { ChannelOwner } from './channelOwner';
 import { ConsoleMessage } from './consoleMessage';
 import { Dialog } from './dialog';
 import { Download } from './download';
-import { ElementHandle } from './elementHandle';
+import { ElementHandle, determineScreenshotType } from './elementHandle';
 import { Worker } from './worker';
 import { Frame, FunctionWithSource, verifyLoadState, WaitForNavigationOptions } from './frame';
 import { Keyboard, Mouse } from './input';
@@ -425,7 +425,9 @@ export class Page extends ChannelOwner<PageChannel, PageInitializer> {
 
   async screenshot(options: PageScreenshotOptions & { path?: string } = {}): Promise<Buffer> {
     return this._wrapApiCall('page.screenshot', async () => {
-      const buffer = Buffer.from((await this._channel.screenshot(options)).binary, 'base64');
+      const type = determineScreenshotType(options);
+      const result = await this._channel.screenshot({ ...options, type });
+      const buffer = Buffer.from(result.binary, 'base64');
       if (options.path) {
         await mkdirIfNeeded(options.path);
         await fsWriteFileAsync(options.path, buffer);

src/rpc/protocol.yml

@@ -1513,7 +1513,6 @@ ElementHandle:
           literals:
           - png
           - jpeg
-        path: string?
         quality: number?
         omitBackground: boolean?
       returns:

src/rpc/validator.ts

@@ -718,7 +718,6 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme {
   scheme.ElementHandleScreenshotParams = tObject({
     timeout: tOptional(tNumber),
     type: tOptional(tEnum(['png', 'jpeg'])),
-    path: tOptional(tString),
     quality: tOptional(tNumber),
     omitBackground: tOptional(tBoolean),
   });

src/screenshotter.ts

@@ -15,11 +15,8 @@
  * limitations under the License.
  */
 
-import * as fs from 'fs';
-import * as mime from 'mime';
-import * as util from 'util';
 import * as dom from './dom';
-import { assert, helper, mkdirIfNeeded } from './helper';
+import { assert, helper } from './helper';
 import { Page } from './page';
 import * as types from './types';
 import { rewriteErrorMessage } from './utils/stackTrace';
@@ -153,10 +150,6 @@ export class Screenshotter {
     if (shouldSetDefaultBackground)
       await this._page._delegate.setBackgroundColor();
     progress.throwIfAborted(); // Avoid side effects.
-    if (options.path) {
-      await mkdirIfNeeded(options.path);
-      await util.promisify(fs.writeFile)(options.path, buffer);
-    }
     if ((options as any).__testHookAfterScreenshot)
       await (options as any).__testHookAfterScreenshot();
     return buffer;
@@ -206,13 +199,6 @@ function validateScreenshotOptions(options: types.ScreenshotOptions): 'png' | 'j
   if (options.type) {
     assert(options.type === 'png' || options.type === 'jpeg', 'Unknown options.type value: ' + options.type);
     format = options.type;
-  } else if (options.path) {
-    const mimeType = mime.getType(options.path);
-    if (mimeType === 'image/png')
-      format = 'png';
-    else if (mimeType === 'image/jpeg')
-      format = 'jpeg';
-    assert(format, 'Unsupported screenshot mime type: ' + mimeType);
   }
 
   if (!format)

src/types.ts

@@ -51,7 +51,6 @@ export type WaitForNavigationOptions = TimeoutOptions & {
 
 export type ElementScreenshotOptions = TimeoutOptions & {
   type?: 'png' | 'jpeg',
-  path?: string,
   quality?: number,
   omitBackground?: boolean,
 };

test/elementhandle-screenshot.spec.ts

@@ -19,6 +19,8 @@ import './base.fixture';
 import utils from './utils';
 const {WIRE, HEADLESS} = testOptions;
 import {PNG} from 'pngjs';
+import path from 'path';
+import fs from 'fs';
 
 // Firefox headful produces a different image.
 const ffheadful = FFOX && !HEADLESS;
@@ -369,3 +371,13 @@ it.skip(ffheadful)('should take screenshot of disabled button', async({page}) =>
   const screenshot = await button.screenshot();
   expect(screenshot).toBeInstanceOf(Buffer);
 });
+
+it.skip(ffheadful)('path option should create subdirectories', async({page, server, golden, tmpDir}) => {
+  await page.setViewportSize({width: 500, height: 500});
+  await page.goto(server.PREFIX + '/grid.html');
+  await page.evaluate(() => window.scrollBy(50, 100));
+  const elementHandle = await page.$('.box:nth-of-type(3)');
+  const outputPath = path.join(tmpDir, 'these', 'are', 'directories', 'screenshot.png');
+  await elementHandle.screenshot({path: outputPath});
+  expect(await fs.promises.readFile(outputPath)).toMatchImage(golden('screenshot-element-bounding-box.png'));
+});

test/page-screenshot.spec.ts

@@ -265,3 +265,17 @@ it.skip(ffheadful)('path option should create subdirectories', async({page, serv
   await page.screenshot({path: outputPath});
   expect(await fs.promises.readFile(outputPath)).toMatchImage(golden('screenshot-sanity.png'));
 });
+
+it.skip(ffheadful)('path option should detect jpeg', async({page, server, golden, tmpDir}) => {
+  await page.setViewportSize({ width: 100, height: 100 });
+  await page.goto(server.EMPTY_PAGE);
+  const outputPath = path.join(tmpDir, 'screenshot.jpg');
+  const screenshot = await page.screenshot({omitBackground: true, path: outputPath});
+  expect(await fs.promises.readFile(outputPath)).toMatchImage(golden('white.jpg'));
+  expect(screenshot).toMatchImage(golden('white.jpg'));
+});
+
+it.skip(ffheadful)('path option should throw for unsupported mime type', async({page, server, golden, tmpDir}) => {
+  const error = await page.screenshot({ path: 'file.txt' }).catch(e => e);
+  expect(error.message).toContain('path: unsupported mime type "text/plain"');
+});