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 BrowserWindow option inheritance #28550

Merged
merged 11 commits into from Apr 21, 2021
13 changes: 8 additions & 5 deletions docs/api/web-contents.md
Expand Up @@ -202,9 +202,9 @@ Returns:
* `frameName` String - Name given to the created window in the
`window.open()` call.
* `options` BrowserWindowConstructorOptions - The options used to create the
BrowserWindow. They are merged in increasing precedence: options inherited
from the parent, parsed options from the `features` string from
`window.open()`, and options given by
BrowserWindow. They are merged in increasing precedence: parsed options
from the `features` string from `window.open()`, security-related
webPreferences inherited from the parent, and options given by
[`webContents.setWindowOpenHandler`](web-contents.md#contentssetwindowopenhandlerhandler).
Unrecognized options are not filtered out.
* `additionalFeatures` String[] - The non-standard features (features not
Expand Down Expand Up @@ -1138,8 +1138,11 @@ Ignore application menu shortcuts while this web contents is focused.
without a recognized 'action' value will result in a console error and have
the same effect as returning `{action: 'deny'}`.

Called before creating a window when `window.open()` is called from the
renderer. See [`window.open()`](window-open.md) for more details and how to use this in conjunction with `did-create-window`.
Called before creating a window a new window is requested by the renderer, e.g.
by `window.open()`, a link with `target="_blank"`, shift+clicking on a link, or
submitting a form with `<form target="_blank">`. See
[`window.open()`](window-open.md) for more details and how to use this in
conjunction with `did-create-window`.

#### `contents.setAudioMuted(muted)`

Expand Down
5 changes: 2 additions & 3 deletions docs/api/window-open.md
Expand Up @@ -23,9 +23,8 @@ BrowserWindow in the main process by using `webContents.setWindowOpenHandler()`
for renderer-created windows.

BrowserWindow constructor options are set by, in increasing precedence
order: options inherited from the parent, parsed options
from the `features` string from `window.open()`, security-related webPreferences
inherited from the parent, and options given by
order: parsed options from the `features` string from `window.open()`,
security-related webPreferences inherited from the parent, and options given by
[`webContents.setWindowOpenHandler`](web-contents.md#contentssetwindowopenhandlerhandler).
Note that `webContents.setWindowOpenHandler` has final say and full privilege
because it is invoked in the main process.
Expand Down
21 changes: 21 additions & 0 deletions docs/breaking-changes.md
Expand Up @@ -28,6 +28,27 @@ ensure your code works with this property enabled. It has been enabled by defau

You will be affected by this change if you use either `webFrame.executeJavaScript` or `webFrame.executeJavaScriptInIsolatedWorld`. You will need to ensure that values returned by either of those methods are supported by the [Context Bridge API](api/context-bridge.md#parameter--error--return-type-support) as these methods use the same value passing semantics.

### Removed: BrowserWindowConstructorOptions inheriting from parent windows

Prior to Electron 14, windows opened with `window.open` would inherit
BrowserWindow constructor options such as `transparent` and `resizable` from
their parent window. Beginning with Electron 14, this behavior is removed, and
windows will not inherit any BrowserWindow constructor options from their
parents.

Instead, explicitly set options for the new window with `setWindowOpenHandler`:

```js
webContents.setWindowOpenHandler((details) => {
return {
action: 'allow',
overrideBrowserWindowOptions: {
// ...
}
}
})
```

## Planned Breaking API Changes (13.0)

### API Changed: `session.setPermissionCheckHandler(handler)`
Expand Down
36 changes: 7 additions & 29 deletions lib/browser/guest-window-manager.ts
Expand Up @@ -200,42 +200,40 @@ const securityWebPreferences: { [key: string]: boolean } = {
enableWebSQL: false
};

function makeBrowserWindowOptions ({ embedder, features, overrideOptions, useDeprecatedBehaviorForBareValues = true, useDeprecatedBehaviorForOptionInheritance = true }: {
function makeBrowserWindowOptions ({ embedder, features, overrideOptions, useDeprecatedBehaviorForBareValues = true }: {
embedder: WebContents,
features: string,
overrideOptions?: BrowserWindowConstructorOptions,
useDeprecatedBehaviorForBareValues?: boolean
useDeprecatedBehaviorForOptionInheritance?: boolean
}) {
const { options: parsedOptions, webPreferences: parsedWebPreferences, additionalFeatures } = parseFeatures(features, useDeprecatedBehaviorForBareValues);

const deprecatedInheritedOptions = getDeprecatedInheritedOptions(embedder);

return {
additionalFeatures,
options: {
...(useDeprecatedBehaviorForOptionInheritance && deprecatedInheritedOptions),
show: true,
width: 800,
height: 600,
...parsedOptions,
...overrideOptions,
webPreferences: makeWebPreferences({ embedder, insecureParsedWebPreferences: parsedWebPreferences, secureOverrideWebPreferences: overrideOptions && overrideOptions.webPreferences, useDeprecatedBehaviorForOptionInheritance: true })
webPreferences: makeWebPreferences({
embedder,
insecureParsedWebPreferences: parsedWebPreferences,
secureOverrideWebPreferences: overrideOptions && overrideOptions.webPreferences
})
} as Electron.BrowserViewConstructorOptions
};
}

export function makeWebPreferences ({ embedder, secureOverrideWebPreferences = {}, insecureParsedWebPreferences: parsedWebPreferences = {}, useDeprecatedBehaviorForOptionInheritance = true }: {
export function makeWebPreferences ({ embedder, secureOverrideWebPreferences = {}, insecureParsedWebPreferences: parsedWebPreferences = {} }: {
embedder: WebContents,
insecureParsedWebPreferences?: ReturnType<typeof parseFeatures>['webPreferences'],
// Note that override preferences are considered elevated, and should only be
// sourced from the main process, as they override security defaults. If you
// have unvetted prefs, use parsedWebPreferences.
secureOverrideWebPreferences?: BrowserWindowConstructorOptions['webPreferences'],
useDeprecatedBehaviorForBareValues?: boolean
useDeprecatedBehaviorForOptionInheritance?: boolean
}) {
const deprecatedInheritedOptions = getDeprecatedInheritedOptions(embedder);
const parentWebPreferences = embedder.getLastWebPreferences();
const securityWebPreferencesFromParent = (Object.keys(securityWebPreferences).reduce((map, key) => {
if (securityWebPreferences[key] === parentWebPreferences[key as keyof Electron.WebPreferences]) {
Expand All @@ -246,7 +244,6 @@ export function makeWebPreferences ({ embedder, secureOverrideWebPreferences = {
const openerId = parentWebPreferences.nativeWindowOpen ? null : embedder.id;

return {
...(useDeprecatedBehaviorForOptionInheritance && deprecatedInheritedOptions ? deprecatedInheritedOptions.webPreferences : null),
...parsedWebPreferences,
// Note that order is key here, we want to disallow the renderer's
// ability to change important security options but allow main (via
Expand All @@ -259,25 +256,6 @@ export function makeWebPreferences ({ embedder, secureOverrideWebPreferences = {
};
}

/**
* Current Electron behavior is to inherit all options from the parent window.
* In practical use, this is kind of annoying because consumers have to know
* about the parent window's preferences in order to unset them and makes child
* windows even more of an anomaly. In 11.0.0 we will remove this behavior and
* only critical security preferences will be inherited by default.
*/
function getDeprecatedInheritedOptions (embedder: WebContents) {
if (!embedder.browserWindowOptions) {
// If it's a webview, return just the webPreferences.
return {
webPreferences: embedder.getLastWebPreferences()
};
}

const { type, show, ...inheritableOptions } = embedder.browserWindowOptions;
return inheritableOptions;
}

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

Expand Down
4 changes: 0 additions & 4 deletions shell/browser/api/electron_api_browser_window.cc
Expand Up @@ -78,10 +78,6 @@ BrowserWindow::BrowserWindow(gin::Arguments* args,
api_web_contents_->AddObserver(this);
Observe(api_web_contents_->web_contents());

// Keep a copy of the options for later use.
gin_helper::Dictionary(isolate, web_contents.ToV8().As<v8::Object>())
.Set("browserWindowOptions", options);

// Associate with BrowserWindow.
web_contents->SetOwnerWindow(window());

Expand Down
24 changes: 9 additions & 15 deletions spec-main/api-browser-window-spec.ts
Expand Up @@ -2372,7 +2372,7 @@ describe('BrowserWindow module', () => {
}
});
let childWc: WebContents | null = null;
w.webContents.setWindowOpenHandler(() => ({ action: 'allow', overrideBrowserWindowOptions: { webPreferences: { preload } } }));
w.webContents.setWindowOpenHandler(() => ({ action: 'allow', overrideBrowserWindowOptions: { webPreferences: { preload, contextIsolation: false } } }));

w.webContents.on('did-create-window', (win) => {
childWc = win.webContents;
Expand Down Expand Up @@ -2587,6 +2587,10 @@ describe('BrowserWindow module', () => {
preload
}
});
w.webContents.setWindowOpenHandler(() => ({
action: 'allow',
overrideBrowserWindowOptions: { show: false, webPreferences: { contextIsolation: false, webviewTag: true, nativeWindowOpen: true, nodeIntegrationInSubFrames: true } }
}));
w.webContents.once('new-window', (event, url, frameName, disposition, options) => {
options.show = false;
});
Expand Down Expand Up @@ -2665,7 +2669,9 @@ describe('BrowserWindow module', () => {
action: 'allow',
overrideBrowserWindowOptions: {
webPreferences: {
preload: path.join(fixtures, 'api', 'window-open-preload.js')
preload: path.join(fixtures, 'api', 'window-open-preload.js'),
contextIsolation: false,
nodeIntegrationInSubFrames: true
}
}
}));
Expand Down Expand Up @@ -3075,19 +3081,7 @@ describe('BrowserWindow module', () => {
expect(url).to.equal('http://example.com/test');
expect(frameName).to.equal('');
expect(disposition).to.equal('foreground-tab');
expect(options).to.deep.equal({
show: true,
width: 800,
height: 600,
webPreferences: {
contextIsolation: true,
nodeIntegration: false,
webviewTag: false,
nodeIntegrationInSubFrames: false,
openerId: options.webPreferences!.openerId
},
webContents: undefined
});
expect(options).to.be.an('object').not.null();
expect(referrer.policy).to.equal('strict-origin-when-cross-origin');
expect(referrer.url).to.equal('');
expect(additionalFeatures).to.deep.equal([]);
Expand Down
2 changes: 1 addition & 1 deletion spec-main/chromium-spec.ts
Expand Up @@ -970,7 +970,7 @@ describe('chromium features', () => {

const parentCode = `new Promise((resolve) => {
// This is context (3), a child window of the WebView.
const child = window.open(${JSON.stringify(child)}, "", "show=no")
const child = window.open(${JSON.stringify(child)}, "", "show=no,contextIsolation=no,nodeIntegration=yes")
window.addEventListener("message", e => {
resolve(e.data)
})
Expand Down
89 changes: 36 additions & 53 deletions spec-main/fixtures/snapshots/native-window-open.snapshot.txt
Expand Up @@ -8,27 +8,23 @@
"frame-name",
"new-window",
{
"show": true,
"width": 800,
"title": "cool",
"backgroundColor": "blue",
"focusable": false,
"height": 600,
"top": 5,
"left": 10,
"resizable": false,
"x": 10,
"y": 5,
"webPreferences": {
"nativeWindowOpen": true,
"sandbox": true,
"backgroundColor": "blue",
"contextIsolation": true,
"nativeWindowOpen": true,
"nodeIntegration": false,
"sandbox": true,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
},
"show": true,
"height": 600,
"top": 5,
"left": 10,
"resizable": false,
"x": 10,
"y": 5,
"webContents": "[WebContents]"
},
[],
Expand All @@ -47,26 +43,22 @@
"frame-name",
"new-window",
{
"show": true,
"width": 800,
"title": "cool",
"backgroundColor": "blue",
"focusable": false,
"height": 600,
"resizable": false,
"x": 0,
"y": 10,
"webPreferences": {
"nativeWindowOpen": true,
"sandbox": true,
"backgroundColor": "blue",
"zoomFactor": "2",
"contextIsolation": true,
"nativeWindowOpen": true,
"nodeIntegration": false,
"sandbox": true,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
},
"show": true,
"height": 600,
"resizable": false,
"x": 0,
"y": 10,
"webContents": "[WebContents]"
},
[],
Expand All @@ -85,22 +77,20 @@
"frame-name",
"new-window",
{
"show": true,
"width": 800,
"title": "cool",
"height": 600,
"backgroundColor": "gray",
"focusable": false,
"webPreferences": {
"nativeWindowOpen": true,
"sandbox": true,
"backgroundColor": "gray",
"contextIsolation": true,
"nativeWindowOpen": true,
"nodeIntegration": false,
"sandbox": true,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
"openerId": null,
"backgroundColor": "gray"
},
"show": true,
"height": 600,
"x": 100,
"y": 100,
"webContents": "[WebContents]"
Expand All @@ -121,24 +111,21 @@
"frame-name",
"new-window",
{
"show": true,
"width": 800,
"height": 600,
"x": 50,
"y": 20,
"title": "sup",
"backgroundColor": "blue",
"focusable": false,
"webPreferences": {
"nativeWindowOpen": true,
"sandbox": true,
"backgroundColor": "blue",
"contextIsolation": true,
"nativeWindowOpen": true,
"nodeIntegration": false,
"sandbox": true,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
},
"show": true,
"height": 600,
"x": 50,
"y": 20,
"webContents": "[WebContents]"
},
[],
Expand All @@ -157,26 +144,22 @@
"frame-name",
"new-window",
{
"show": false,
"width": 800,
"title": "cool",
"backgroundColor": "blue",
"focusable": false,
"height": 600,
"top": 1,
"left": 1,
"x": 1,
"y": 1,
"webPreferences": {
"nativeWindowOpen": true,
"sandbox": true,
"backgroundColor": "blue",
"contextIsolation": true,
"nativeWindowOpen": true,
"nodeIntegration": false,
"sandbox": true,
"webviewTag": false,
"nodeIntegrationInSubFrames": false,
"openerId": null
},
"show": false,
"height": 600,
"top": 1,
"left": 1,
"x": 1,
"y": 1,
"webContents": "[WebContents]"
},
[],
Expand Down