feat: add more info in setWindowOpenHandler details (#28518) (#29277)

* fix: invoke the window open handler for _blank links

* feat: add disposition to setWindowOpenHandler details

* fix: pass postData to new-window event

* postData can be heterogeneous

* fix type of postBody

* fix type of UploadFile and UploadRawData to be discriminated unions

* exclude the empty string from additionalFeatures

* add a test

* add postBody and referrer to setWindowOpenHandler args

* appease typescript

* Update api-browser-window-spec.ts

* update snapshots

Co-authored-by: Jeremy Rose <[email protected]>

docs/api/browser-window.md

@@ -1382,7 +1382,7 @@ Captures a snapshot of the page within `rect`. Omitting `rect` will capture the
   * `httpReferrer` (String | [Referrer](structures/referrer.md)) (optional) - An HTTP Referrer URL.
   * `userAgent` String (optional) - A user agent originating the request.
   * `extraHeaders` String (optional) - Extra headers separated by "\n"
-  * `postData` ([UploadRawData[]](structures/upload-raw-data.md) | [UploadFile[]](structures/upload-file.md)) (optional)
+  * `postData` ([UploadRawData](structures/upload-raw-data.md) | [UploadFile](structures/upload-file.md))[] (optional)
   * `baseURLForDataURL` String (optional) - Base URL (with trailing path separator) for files to be loaded by the data URL. This is needed only if the specified `url` is a data URL and needs to load other files.
 
 Returns `Promise<void>` - the promise will resolve when the page has finished loading

docs/api/structures/post-body.md

@@ -1,6 +1,6 @@
 # PostBody Object
 
-* `data` Array<[PostData](./post-data.md)> - The post data to be sent to the
+* `data` ([UploadRawData](upload-raw-data.md) | [UploadFile](upload-file.md))[] - The post data to be sent to the
   new window.
 * `contentType` String - The `content-type` header used for the data. One of
   `application/x-www-form-urlencoded` or `multipart/form-data`. Corresponds to

docs/api/structures/post-data.md

@@ -1,21 +0,0 @@
-# PostData Object
-
-* `type` String - One of the following:
-  * `rawData` - The data is available as a `Buffer`, in the `rawData` field.
-  * `file` - The object represents a file. The `filePath`, `offset`, `length`
-    and `modificationTime` fields will be used to describe the file.
-  * `blob` - The object represents a `Blob`. The `blobUUID` field will be used
-    to describe the `Blob`.
-* `bytes` String (optional) - The raw bytes of the post data in a `Buffer`.
-  Required for the `rawData` type.
-* `filePath` String (optional) - The path of the file being uploaded. Required
-  for the `file` type.
-* `blobUUID` String (optional) - The `UUID` of the `Blob` being uploaded.
-  Required for the `blob` type.
-* `offset` Integer (optional) - The offset from the beginning of the file being
-  uploaded, in bytes. Only valid for `file` types.
-* `length` Integer (optional) - The length of the file being uploaded, in bytes.
-  If set to `-1`, the whole file will be uploaded. Only valid for `file` types.
-* `modificationTime` Double (optional) - The modification time of the file
-  represented by a double, which is the number of seconds since the `UNIX Epoch`
-  (Jan 1, 1970). Only valid for `file` types.

docs/api/web-contents.md

@@ -914,7 +914,7 @@ in `webPreferences`.
   * `httpReferrer` (String | [Referrer](structures/referrer.md)) (optional) - An HTTP Referrer url.
   * `userAgent` String (optional) - A user agent originating the request.
   * `extraHeaders` String (optional) - Extra headers separated by "\n".
-  * `postData` ([UploadRawData[]](structures/upload-raw-data.md) | [UploadFile[]](structures/upload-file.md)) (optional)
+  * `postData` ([UploadRawData](structures/upload-raw-data.md) | [UploadFile](structures/upload-file.md))[] (optional)
   * `baseURLForDataURL` String (optional) - Base url (with trailing path separator) for files to be loaded by the data url. This is needed only if the specified `url` is a data url and needs to load other files.
 
 Returns `Promise<void>` - the promise will resolve when the page has finished loading
@@ -1184,6 +1184,15 @@ Ignore application menu shortcuts while this web contents is focused.
     * `url` String - The _resolved_ version of the URL passed to `window.open()`. e.g. opening a window with `window.open('foo')` will yield something like `https://the-origin/the/current/path/foo`.
     * `frameName` String - Name of the window provided in `window.open()`
     * `features` String - Comma separated list of window features provided to `window.open()`.
+    * `disposition` String - Can be `default`, `foreground-tab`, `background-tab`,
+      `new-window`, `save-to-disk` or `other`.
+    * `referrer` [Referrer](structures/referrer.md) - The referrer that will be
+      passed to the new window. May or may not result in the `Referer` header being
+      sent, depending on the referrer policy.
+    * `postBody` [PostBody](structures/post-body.md) (optional) - The post data that
+      will be sent to the new window, along with the appropriate headers that will
+      be set. If no post data is to be sent, the value will be `null`. Only defined
+      when the window is being created by a form that set `target=_blank`.
 
   Returns `{action: 'deny'} | {action: 'allow', overrideBrowserWindowOptions?: BrowserWindowConstructorOptions}` - `deny` cancels the creation of the new
   window. `allow` will allow the new window to be created. Specifying `overrideBrowserWindowOptions` allows customization of the created window.

docs/api/webview-tag.md

@@ -274,7 +274,7 @@ webview.addEventListener('dom-ready', () => {
   * `httpReferrer` (String | [Referrer](structures/referrer.md)) (optional) - An HTTP Referrer url.
   * `userAgent` String (optional) - A user agent originating the request.
   * `extraHeaders` String (optional) - Extra headers separated by "\n"
-  * `postData` ([UploadRawData[]](structures/upload-raw-data.md) | [UploadFile[]](structures/upload-file.md)) (optional)
+  * `postData` ([UploadRawData](structures/upload-raw-data.md) | [UploadFile](structures/upload-file.md))[] (optional)
   * `baseURLForDataURL` String (optional) - Base url (with trailing path separator) for files to be loaded by the data url. This is needed only if the specified `url` is a data url and needs to load other files.
 
 Returns `Promise<void>` - The promise will resolve when the page has finished loading

filenames.auto.gni

@@ -103,7 +103,6 @@ auto_filenames = {
     "docs/api/structures/notification-response.md",
     "docs/api/structures/point.md",
     "docs/api/structures/post-body.md",
-    "docs/api/structures/post-data.md",
     "docs/api/structures/printer-info.md",
     "docs/api/structures/process-memory-info.md",
     "docs/api/structures/process-metric.md",

lib/browser/api/web-contents.ts

@@ -3,7 +3,7 @@ import type { BrowserWindowConstructorOptions, LoadURLOptions } from 'electron/m
 
 import * as url from 'url';
 import * as path from 'path';
-import { openGuestWindow, makeWebPreferences } from '@electron/internal/browser/guest-window-manager';
+import { openGuestWindow, makeWebPreferences, parseContentTypeFormat } from '@electron/internal/browser/guest-window-manager';
 import { NavigationController } from '@electron/internal/browser/navigation-controller';
 import { ipcMainInternal } from '@electron/internal/browser/ipc-main-internal';
 import * as ipcMainUtils from '@electron/internal/browser/ipc-main-internal-utils';
@@ -411,11 +411,11 @@ WebContents.prototype.setWindowOpenHandler = function (handler: (details: Electr
   this._windowOpenHandler = handler;
 };
 
-WebContents.prototype._callWindowOpenHandler = function (event: Electron.Event, url: string, frameName: string, rawFeatures: string): BrowserWindowConstructorOptions | null {
+WebContents.prototype._callWindowOpenHandler = function (event: Electron.Event, details: Electron.HandlerDetails): BrowserWindowConstructorOptions | null {
   if (!this._windowOpenHandler) {
     return null;
   }
-  const response = this._windowOpenHandler({ url, frameName, features: rawFeatures });
+  const response = this._windowOpenHandler(details);
 
   if (typeof response !== 'object') {
     event.preventDefault();
@@ -571,9 +571,21 @@ WebContents.prototype._init = function () {
 
   if (this.getType() !== 'remote') {
     // Make new windows requested by links behave like "window.open".
-    this.on('-new-window' as any, (event: ElectronInternal.Event, url: string, frameName: string, disposition: string,
+    this.on('-new-window' as any, (event: ElectronInternal.Event, url: string, frameName: string, disposition: Electron.HandlerDetails['disposition'],
       rawFeatures: string, referrer: Electron.Referrer, postData: PostData) => {
-      const options = this._callWindowOpenHandler(event, url, frameName, rawFeatures);
+      const postBody = postData ? {
+        data: postData,
+        ...parseContentTypeFormat(postData)
+      } : undefined;
+      const details: Electron.HandlerDetails = {
+        url,
+        frameName,
+        features: rawFeatures,
+        referrer,
+        postBody,
+        disposition
+      };
+      const options = this._callWindowOpenHandler(event, details);
       if (!event.defaultPrevented) {
         openGuestWindow({
           event,
@@ -582,18 +594,26 @@ WebContents.prototype._init = function () {
           referrer,
           postData,
           overrideBrowserWindowOptions: options || {},
-          windowOpenArgs: {
-            url,
-            frameName,
-            features: rawFeatures
-          }
+          windowOpenArgs: details
         });
       }
     });
 
     let windowOpenOverriddenOptions: BrowserWindowConstructorOptions | null = null;
-    this.on('-will-add-new-contents' as any, (event: ElectronInternal.Event, url: string, frameName: string, rawFeatures: string) => {
-      windowOpenOverriddenOptions = this._callWindowOpenHandler(event, url, frameName, rawFeatures);
+    this.on('-will-add-new-contents' as any, (event: ElectronInternal.Event, url: string, frameName: string, rawFeatures: string, disposition: Electron.HandlerDetails['disposition'], referrer: Electron.Referrer, postData: PostData) => {
+      const postBody = postData ? {
+        data: postData,
+        ...parseContentTypeFormat(postData)
+      } : undefined;
+      const details: Electron.HandlerDetails = {
+        url,
+        frameName,
+        features: rawFeatures,
+        disposition,
+        referrer,
+        postBody
+      };
+      windowOpenOverriddenOptions = this._callWindowOpenHandler(event, details);
       if (!event.defaultPrevented) {
         const secureOverrideWebPreferences = windowOpenOverriddenOptions ? {
           // Allow setting of backgroundColor as a webPreference even though

lib/browser/guest-window-manager.ts

@@ -292,7 +292,7 @@ const MULTIPART_CONTENT_TYPE = 'multipart/form-data';
 const URL_ENCODED_CONTENT_TYPE = 'application/x-www-form-urlencoded';
 
 // Figure out appropriate headers for post data.
-const parseContentTypeFormat = function (postData: Exclude<PostData, undefined>) {
+export const parseContentTypeFormat = function (postData: Exclude<PostData, undefined>) {
   if (postData.length) {
     if (postData[0].type === 'rawData') {
       // For multipart forms, the first element will start with the boundary

lib/browser/guest-window-proxy.ts

@@ -73,14 +73,15 @@ ipcMainInternal.on(
       );
     }
 
-    const browserWindowOptions = event.sender._callWindowOpenHandler(event, url, frameName, features);
+    const referrer: Electron.Referrer = { url: '', policy: 'strict-origin-when-cross-origin' };
+    const browserWindowOptions = event.sender._callWindowOpenHandler(event, { url, frameName, features, disposition: 'new-window', referrer });
     if (event.defaultPrevented) {
       return;
     }
     const guest = openGuestWindow({
       event,
       embedder: event.sender,
-      referrer: { url: '', policy: 'default' },
+      referrer,
       disposition: 'new-window',
       overrideBrowserWindowOptions: browserWindowOptions!,
       windowOpenArgs: {

shell/browser/api/electron_api_web_contents.cc

@@ -1013,8 +1013,9 @@ bool WebContents::IsWebContentsCreationOverridden(
     content::mojom::WindowContainerType window_container_type,
     const GURL& opener_url,
     const content::mojom::CreateNewWindowParams& params) {
-  bool default_prevented = Emit("-will-add-new-contents", params.target_url,
-                                params.frame_name, params.raw_features);
+  bool default_prevented = Emit(
+      "-will-add-new-contents", params.target_url, params.frame_name,
+      params.raw_features, params.disposition, *params.referrer, params.body);
   // If the app prevented the default, redirect to CreateCustomWebContents,
   // which always returns nullptr, which will result in the window open being
   // prevented (window.open() will return null in the renderer).

shell/common/gin_converters/blink_converter.cc

@@ -13,6 +13,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "gin/converter.h"
 #include "shell/common/gin_converters/gfx_converter.h"
+#include "shell/common/gin_converters/gurl_converter.h"
 #include "shell/common/gin_converters/value_converter.h"
 #include "shell/common/gin_helper/dictionary.h"
 #include "shell/common/keyboard_util.h"
@@ -23,6 +24,7 @@
 #include "third_party/blink/public/common/input/web_mouse_event.h"
 #include "third_party/blink/public/common/input/web_mouse_wheel_event.h"
 #include "third_party/blink/public/common/widget/device_emulation_params.h"
+#include "third_party/blink/public/mojom/loader/referrer.mojom.h"
 #include "ui/base/clipboard/clipboard.h"
 #include "ui/events/blink/blink_event_util.h"
 #include "ui/events/keycodes/dom/keycode_converter.h"
@@ -481,6 +483,33 @@ bool Converter<network::mojom::ReferrerPolicy>::FromV8(
   return true;
 }
 
+// static
+v8::Local<v8::Value> Converter<blink::mojom::Referrer>::ToV8(
+    v8::Isolate* isolate,
+    const blink::mojom::Referrer& val) {
+  gin_helper::Dictionary dict = gin::Dictionary::CreateEmpty(isolate);
+  dict.Set("url", ConvertToV8(isolate, val.url));
+  dict.Set("policy", ConvertToV8(isolate, val.policy));
+  return gin::ConvertToV8(isolate, dict);
+}
+//
+// static
+bool Converter<blink::mojom::Referrer>::FromV8(v8::Isolate* isolate,
+                                               v8::Local<v8::Value> val,
+                                               blink::mojom::Referrer* out) {
+  gin_helper::Dictionary dict;
+  if (!ConvertFromV8(isolate, val, &dict))
+    return false;
+
+  if (!dict.Get("url", &out->url))
+    return false;
+
+  if (!dict.Get("policy", &out->policy))
+    return false;
+
+  return true;
+}
+
 v8::Local<v8::Value> Converter<blink::CloneableMessage>::ToV8(
     v8::Isolate* isolate,
     const blink::CloneableMessage& in) {

shell/common/gin_converters/blink_converter.h

@@ -10,6 +10,7 @@
 #include "third_party/blink/public/common/input/web_input_event.h"
 #include "third_party/blink/public/common/messaging/cloneable_message.h"
 #include "third_party/blink/public/common/web_cache/web_cache_resource_type_stats.h"
+#include "third_party/blink/public/mojom/loader/referrer.mojom-forward.h"
 
 namespace blink {
 class WebMouseEvent;
@@ -95,6 +96,15 @@ struct Converter<network::mojom::ReferrerPolicy> {
                      network::mojom::ReferrerPolicy* out);
 };
 
+template <>
+struct Converter<blink::mojom::Referrer> {
+  static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,
+                                   const blink::mojom::Referrer& val);
+  static bool FromV8(v8::Isolate* isolate,
+                     v8::Local<v8::Value> val,
+                     blink::mojom::Referrer* out);
+};
+
 template <>
 struct Converter<blink::CloneableMessage> {
   static v8::Local<v8::Value> ToV8(v8::Isolate* isolate,

spec-main/api-browser-window-spec.ts

@@ -3249,7 +3249,7 @@ describe('BrowserWindow module', () => {
       expect(additionalFeatures).to.deep.equal([]);
       expect(postBody.data).to.have.length(1);
       expect(postBody.data[0].type).to.equal('rawData');
-      expect(postBody.data[0].bytes).to.deep.equal(Buffer.from('post-test-key=post-test-value'));
+      expect((postBody.data[0] as any).bytes).to.deep.equal(Buffer.from('post-test-key=post-test-value'));
       expect(postBody.contentType).to.equal('application/x-www-form-urlencoded');
     });
   });

spec-main/fixtures/snapshots/proxy-window-open.snapshot.txt

@@ -30,7 +30,7 @@
     [],
     {
       "url": "",
-      "policy": "default"
+      "policy": "strict-origin-when-cross-origin"
     },
     null
   ],
@@ -64,7 +64,7 @@
     [],
     {
       "url": "",
-      "policy": "default"
+      "policy": "strict-origin-when-cross-origin"
     },
     null
   ],
@@ -98,7 +98,7 @@
     [],
     {
       "url": "",
-      "policy": "default"
+      "policy": "strict-origin-when-cross-origin"
     },
     null
   ],
@@ -131,7 +131,7 @@
     [],
     {
       "url": "",
-      "policy": "default"
+      "policy": "strict-origin-when-cross-origin"
     },
     null
   ],
@@ -165,7 +165,7 @@
     [],
     {
       "url": "",
-      "policy": "default"
+      "policy": "strict-origin-when-cross-origin"
     },
     null
   ]

spec-main/guest-window-manager-spec.ts

@@ -115,7 +115,6 @@ describe('webContents.setWindowOpenHandler', () => {
       beforeEach(async () => {
         browserWindow = new BrowserWindow(browserWindowOptions);
         await browserWindow.loadURL('about:blank');
-        browserWindow.show();
       });
 
       afterEach(closeAllWindows);
@@ -135,7 +134,7 @@ describe('webContents.setWindowOpenHandler', () => {
           assert.fail('did-create-window should not to be called with an overridden window.open');
         });
 
-        browserWindow.webContents.executeJavaScript("window.open('about:blank') && true");
+        browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no') && true");
 
         await denied;
       });
@@ -184,26 +183,66 @@ describe('webContents.setWindowOpenHandler', () => {
         await denied;
       });
 
-      it('fires handler with correct params', (done) => {
+      it('fires handler with correct params', async () => {
         const testFrameName = 'test-frame-name';
-        const testFeatures = 'top=10&left=10&something-unknown';
+        const testFeatures = 'top=10&left=10&something-unknown&show=no';
         const testUrl = 'app://does-not-exist/';
-        browserWindow.webContents.setWindowOpenHandler(({ url, frameName, features }) => {
-          expect(url).to.equal(testUrl);
-          expect(frameName).to.equal(testFrameName);
-          expect(features).to.equal(testFeatures);
-          done();
-          return { action: 'deny' };
+        const details = await new Promise<Electron.HandlerDetails>(resolve => {
+          browserWindow.webContents.setWindowOpenHandler((details) => {
+            setTimeout(() => resolve(details));
+            return { action: 'deny' };
+          });
+
+          browserWindow.webContents.executeJavaScript(`window.open('${testUrl}', '${testFrameName}', '${testFeatures}') && true`);
+        });
+        const { url, frameName, features, disposition, referrer } = details;
+        expect(url).to.equal(testUrl);
+        expect(frameName).to.equal(testFrameName);
+        expect(features).to.equal(testFeatures);
+        expect(disposition).to.equal('new-window');
+        expect(referrer).to.deep.equal({
+          policy: 'strict-origin-when-cross-origin',
+          url: ''
         });
+      });
 
-        browserWindow.webContents.executeJavaScript(`window.open('${testUrl}', '${testFrameName}', '${testFeatures}') && true`);
+      it('includes post body', async () => {
+        const details = await new Promise<Electron.HandlerDetails>(resolve => {
+          browserWindow.webContents.setWindowOpenHandler((details) => {
+            setTimeout(() => resolve(details));
+            return { action: 'deny' };
+          });
+
+          browserWindow.webContents.loadURL(`data:text/html,${encodeURIComponent(`
+            <form action="http://example.com" target="_blank" method="POST" id="form">
+              <input name="key" value="value"></input>
+            </form>
+            <script>form.submit()</script>
+          `)}`);
+        });
+        const { url, frameName, features, disposition, referrer, postBody } = details;
+        expect(url).to.equal('http://example.com/');
+        expect(frameName).to.equal('');
+        expect(features).to.deep.equal('');
+        expect(disposition).to.equal('foreground-tab');
+        expect(referrer).to.deep.equal({
+          policy: 'strict-origin-when-cross-origin',
+          url: ''
+        });
+        expect(postBody).to.deep.equal({
+          contentType: 'application/x-www-form-urlencoded',
+          data: [{
+            type: 'rawData',
+            bytes: Buffer.from('key=value')
+          }]
+        });
       });
 
       it('does fire window creation events if an override returns action: allow', async () => {
         browserWindow.webContents.setWindowOpenHandler(() => ({ action: 'allow' }));
 
         setImmediate(() => {
-          browserWindow.webContents.executeJavaScript("window.open('about:blank') && true");
+          browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no') && true");
         });
 
         await Promise.all([
@@ -222,7 +261,7 @@ describe('webContents.setWindowOpenHandler', () => {
           done();
         });
 
-        browserWindow.webContents.executeJavaScript("window.open('about:blank') && true");
+        browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no') && true");
       });
     });
   }

typings/internal-electron.d.ts

@@ -69,8 +69,8 @@ declare namespace Electron {
     equal(other: WebContents): boolean;
     _initiallyShown: boolean;
     browserWindowOptions: BrowserWindowConstructorOptions;
-    _windowOpenHandler: ((opts: {url: string, frameName: string, features: string}) => any) | null;
-    _callWindowOpenHandler(event: any, url: string, frameName: string, rawFeatures: string): Electron.BrowserWindowConstructorOptions | null;
+    _windowOpenHandler: ((details: Electron.HandlerDetails) => any) | null;
+    _callWindowOpenHandler(event: any, details: Electron.HandlerDetails): Electron.BrowserWindowConstructorOptions | null;
     _setNextChildWebPreferences(prefs: Partial<Electron.BrowserWindowConstructorOptions['webPreferences']> & Pick<Electron.BrowserWindowConstructorOptions, 'backgroundColor'>): void;
     _send(internal: boolean, channel: string, args: any): boolean;
     _sendToFrameInternal(frameId: number | [number, number], channel: string, ...args: any[]): boolean;