fix(popups): do not override popup size from window features (#2139)

We usually force window size from the browser context. However,
popups that have window features insist on a specific window size,
so we respect that.

Author: dgozman

src/chromium/crPage.ts

@@ -56,6 +56,13 @@ export class CRPage implements PageDelegate {
   private readonly _pagePromise: Promise<Page | Error>;
   _initializedPage: Page | null = null;
 
+  // Holds window features for the next popup being opened via window.open,
+  // until the popup target arrives. This could be racy if two oopifs
+  // simultaneously call window.open with window features: the order
+  // of their Page.windowOpen events is not guaranteed to match the order
+  // of new popup targets.
+  readonly _nextWindowOpenPopupFeatures: string[][] = [];
+
   constructor(client: CRSession, targetId: string, browserContext: CRBrowserContext, opener: CRPage | null, hasUIWindow: boolean) {
     this._targetId = targetId;
     this._opener = opener;
@@ -68,6 +75,12 @@ export class CRPage implements PageDelegate {
     this._mainFrameSession = new FrameSession(this, client, targetId, null);
     this._sessions.set(targetId, this._mainFrameSession);
     client.once(CRSessionEvents.Disconnected, () => this._page._didDisconnect());
+    if (opener && browserContext._options.viewport !== null) {
+      const features = opener._nextWindowOpenPopupFeatures.shift() || [];
+      const viewportSize = helper.getViewportSizeFromWindowFeatures(features);
+      if (viewportSize)
+        this._page._state.viewportSize = viewportSize;
+    }
     this._pagePromise = this._mainFrameSession._initialize(hasUIWindow).then(() => this._initializedPage = this._page).catch(e => e);
   }
 
@@ -372,6 +385,7 @@ class FrameSession {
       helper.addEventListener(this._client, 'Runtime.executionContextsCleared', event => this._onExecutionContextsCleared()),
       helper.addEventListener(this._client, 'Target.attachedToTarget', event => this._onAttachedToTarget(event)),
       helper.addEventListener(this._client, 'Target.detachedFromTarget', event => this._onDetachedFromTarget(event)),
+      helper.addEventListener(this._client, 'Page.windowOpen', event => this._onWindowOpen(event)),
     ];
   }
 
@@ -591,6 +605,10 @@ class FrameSession {
     this._page._removeWorker(event.sessionId);
   }
 
+  _onWindowOpen(event: Protocol.Page.windowOpenPayload) {
+    this._crPage._nextWindowOpenPopupFeatures.push(event.windowFeatures);
+  }
+
   async _onConsoleAPI(event: Protocol.Runtime.consoleAPICalledPayload) {
     if (event.executionContextId === 0) {
       // DevTools protocol stores the last 1000 console messages. These

src/helper.ts

@@ -346,6 +346,16 @@ class Helper {
       }
     }
   }
+
+  static getViewportSizeFromWindowFeatures(features: string[]): types.Size | null {
+    const widthString = features.find(f => f.startsWith('width='));
+    const heightString = features.find(f => f.startsWith('height='));
+    const width = widthString ? parseInt(widthString.substring(6), 10) : NaN;
+    const height = heightString ? parseInt(heightString.substring(7), 10) : NaN;
+    if (!Number.isNaN(width) && !Number.isNaN(height))
+      return { width, height };
+    return null;
+  }
 }
 
 export function assert(value: any, message?: string): asserts value {

src/webkit/wkBrowser.ts

@@ -57,6 +57,7 @@ export class WKBrowser extends BrowserBase {
       helper.addEventListener(this._browserSession, 'Playwright.pageProxyCreated', this._onPageProxyCreated.bind(this)),
       helper.addEventListener(this._browserSession, 'Playwright.pageProxyDestroyed', this._onPageProxyDestroyed.bind(this)),
       helper.addEventListener(this._browserSession, 'Playwright.provisionalLoadFailed', event => this._onProvisionalLoadFailed(event)),
+      helper.addEventListener(this._browserSession, 'Playwright.windowOpen', event => this._onWindowOpen(event)),
       helper.addEventListener(this._browserSession, 'Playwright.downloadCreated', this._onDownloadCreated.bind(this)),
       helper.addEventListener(this._browserSession, 'Playwright.downloadFilenameSuggested', this._onDownloadFilenameSuggested.bind(this)),
       helper.addEventListener(this._browserSession, 'Playwright.downloadFinished', this._onDownloadFinished.bind(this)),
@@ -180,6 +181,13 @@ export class WKBrowser extends BrowserBase {
     wkPage.handleProvisionalLoadFailed(event);
   }
 
+  _onWindowOpen(event: Protocol.Playwright.windowOpenPayload) {
+    const wkPage = this._wkPages.get(event.pageProxyId);
+    if (!wkPage)
+      return;
+    wkPage.handleWindowOpen(event);
+  }
+
   isConnected(): boolean {
     return !this._connection.isClosed();
   }

src/webkit/wkPage.ts

@@ -16,7 +16,7 @@
  */
 
 import * as frames from '../frames';
-import { helper, RegisteredListener, assert } from '../helper';
+import { helper, RegisteredListener, assert, debugAssert } from '../helper';
 import * as dom from '../dom';
 import * as network from '../network';
 import { WKSession } from './wkConnection';
@@ -68,6 +68,10 @@ export class WKPage implements PageDelegate {
   _firstNonInitialNavigationCommittedReject = (e: Error) => {};
   private _lastConsoleMessage: { derivedType: string, text: string, handles: JSHandle[]; count: number, location: ConsoleMessageLocation; } | null = null;
 
+  // Holds window features for the next popup being opened via window.open,
+  // until the popup page proxy arrives.
+  private _nextWindowOpenPopupFeatures?: string[];
+
   constructor(browserContext: WKBrowserContext, pageProxySession: WKSession, opener: WKPage | null) {
     this._pageProxySession = pageProxySession;
     this._opener = opener;
@@ -90,6 +94,12 @@ export class WKPage implements PageDelegate {
       this._firstNonInitialNavigationCommittedFulfill = f;
       this._firstNonInitialNavigationCommittedReject = r;
     });
+    if (opener && browserContext._options.viewport !== null && opener._nextWindowOpenPopupFeatures) {
+      const viewportSize = helper.getViewportSizeFromWindowFeatures(opener._nextWindowOpenPopupFeatures);
+      opener._nextWindowOpenPopupFeatures = undefined;
+      if (viewportSize)
+        this._page._state.viewportSize = viewportSize;
+    }
   }
 
   private async _initializePageProxySession() {
@@ -243,6 +253,11 @@ export class WKPage implements PageDelegate {
     this._page._frameManager.provisionalLoadFailed(this._page.mainFrame(), event.loaderId, errorText);
   }
 
+  handleWindowOpen(event: Protocol.Playwright.windowOpenPayload) {
+    debugAssert(!this._nextWindowOpenPopupFeatures);
+    this._nextWindowOpenPopupFeatures = event.windowFeatures;
+  }
+
   async pageOrError(): Promise<Page | Error> {
     return this._pagePromise;
   }

test/popup.spec.js

@@ -138,6 +138,26 @@ describe('window.open', function() {
     await context.close();
     expect(size).toEqual({width: 400, height: 500});
   });
+  it('should use viewport size from window features', async function({browser, server}) {
+    const context = await browser.newContext({
+      viewport: { width: 700, height: 700 }
+    });
+    const page = await context.newPage();
+    await page.goto(server.EMPTY_PAGE);
+    const [size, popup] = await Promise.all([
+      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=600,height=300,top=0,left=0');
+        return { width: win.innerWidth, height: win.innerHeight };
+      }),
+      page.waitForEvent('popup'),
+    ]);
+    await popup.setViewportSize({ width: 500, height: 400 });
+    await popup.waitForLoadState();
+    const resized = await popup.evaluate(() => ({ width: window.innerWidth, height: window.innerHeight }));
+    await context.close();
+    expect(size).toEqual({width: 600, height: 300});
+    expect(resized).toEqual({width: 500, height: 400});
+  });
   it('should respect routes from browser context', async function({browser, server}) {
     const context = await browser.newContext();
     const page = await context.newPage();