api(viewport): do not allow isMobile and deviceScaleFactor for null viewports (#2190)

Also make sure we do not alter viewport when passed null.

Author: dgozman

src/browserContext.ts

@@ -186,6 +186,10 @@ export function assertBrowserContextIsNotOwned(context: BrowserContextBase) {
 
 export function validateBrowserContextOptions(options: BrowserContextOptions): BrowserContextOptions {
   const result = { ...options };
+  if (result.viewport === null && result.deviceScaleFactor !== undefined)
+    throw new Error(`"deviceScaleFactor" option is not supported with null "viewport"`);
+  if (result.viewport === null && result.isMobile !== undefined)
+    throw new Error(`"isMobile" option is not supported with null "viewport"`);
   if (!result.viewport && result.viewport !== null)
     result.viewport = { width: 1280, height: 720 };
   if (result.viewport)

src/chromium/crPage.ts

@@ -429,7 +429,7 @@ class FrameSession {
       promises.push(this._client.send('Page.setBypassCSP', { enabled: true }));
     if (options.ignoreHTTPSErrors)
       promises.push(this._client.send('Security.setIgnoreCertificateErrors', { ignore: true }));
-    if (this._isMainFrame() && options.viewport)
+    if (this._isMainFrame())
       promises.push(this._updateViewport());
     if (options.hasTouch)
       promises.push(this._client.send('Emulation.setTouchEmulationEnabled', { enabled: true }));
@@ -706,23 +706,23 @@ class FrameSession {
   async _updateViewport(): Promise<void> {
     assert(this._isMainFrame());
     const options = this._crPage._browserContext._options;
-    let viewport = options.viewport || { width: 0, height: 0 };
     const viewportSize = this._page._state.viewportSize;
-    if (viewportSize)
-      viewport = { ...viewport, ...viewportSize };
-    const isLandscape = viewport.width > viewport.height;
+    if (viewportSize === null)
+      return;
+    const isLandscape = viewportSize.width > viewportSize.height;
     const promises = [
       this._client.send('Emulation.setDeviceMetricsOverride', {
         mobile: !!options.isMobile,
-        width: viewport.width,
-        height: viewport.height,
-        screenWidth: viewport.width,
-        screenHeight: viewport.height,
+        width: viewportSize.width,
+        height: viewportSize.height,
+        screenWidth: viewportSize.width,
+        screenHeight: viewportSize.height,
         deviceScaleFactor: options.deviceScaleFactor || 1,
         screenOrientation: isLandscape ? { angle: 90, type: 'landscapePrimary' } : { angle: 0, type: 'portraitPrimary' },
       }),
     ];
     if (this._windowId) {
+      // TODO: popup windows have their own insets.
       let insets = { width: 24, height: 88 };
       if (process.platform === 'win32')
         insets = { width: 16, height: 88 };
@@ -733,7 +733,7 @@ class FrameSession {
 
       promises.push(this._client.send('Browser.setWindowBounds', {
         windowId: this._windowId,
-        bounds: { width: viewport.width + insets.width, height: viewport.height + insets.height }
+        bounds: { width: viewportSize.width + insets.width, height: viewportSize.height + insets.height }
       }));
     }
     await Promise.all(promises);

src/page.ts

@@ -120,15 +120,8 @@ export class Page extends ExtendedEventEmitter implements InnerLogger {
     this._disconnectedCallback = () => {};
     this._disconnectedPromise = new Promise(f => this._disconnectedCallback = f);
     this._browserContext = browserContext;
-    let viewportSize: types.Size | null = null;
-    if (browserContext._options.viewport) {
-      viewportSize = {
-        width: browserContext._options.viewport.width,
-        height: browserContext._options.viewport.height,
-      };
-    }
     this._state = {
-      viewportSize,
+      viewportSize: browserContext._options.viewport ? { ...browserContext._options.viewport } : null,
       mediaType: null,
       colorScheme: null,
       extraHTTPHeaders: null,

src/webkit/wkPage.ts

@@ -100,8 +100,7 @@ export class WKPage implements PageDelegate {
     const contextOptions = this._browserContext._options;
     if (contextOptions.javaScriptEnabled === false)
       promises.push(this._pageProxySession.send('Emulation.setJavaScriptEnabled', { enabled: false }));
-    if (this._page._state.viewportSize || contextOptions.viewport)
-      promises.push(this._updateViewport());
+    promises.push(this._updateViewport());
     promises.push(this.updateHttpCredentials());
     if (this._browserContext._permissions.size) {
       for (const [key, value] of this._browserContext._permissions)
@@ -573,24 +572,23 @@ export class WKPage implements PageDelegate {
 
   async _updateViewport(): Promise<void> {
     const options = this._browserContext._options;
-    let viewport = options.viewport || { width: 0, height: 0 };
     const viewportSize = this._page._state.viewportSize;
-    if (viewportSize)
-      viewport = { ...viewport, ...viewportSize };
+    if (viewportSize === null)
+      return;
     const promises: Promise<any>[] = [
       this._pageProxySession.send('Emulation.setDeviceMetricsOverride', {
-        width: viewport.width,
-        height: viewport.height,
+        width: viewportSize.width,
+        height: viewportSize.height,
         fixedLayout: !!options.isMobile,
         deviceScaleFactor: options.deviceScaleFactor || 1
       }),
       this._session.send('Page.setScreenSizeOverride', {
-        width: viewport.width,
-        height: viewport.height,
+        width: viewportSize.width,
+        height: viewportSize.height,
       }),
     ];
     if (options.isMobile) {
-      const angle = viewport.width > viewport.height ? 90 : 0;
+      const angle = viewportSize.width > viewportSize.height ? 90 : 0;
       promises.push(this._session.send('Page.setOrientationOverride', { angle }));
     }
     await Promise.all(promises);

test/browsercontext.spec.js

@@ -102,6 +102,14 @@ describe('BrowserContext', function() {
     expect(await page.evaluate('window.innerHeight')).toBe(789);
     await context.close();
   });
+  it('should not allow deviceScaleFactor with null viewport', async({ browser }) => {
+    const error = await browser.newContext({ viewport: null, deviceScaleFactor: 1 }).catch(e => e);
+    expect(error.message).toBe('"deviceScaleFactor" option is not supported with null "viewport"');
+  });
+  it('should not allow isMobile with null viewport', async({ browser }) => {
+    const error = await browser.newContext({ viewport: null, isMobile: true }).catch(e => e);
+    expect(error.message).toBe('"isMobile" option is not supported with null "viewport"');
+  });
   it('close() should work for empty context', async({ browser }) => {
     const context = await browser.newContext();
     await context.close();

test/emulation.spec.js

@@ -77,6 +77,13 @@ describe('BrowserContext({viewport})', function() {
     await page.goto(server.PREFIX + '/detect-touch.html');
     expect(await page.evaluate(() => document.body.textContent.trim())).toBe('NO');
   });
+  it('should support touch with null viewport', async({browser, server}) => {
+    const context = await browser.newContext({ viewport: null, hasTouch: true });
+    const page = await context.newPage();
+    await page.goto(server.PREFIX + '/mobile.html');
+    expect(await page.evaluate(() => 'ontouchstart' in window)).toBe(true);
+    await context.close();
+  });
 });
 
 describe.skip(FFOX)('viewport.isMobile', () => {

test/headful.spec.js

@@ -114,4 +114,22 @@ describe('Headful', function() {
     }
     await browser.close();
   });
+  it.fail(WEBKIT)('should not override viewport size when passed null', async function({browserType, defaultBrowserOptions, server}) {
+    // Our WebKit embedder does not respect window features.
+    const browser = await browserType.launch({...defaultBrowserOptions, headless: false });
+    const context = await browser.newContext({ viewport: null });
+    const page = await context.newPage();
+    await page.goto(server.EMPTY_PAGE);
+    const [popup] = await Promise.all([
+      page.waitForEvent('popup'),
+      page.evaluate(() => {
+        const win = window.open(window.location.href, 'Title', 'toolbar=no,location=no,directories=no,status=no,menubar=no,scrollbars=yes,resizable=yes,width=780,height=200,top=0,left=0');
+        win.resizeTo(500, 450);
+      }),
+    ]);
+    await popup.waitForLoadState();
+    const size = await popup.evaluate(() => ({ width: window.outerWidth, height: window.outerHeight }));
+    await context.close();
+    expect(size).toEqual({width: 500, height: 450});
+  });
 });