diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index 19d5167b14dd..2291b8ffa80a 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -203,9 +203,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. * `referrer` [Referrer](structures/referrer.md) - The referrer that will be @@ -1148,8 +1148,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 `
`. See +[`window.open()`](window-open.md) for more details and how to use this in +conjunction with `did-create-window`. #### `contents.setAudioMuted(muted)` diff --git a/docs/api/window-open.md b/docs/api/window-open.md index b28ec6543a70..7d8bfa262dbf 100644 --- a/docs/api/window-open.md +++ b/docs/api/window-open.md @@ -22,9 +22,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. diff --git a/docs/breaking-changes.md b/docs/breaking-changes.md index f7fc6af2673d..c2d7d7edeca4 100644 --- a/docs/breaking-changes.md +++ b/docs/breaking-changes.md @@ -38,6 +38,27 @@ to open synchronously scriptable child windows, among other incompatibilities. See the documentation for [window.open in Electron](api/window-open.md) for more details. +### 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: { + // ... + } + } +}) +``` + ### Removed: `additionalFeatures` The deprecated `additionalFeatures` property in the `new-window` and diff --git a/lib/browser/guest-window-manager.ts b/lib/browser/guest-window-manager.ts index 5de6edbf61f4..03fd27619925 100644 --- a/lib/browser/guest-window-manager.ts +++ b/lib/browser/guest-window-manager.ts @@ -198,39 +198,37 @@ const securityWebPreferences: { [key: string]: boolean } = { enableWebSQL: false }; -function makeBrowserWindowOptions ({ embedder, features, overrideOptions, useDeprecatedBehaviorForOptionInheritance = true }: { +function makeBrowserWindowOptions ({ embedder, features, overrideOptions }: { embedder: WebContents, features: string, overrideOptions?: BrowserWindowConstructorOptions, - useDeprecatedBehaviorForOptionInheritance?: boolean }) { const { options: parsedOptions, webPreferences: parsedWebPreferences } = parseFeatures(features); - const deprecatedInheritedOptions = getDeprecatedInheritedOptions(embedder); - return { 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['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'], - 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]) { @@ -241,7 +239,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 @@ -254,25 +251,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; diff --git a/lib/browser/guest-window-proxy.ts b/lib/browser/guest-window-proxy.ts index d16d7ce63518..ed9e7e780c4c 100644 --- a/lib/browser/guest-window-proxy.ts +++ b/lib/browser/guest-window-proxy.ts @@ -135,7 +135,7 @@ handleMessage( if (!windowMethods.has(method)) { console.error( - `Blocked ${event.sender.getURL()} from calling method: ${method}` + `Blocked ${event.senderFrame.url} from calling method: ${method}` ); throw new Error(`Invalid method: ${method}`); } diff --git a/lib/renderer/window-setup.ts b/lib/renderer/window-setup.ts index 6b33899991db..91cee9a840f0 100644 --- a/lib/renderer/window-setup.ts +++ b/lib/renderer/window-setup.ts @@ -270,7 +270,24 @@ export const windowSetup = ( if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueWithDynamicPropsFromIsolatedWorld(['open'], window.open); } - if (openerId != null) { + // If this window uses nativeWindowOpen, but its opener window does not, we + // need to proxy window.opener in order to let the page communicate with its + // opener. + // Additionally, windows opened from a nativeWindowOpen child of a + // non-nativeWindowOpen parent will initially have their WebPreferences + // copied from their opener before having them updated, meaning openerId is + // initially incorrect. We detect this situation by checking for + // window.opener, which will be non-null for a natively-opened child, so we + // can ignore the openerId in that case, since it's incorrectly copied from + // the parent. This is, uh, confusing, so here's a diagram that will maybe + // help? + // + // [ grandparent window ] --> [ parent window ] --> [ child window ] + // n.W.O = false n.W.O = true n.W.O = true + // id = 1 id = 2 id = 3 + // openerId = null openerId = 1 openerId = 1 <- !!wrong!! + // opener = null opener = null opener = [parent window] + if (openerId != null && !window.opener) { window.opener = getOrCreateProxy(openerId); if (contextIsolationEnabled) internalContextBridge.overrideGlobalValueWithDynamicPropsFromIsolatedWorld(['opener'], window.opener); } diff --git a/shell/browser/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc index 814b1a313333..11438528160f 100644 --- a/shell/browser/api/electron_api_browser_window.cc +++ b/shell/browser/api/electron_api_browser_window.cc @@ -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()) - .Set("browserWindowOptions", options); - // Associate with BrowserWindow. web_contents->SetOwnerWindow(window()); diff --git a/spec-main/api-browser-window-spec.ts b/spec-main/api-browser-window-spec.ts index d7e49604e0a2..26bb1b8b0dcf 100644 --- a/spec-main/api-browser-window-spec.ts +++ b/spec-main/api-browser-window-spec.ts @@ -2383,7 +2383,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; @@ -2598,6 +2598,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; }); @@ -2676,7 +2680,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 } } })); diff --git a/spec-main/chromium-spec.ts b/spec-main/chromium-spec.ts index 25eed41be532..3de4b6efe894 100644 --- a/spec-main/chromium-spec.ts +++ b/spec-main/chromium-spec.ts @@ -1042,7 +1042,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) }) diff --git a/spec-main/fixtures/snapshots/native-window-open.snapshot.txt b/spec-main/fixtures/snapshots/native-window-open.snapshot.txt index bd31b8d80619..47b14578b84f 100644 --- a/spec-main/fixtures/snapshots/native-window-open.snapshot.txt +++ b/spec-main/fixtures/snapshots/native-window-open.snapshot.txt @@ -8,27 +8,23 @@ "frame-name", "new-window", { - "width": 800, - "title": "cool", - "backgroundColor": "blue", - "focusable": false, - "webPreferences": { - "nativeWindowOpen": true, - "sandbox": true, - "backgroundColor": "blue", - "contextIsolation": true, - "nodeIntegration": false, - "webviewTag": false, - "nodeIntegrationInSubFrames": false, - "openerId": null - }, "show": true, + "width": 800, "height": 600, "top": 5, "left": 10, "resizable": false, "x": 10, "y": 5, + "webPreferences": { + "contextIsolation": true, + "nativeWindowOpen": true, + "nodeIntegration": false, + "sandbox": true, + "webviewTag": false, + "nodeIntegrationInSubFrames": false, + "openerId": null + }, "webContents": "[WebContents]" }, [], @@ -47,26 +43,22 @@ "frame-name", "new-window", { - "width": 800, - "title": "cool", - "backgroundColor": "blue", - "focusable": false, - "webPreferences": { - "nativeWindowOpen": true, - "sandbox": true, - "backgroundColor": "blue", - "zoomFactor": "2", - "contextIsolation": true, - "nodeIntegration": false, - "webviewTag": false, - "nodeIntegrationInSubFrames": false, - "openerId": null - }, "show": true, + "width": 800, "height": 600, "resizable": false, "x": 0, "y": 10, + "webPreferences": { + "zoomFactor": "2", + "contextIsolation": true, + "nativeWindowOpen": true, + "nodeIntegration": false, + "sandbox": true, + "webviewTag": false, + "nodeIntegrationInSubFrames": false, + "openerId": null + }, "webContents": "[WebContents]" }, [], @@ -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]" @@ -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]" }, [], @@ -157,26 +144,22 @@ "frame-name", "new-window", { - "width": 800, - "title": "cool", - "backgroundColor": "blue", - "focusable": false, - "webPreferences": { - "nativeWindowOpen": true, - "sandbox": true, - "backgroundColor": "blue", - "contextIsolation": true, - "nodeIntegration": false, - "webviewTag": false, - "nodeIntegrationInSubFrames": false, - "openerId": null - }, "show": false, + "width": 800, "height": 600, "top": 1, "left": 1, "x": 1, "y": 1, + "webPreferences": { + "contextIsolation": true, + "nativeWindowOpen": true, + "nodeIntegration": false, + "sandbox": true, + "webviewTag": false, + "nodeIntegrationInSubFrames": false, + "openerId": null + }, "webContents": "[WebContents]" }, [], diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 1f5647ab03a2..26e43b12ae74 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -91,7 +91,7 @@ describe('chromium feature', () => { slashes: true }); const message = waitForEvent(window, 'message'); - const b = window.open(windowUrl, '', 'nodeIntegration=no,show=no'); + const b = window.open(windowUrl, '', 'nodeIntegration=no,contextIsolation=no,show=no'); const event = await message; b.close(); expect(event.data.isProcessGlobalUndefined).to.be.true(); @@ -107,7 +107,7 @@ describe('chromium feature', () => { slashes: true }); const message = waitForEvent(window, 'message'); - const b = window.open(windowUrl, '', 'webviewTag=no,nodeIntegration=yes,show=no'); + const b = window.open(windowUrl, '', 'webviewTag=no,contextIsolation=no,nodeIntegration=yes,show=no'); const event = await message; b.close(); expect(event.data.isWebViewGlobalUndefined).to.be.true(); @@ -218,7 +218,7 @@ describe('chromium feature', () => { it('delivers messages that match the origin', async () => { const message = waitForEvent(window, 'message'); - const b = window.open(serverURL, '', 'show=no'); + const b = window.open(serverURL, '', 'show=no,contextIsolation=no,nodeIntegration=yes'); const event = await message; b.close(); expect(event.data).to.equal('deliver'); diff --git a/spec/fixtures/pages/window-open-postMessage-driver.html b/spec/fixtures/pages/window-open-postMessage-driver.html index 89408b458458..65f40417c74d 100644 --- a/spec/fixtures/pages/window-open-postMessage-driver.html +++ b/spec/fixtures/pages/window-open-postMessage-driver.html @@ -1,5 +1,5 @@