feat: enhance native window.open to match the custom implementation's behavior (#19703)

Co-authored-by: Andy Locascio <andy@slack-corp.com>

lib/browser/api/net.js

@@ -126,7 +126,6 @@ class SlurpStream extends Writable {
     this._data = Buffer.concat([this._data, chunk]);
     callback();
   }
-
   data () { return this._data; }
 }
 

lib/browser/api/web-contents.js

@@ -11,6 +11,7 @@ const { internalWindowOpen } = require('@electron/internal/browser/guest-window-
 const NavigationController = require('@electron/internal/browser/navigation-controller');
 const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal');
 const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils');
+const { convertFeaturesString } = require('@electron/internal/common/parse-features-string');
 const { MessagePortMain } = require('@electron/internal/browser/message-port-main');
 
 // session is not used here, the purpose is to make sure session is initalized
@@ -506,40 +507,42 @@ WebContents.prototype._init = function () {
     this.reload();
   });
 
-  // Handle window.open for BrowserWindow and BrowserView.
-  if (['browserView', 'window'].includes(this.getType())) {
+  if (this.getType() !== 'remote') {
     // Make new windows requested by links behave like "window.open".
     this.on('-new-window', (event, url, frameName, disposition,
-      additionalFeatures, postData,
-      referrer) => {
-      const options = {
+      rawFeatures, referrer, postData) => {
+      const { options, additionalFeatures } = convertFeaturesString(rawFeatures, frameName);
+      const mergedOptions = {
         show: true,
         width: 800,
-        height: 600
+        height: 600,
+        ...options
       };
-      internalWindowOpen(event, url, referrer, frameName, disposition, options, additionalFeatures, postData);
+
+      internalWindowOpen(event, url, referrer, frameName, disposition, mergedOptions, additionalFeatures, postData);
     });
 
     // Create a new browser window for the native implementation of
     // "window.open", used in sandbox and nativeWindowOpen mode.
     this.on('-add-new-contents', (event, webContents, disposition,
-      userGesture, left, top, width, height, url, frameName) => {
+      userGesture, left, top, width, height, url, frameName,
+      referrer, rawFeatures, postData) => {
       if ((disposition !== 'foreground-tab' && disposition !== 'new-window' &&
            disposition !== 'background-tab')) {
         event.preventDefault();
         return;
       }
 
-      const options = {
+      const { options, additionalFeatures } = convertFeaturesString(rawFeatures, frameName);
+      const mergedOptions = {
         show: true,
-        x: left,
-        y: top,
-        width: width || 800,
-        height: height || 600,
-        webContents
+        width: 800,
+        height: 600,
+        webContents,
+        ...options
       };
-      const referrer = { url: '', policy: 'default' };
-      internalWindowOpen(event, url, referrer, frameName, disposition, options);
+
+      internalWindowOpen(event, url, referrer, frameName, disposition, mergedOptions, additionalFeatures, postData);
     });
   }
 

lib/browser/guest-view-manager.js

@@ -3,7 +3,7 @@
 const { webContents } = require('electron');
 const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal');
 const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils');
-const parseFeaturesString = require('@electron/internal/common/parse-features-string');
+const { parseFeaturesString } = require('@electron/internal/common/parse-features-string');
 const { syncMethods, asyncMethods, properties } = require('@electron/internal/common/web-view-methods');
 const { serialize } = require('@electron/internal/common/type-utils');
 
@@ -141,17 +141,6 @@ const createGuest = function (embedder, params) {
     }
   });
 
-  // Forward internal web contents event to embedder to handle
-  // native window.open setup
-  guest.on('-add-new-contents', (...args) => {
-    if (guest.getLastWebPreferences().nativeWindowOpen === true) {
-      const embedder = getEmbedder(guestInstanceId);
-      if (embedder != null) {
-        embedder.emit('-add-new-contents', ...args);
-      }
-    }
-  });
-
   return guestInstanceId;
 };
 

lib/browser/guest-window-manager.js

@@ -5,7 +5,7 @@ const { BrowserWindow } = electron;
 const { isSameOrigin } = process.electronBinding('v8_util');
 const { ipcMainInternal } = require('@electron/internal/browser/ipc-main-internal');
 const ipcMainUtils = require('@electron/internal/browser/ipc-main-internal-utils');
-const parseFeaturesString = require('@electron/internal/common/parse-features-string');
+const { convertFeaturesString } = require('@electron/internal/common/parse-features-string');
 
 const hasProp = {}.hasOwnProperty;
 const frameToGuest = new Map();
@@ -57,7 +57,11 @@ const mergeBrowserWindowOptions = function (embedder, options) {
     // if parent's visibility is available, that overrides 'show' flag (#12125)
     const win = BrowserWindow.fromWebContents(embedder.webContents);
     if (win != null) {
-      parentOptions = { ...embedder.browserWindowOptions, show: win.isVisible() };
+      parentOptions = {
+        ...win.getBounds(),
+        ...embedder.browserWindowOptions,
+        show: win.isVisible()
+      };
     }
 
     // Inherit the original options if it is a BrowserWindow.
@@ -83,6 +87,40 @@ const mergeBrowserWindowOptions = function (embedder, options) {
   return options;
 };
 
+const MULTIPART_CONTENT_TYPE = 'multipart/form-data';
+const URL_ENCODED_CONTENT_TYPE = 'application/x-www-form-urlencoded';
+function makeContentTypeHeader ({ contentType, boundary }) {
+  const header = `content-type: ${contentType};`;
+  if (contentType === MULTIPART_CONTENT_TYPE) {
+    return `${header} boundary=${boundary}`;
+  }
+  return header;
+}
+
+// Figure out appropriate headers for post data.
+const parseContentTypeFormat = function (postData) {
+  if (postData.length) {
+    // For multipart forms, the first element will start with the boundary
+    // notice, which looks something like `------WebKitFormBoundary12345678`
+    // Note, this regex would fail when submitting a urlencoded form with an
+    // input attribute of name="--theKey", but, uhh, don't do that?
+    const postDataFront = postData[0].bytes.toString();
+    const boundary = /^--.*[^-\r\n]/.exec(postDataFront);
+    if (boundary) {
+      return {
+        boundary: boundary[0].substr(2),
+        contentType: MULTIPART_CONTENT_TYPE
+      };
+    }
+  }
+  // Either the form submission didn't contain any inputs (the postData array
+  // was empty), or we couldn't find the boundary and thus we can assume this is
+  // a key=value style form.
+  return {
+    contentType: URL_ENCODED_CONTENT_TYPE
+  };
+};
+
 // Setup a new guest with |embedder|
 const setupGuest = function (embedder, frameName, guest, options) {
   // When |embedder| is destroyed we should also destroy attached guest, and if
@@ -134,14 +172,7 @@ const createGuest = function (embedder, url, referrer, frameName, options, postD
     };
     if (postData != null) {
       loadOptions.postData = postData;
-      loadOptions.extraHeaders = 'content-type: application/x-www-form-urlencoded';
-      if (postData.length > 0) {
-        const postDataFront = postData[0].bytes.toString();
-        const boundary = /^--.*[^-\r\n]/.exec(postDataFront);
-        if (boundary != null) {
-          loadOptions.extraHeaders = `content-type: multipart/form-data; boundary=${boundary[0].substr(2)}`;
-        }
-      }
+      loadOptions.extraHeaders = makeContentTypeHeader(parseContentTypeFormat(postData));
     }
     guest.loadURL(url, loadOptions);
   }
@@ -192,68 +223,21 @@ ipcMainInternal.on('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', (event, url, fra
   if (frameName == null) frameName = '';
   if (features == null) features = '';
 
-  const options = {};
-
-  const ints = ['x', 'y', 'width', 'height', 'minWidth', 'maxWidth', 'minHeight', 'maxHeight', 'zoomFactor'];
-  const webPreferences = ['zoomFactor', 'nodeIntegration', 'enableRemoteModule', 'preload', 'javascript', 'contextIsolation', 'webviewTag'];
   const disposition = 'new-window';
-
-  // Used to store additional features
-  const additionalFeatures = [];
-
-  // Parse the features
-  parseFeaturesString(features, function (key, value) {
-    if (value === undefined) {
-      additionalFeatures.push(key);
-    } else {
-      // Don't allow webPreferences to be set since it must be an object
-      // that cannot be directly overridden
-      if (key === 'webPreferences') return;
-
-      if (webPreferences.includes(key)) {
-        if (options.webPreferences == null) {
-          options.webPreferences = {};
-        }
-        options.webPreferences[key] = value;
-      } else {
-        options[key] = value;
-      }
-    }
-  });
-  if (options.left) {
-    if (options.x == null) {
-      options.x = options.left;
-    }
-  }
-  if (options.top) {
-    if (options.y == null) {
-      options.y = options.top;
-    }
-  }
-  if (options.title == null) {
-    options.title = frameName;
-  }
-  if (options.width == null) {
-    options.width = 800;
-  }
-  if (options.height == null) {
-    options.height = 600;
-  }
-
-  for (const name of ints) {
-    if (options[name] != null) {
-      options[name] = parseInt(options[name], 10);
-    }
-  }
-
+  const { options, additionalFeatures } = convertFeaturesString(features, frameName);
   const referrer = { url: '', policy: 'default' };
-  internalWindowOpen(event, url, referrer, frameName, disposition, options, additionalFeatures);
+  internalWindowOpen(event, url, referrer, frameName, disposition, options, additionalFeatures, null);
 });
 
 // Routed window.open messages with fully parsed options
 function internalWindowOpen (event, url, referrer, frameName, disposition, options, additionalFeatures, postData) {
   options = mergeBrowserWindowOptions(event.sender, options);
-  event.sender.emit('new-window', event, url, frameName, disposition, options, additionalFeatures, referrer);
+  const postBody = postData ? {
+    data: postData,
+    ...parseContentTypeFormat(postData)
+  } : null;
+
+  event.sender.emit('new-window', event, url, frameName, disposition, options, additionalFeatures, referrer, postBody);
   const { newGuest } = event;
   if ((event.sender.getType() === 'webview' && event.sender.getLastWebPreferences().disablePopups) || event.defaultPrevented) {
     if (newGuest != null) {