Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove nativeWindowOpen option #29405

Merged
merged 12 commits into from Jan 6, 2022
1 change: 0 additions & 1 deletion docs/README.md
Expand Up @@ -106,7 +106,6 @@ These individual tutorials expand on topics discussed in the guide above.
* [`File` Object](api/file-object.md)
* [`<webview>` Tag](api/webview-tag.md)
* [`window.open` Function](api/window-open.md)
* [`BrowserWindowProxy` Object](api/browser-window-proxy.md)

### Modules for the Main Process:

Expand Down
54 changes: 0 additions & 54 deletions docs/api/browser-window-proxy.md

This file was deleted.

3 changes: 0 additions & 3 deletions docs/api/browser-window.md
Expand Up @@ -341,9 +341,6 @@ It creates a new `BrowserWindow` with native properties as set by the `options`.
[Chrome Content Scripts][chrome-content-scripts]. You can access this
context in the dev tools by selecting the 'Electron Isolated Context'
entry in the combo box at the top of the Console tab.
* `nativeWindowOpen` boolean (optional) - Whether to use native
`window.open()`. Defaults to `true`. Child windows will always have node
integration disabled unless `nodeIntegrationInSubFrames` is true.
* `webviewTag` boolean (optional) - Whether to enable the [`<webview>` tag](webview-tag.md).
Defaults to `false`. **Note:** The
`preload` script configured for the `<webview>` will have node integration
Expand Down
14 changes: 0 additions & 14 deletions docs/api/structures/post-body.md
Expand Up @@ -7,17 +7,3 @@
the `enctype` attribute of the submitted HTML form.
* `boundary` string (optional) - The boundary used to separate multiple parts of
the message. Only valid when `contentType` is `multipart/form-data`.

Note that keys starting with `--` are not currently supported. For example, this will errantly submit as `multipart/form-data` when `nativeWindowOpen` is set to `false` in webPreferences:

```html
<form
target="_blank"
method="POST"
enctype="application/x-www-form-urlencoded"
action="https://postman-echo.com/post"
>
<input type="text" name="--theKey">
<input type="submit">
</form>
```
36 changes: 1 addition & 35 deletions docs/api/window-open.md
Expand Up @@ -12,10 +12,6 @@ useful for app sub-windows that act as preference panels, or similar, as the
parent can render to the sub-window directly, as if it were a `div` in the
parent. This is the same behavior as in the browser.

When `nativeWindowOpen` is set to false, `window.open` instead results in the
creation of a [`BrowserWindowProxy`](browser-window-proxy.md), a light wrapper
around `BrowserWindow`.

Electron pairs this native Chrome `Window` with a BrowserWindow under the hood.
You can take advantage of all the customization available when creating a
BrowserWindow in the main process by using `webContents.setWindowOpenHandler()`
Expand All @@ -34,7 +30,7 @@ because it is invoked in the main process.
* `frameName` string (optional)
* `features` string (optional)

Returns [`BrowserWindowProxy`](browser-window-proxy.md) | [`Window`](https://developer.mozilla.org/en-US/docs/Web/API/Window)
Returns [`Window`](https://developer.mozilla.org/en-US/docs/Web/API/Window) | null

`features` is a comma-separated key-value list, following the standard format of
the browser. Electron will parse `BrowserWindowConstructorOptions` out of this
Expand Down Expand Up @@ -108,33 +104,3 @@ mainWindow.webContents.setWindowOpenHandler(({ url }) => {
const childWindow = window.open('', 'modal')
childWindow.document.write('<h1>Hello</h1>')
```

### `BrowserWindowProxy` example

```javascript

// main.js
const mainWindow = new BrowserWindow({
webPreferences: { nativeWindowOpen: false }
})

mainWindow.webContents.setWindowOpenHandler(({ url }) => {
if (url.startsWith('https://github.com/')) {
return { action: 'allow' }
}
return { action: 'deny' }
})

mainWindow.webContents.on('did-create-window', (childWindow) => {
// For example...
childWindow.webContents.on('will-navigate', (e) => {
e.preventDefault()
})
})
```

```javascript
// renderer.js
const windowProxy = window.open('https://github.com/', null, 'minimizable=false')
windowProxy.postMessage('hi', '*')
```
12 changes: 12 additions & 0 deletions docs/breaking-changes.md
Expand Up @@ -12,6 +12,18 @@ This document uses the following convention to categorize breaking changes:
* **Deprecated:** An API was marked as deprecated. The API will continue to function, but will emit a deprecation warning, and will be removed in a future release.
* **Removed:** An API or feature was removed, and is no longer supported by Electron.

## Planned Breaking API Changes (18.0)

### Removed: `nativeWindowOpen`

Prior to Electron 15, `window.open` was by default shimmed to use
`BrowserWindowProxy`. This meant that `window.open('about:blank')` did not work
to open synchronously scriptable child windows, among other incompatibilities.
Sice Electron 15 `nativeWindowOpen` was enabled by default.
miniak marked this conversation as resolved.
Show resolved Hide resolved

See the documentation for [window.open in Electron](api/window-open.md)
for more details.

## Planned Breaking API Changes (17.0)

### Removed: `desktopCapturer.getSources` in the renderer
Expand Down
2 changes: 0 additions & 2 deletions filenames.auto.gni
Expand Up @@ -5,7 +5,6 @@ auto_filenames = {
"docs/api/app.md",
"docs/api/auto-updater.md",
"docs/api/browser-view.md",
"docs/api/browser-window-proxy.md",
"docs/api/browser-window.md",
"docs/api/client-request.md",
"docs/api/clipboard.md",
Expand Down Expand Up @@ -226,7 +225,6 @@ auto_filenames = {
"lib/browser/devtools.ts",
"lib/browser/guest-view-manager.ts",
"lib/browser/guest-window-manager.ts",
"lib/browser/guest-window-proxy.ts",
"lib/browser/init.ts",
"lib/browser/ipc-main-impl.ts",
"lib/browser/ipc-main-internal-utils.ts",
Expand Down
3 changes: 1 addition & 2 deletions lib/browser/api/web-contents.ts
Expand Up @@ -705,8 +705,7 @@ WebContents.prototype._init = function () {
}
});

// Create a new browser window for the native implementation of
// "window.open", used in sandbox and nativeWindowOpen mode.
// Create a new browser window for "window.open"
this.on('-add-new-contents' as any, (event: ElectronInternal.Event, webContents: Electron.WebContents, disposition: string,
_userGesture: boolean, _left: number, _top: number, _width: number, _height: number, url: string, frameName: string,
referrer: Electron.Referrer, rawFeatures: string, postData: PostData) => {
Expand Down
1 change: 0 additions & 1 deletion lib/browser/guest-view-manager.ts
Expand Up @@ -56,7 +56,6 @@ function makeWebPreferences (embedder: Electron.WebContents, params: Record<stri
const inheritedWebPreferences = new Map([
['contextIsolation', true],
['javascript', false],
['nativeWindowOpen', true],
['nodeIntegration', false],
['sandbox', true],
['nodeIntegrationInSubFrames', false],
Expand Down
48 changes: 7 additions & 41 deletions lib/browser/guest-window-manager.ts
@@ -1,14 +1,13 @@
/**
* Create and minimally track guest windows at the direction of the renderer
* (via window.open). Here, "guest" roughly means "child" — it's not necessarily
* emblematic of its process status; both in-process (same-origin
* nativeWindowOpen) and out-of-process (cross-origin nativeWindowOpen and
* BrowserWindowProxy) are created here. "Embedder" roughly means "parent."
* emblematic of its process status; both in-process (same-origin) and
* out-of-process (cross-origin) are created here. "Embedder" roughly means
* "parent."
*/
import { BrowserWindow } from 'electron/main';
import type { BrowserWindowConstructorOptions, Referrer, WebContents, LoadURLOptions } from 'electron/main';
import { parseFeatures } from '@electron/internal/browser/parse-features-string';
import { IPC_MESSAGES } from '@electron/internal/common/ipc-messages';

type PostData = LoadURLOptions['postData']
export type WindowOpenArgs = {
Expand All @@ -23,13 +22,12 @@ const unregisterFrameName = (name: string) => frameNamesToWindow.delete(name);
const getGuestWindowByFrameName = (name: string) => frameNamesToWindow.get(name);

/**
* `openGuestWindow` is called for both implementations of window.open
* (BrowserWindowProxy and nativeWindowOpen) to create and setup event handling
* for the new window.
* `openGuestWindow` is called to create and setup event handling for the new
* window.
*
* Until its removal in 12.0.0, the `new-window` event is fired, allowing the
* user to preventDefault() on the passed event (which ends up calling
* DestroyWebContents in the nativeWindowOpen code path).
* DestroyWebContents).
*/
export function openGuestWindow ({ event, embedder, guest, referrer, disposition, postData, overrideBrowserWindowOptions, windowOpenArgs }: {
event: { sender: WebContents, defaultPrevented: boolean },
Expand Down Expand Up @@ -78,22 +76,6 @@ export function openGuestWindow ({ event, embedder, guest, referrer, disposition
webContents: guest,
...browserWindowOptions
});
if (!guest) {
// We should only call `loadURL` if the webContents was constructed by us in
// the case of BrowserWindowProxy (non-sandboxed, nativeWindowOpen: false),
// as navigating to the url when creating the window from an existing
// webContents is not necessary (it will navigate there anyway).
// This can also happen if we enter this function from OpenURLFromTab, in
// which case the browser process is responsible for initiating navigation
// in the new window.
window.loadURL(url, {
httpReferrer: referrer,
...(postData && {
postData,
extraHeaders: formatPostDataHeaders(postData as Electron.UploadRawData[])
})
});
}

handleWindowLifecycleEvents({ embedder, frameName, guest: window });

Expand All @@ -118,9 +100,7 @@ const handleWindowLifecycleEvents = function ({ embedder, guest, frameName }: {
guest.destroy();
};

const cachedGuestId = guest.webContents.id;
const closedByUser = function () {
embedder._sendInternal(`${IPC_MESSAGES.GUEST_WINDOW_MANAGER_WINDOW_CLOSED}_${cachedGuestId}`);
embedder.removeListener('current-render-view-deleted' as any, closedByEmbedder);
};
embedder.once('current-render-view-deleted' as any, closedByEmbedder);
Expand Down Expand Up @@ -195,7 +175,6 @@ function emitDeprecatedNewWindowEvent ({ event, embedder, guest, windowOpenArgs,
const securityWebPreferences: { [key: string]: boolean } = {
contextIsolation: true,
javascript: false,
nativeWindowOpen: true,
nodeIntegration: false,
sandbox: true,
webviewTag: false,
Expand Down Expand Up @@ -245,30 +224,17 @@ export function makeWebPreferences ({ embedder, secureOverrideWebPreferences = {
}
return map;
}, {} as Electron.WebPreferences));
const openerId = parentWebPreferences.nativeWindowOpen ? null : embedder.id;

return {
...parsedWebPreferences,
// Note that order is key here, we want to disallow the renderer's
// ability to change important security options but allow main (via
// setWindowOpenHandler) to change them.
...securityWebPreferencesFromParent,
...secureOverrideWebPreferences,
// Sets correct openerId here to give correct options to 'new-window' event handler
// TODO: Figure out another way to pass this?
openerId
...secureOverrideWebPreferences
};
}

function formatPostDataHeaders (postData: PostData) {
if (!postData) return;

const { contentType, boundary } = parseContentTypeFormat(postData);
if (boundary != null) { return `content-type: ${contentType}; boundary=${boundary}`; }

return `content-type: ${contentType}`;
}

const MULTIPART_CONTENT_TYPE = 'multipart/form-data';
const URL_ENCODED_CONTENT_TYPE = 'application/x-www-form-urlencoded';

Expand Down