fix(page): full page screenshot when defaultViewport is null (#3306)

Fixes #3104
puppeteer · Sep 27, 2018 · 842fee8 · 842fee8
1 parent e75e36b
commit 842fee8
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 7 deletions.
diff --git a/lib/Page.js b/lib/Page.js
@@ -796,19 +796,20 @@ class Page extends EventEmitter {
       clip.scale = 1;
 
     if (options.fullPage) {
-      assert(this._viewport, 'fullPage screenshots do not work without first setting viewport.');
       const metrics = await this._client.send('Page.getLayoutMetrics');
       const width = Math.ceil(metrics.contentSize.width);
       const height = Math.ceil(metrics.contentSize.height);
 
       // Overwrite clip for full page at all times.
       clip = { x: 0, y: 0, width, height, scale: 1 };
-      const mobile = this._viewport.isMobile || false;
-      const deviceScaleFactor = this._viewport.deviceScaleFactor || 1;
-      const landscape = this._viewport.isLandscape || false;
+      const {
+        isMobile = false,
+        deviceScaleFactor = 1,
+        isLandscape = false
+      } = this._viewport || {};
       /** @type {!Protocol.Emulation.ScreenOrientation} */
-      const screenOrientation = landscape ? { angle: 90, type: 'landscapePrimary' } : { angle: 0, type: 'portraitPrimary' };
-      await this._client.send('Emulation.setDeviceMetricsOverride', { mobile, width, height, deviceScaleFactor, screenOrientation });
+      const screenOrientation = isLandscape ? { angle: 90, type: 'landscapePrimary' } : { angle: 0, type: 'portraitPrimary' };
+      await this._client.send('Emulation.setDeviceMetricsOverride', { mobile: isMobile, width, height, deviceScaleFactor, screenOrientation });
     }
     const shouldSetDefaultBackground = options.omitBackground && format === 'png';
     if (shouldSetDefaultBackground)
@@ -817,7 +818,7 @@ class Page extends EventEmitter {
     if (shouldSetDefaultBackground)
       await this._client.send('Emulation.setDefaultBackgroundColorOverride');
 
-    if (options.fullPage)
+    if (options.fullPage && this._viewport)
       await this.setViewport(this._viewport);
 
     const buffer = options.encoding === 'base64' ? result.data : Buffer.from(result.data, 'base64');

diff --git a/test/puppeteer.spec.js b/test/puppeteer.spec.js
@@ -279,6 +279,19 @@ module.exports.addTests = function({testRunner, expect, defaultBrowserOptions})
         expect(page.viewport()).toBe(null);
         await browser.close();
       });
+      it('should take fullPage screenshots when defaultViewport is null', async({server}) => {
+        const options = Object.assign({}, defaultBrowserOptions, {
+          defaultViewport: null
+        });
+        const browser = await puppeteer.launch(options);
+        const page = await browser.newPage();
+        await page.goto(server.PREFIX + '/grid.html');
+        const screenshot = await page.screenshot({
+          fullPage: true
+        });
+        expect(screenshot).toBeInstanceOf(Buffer);
+        await browser.close();
+      });
     });
     describe('Puppeteer.connect', function() {
       it('should be able to connect multiple times to the same browser', async({server}) => {