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 deprecated additionalFeatures #28548

Merged
merged 4 commits into from Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions docs/api/web-contents.md
Expand Up @@ -147,7 +147,8 @@ Returns:
* `options` BrowserWindowConstructorOptions - The options which will be used for creating the new
[`BrowserWindow`](browser-window.md).
* `additionalFeatures` String[] - The non-standard features (features not handled
by Chromium or Electron) given to `window.open()`.
by Chromium or Electron) given to `window.open()`. Deprecated, and will now
always be the empty array `[]`.
* `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.
Expand Down Expand Up @@ -207,8 +208,6 @@ Returns:
`window.open()`, and options given by
[`webContents.setWindowOpenHandler`](web-contents.md#contentssetwindowopenhandlerhandler).
Unrecognized options are not filtered out.
* `additionalFeatures` String[] - The non-standard features (features not
handled Chromium or Electron) _Deprecated_
* `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.
Expand Down
2 changes: 1 addition & 1 deletion docs/api/window-open.md
Expand Up @@ -64,7 +64,7 @@ window.open('https://github.com', '_blank', 'top=500,left=200,frame=false,nodeIn
the parent window.
* Non-standard features (that are not handled by Chromium or Electron) given in
`features` will be passed to any registered `webContents`'s
`did-create-window` event handler in the `additionalFeatures` argument.
`did-create-window` event handler in the `options` argument.
* `frameName` follows the specification of `windowName` located in the [native documentation](https://developer.mozilla.org/en-US/docs/Web/API/Window/open#parameters).

To customize or cancel the creation of the window, you can optionally set an
Expand Down
26 changes: 26 additions & 0 deletions docs/breaking-changes.md
Expand Up @@ -28,6 +28,32 @@ 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: `additionalFeatures`

The deprecated `additionalFeatures` property in the `new-window` and
`did-create-window` events of WebContents has been removed. Since `new-window`
uses positional arguments, the argument is still present, but will always be
the empty array `[]`. (Though note, the `new-window` event itself is
deprecated, and is replaced by `setWindowOpenHandler`.) Bare keys in window
features will now present as keys with the value `true` in the options object.

```js
// Removed in Electron 14
// Triggered by window.open('...', '', 'my-key')
webContents.on('did-create-window', (window, details) => {
if (details.additionalFeatures.includes('my-key')) {
// ...
}
})

// Replace with
webContents.on('did-create-window', (window, details) => {
if (details.options['my-key']) {
// ...
}
})
```

## Planned Breaking API Changes (13.0)

### API Changed: `session.setPermissionCheckHandler(handler)`
Expand Down
17 changes: 6 additions & 11 deletions lib/browser/guest-window-manager.ts
Expand Up @@ -42,7 +42,7 @@ export function openGuestWindow ({ event, embedder, guest, referrer, disposition
windowOpenArgs: WindowOpenArgs,
}): BrowserWindow | undefined {
const { url, frameName, features } = windowOpenArgs;
const { options: browserWindowOptions, additionalFeatures } = makeBrowserWindowOptions({
const { options: browserWindowOptions } = makeBrowserWindowOptions({
embedder,
features,
overrideOptions: overrideBrowserWindowOptions
Expand All @@ -54,7 +54,6 @@ export function openGuestWindow ({ event, embedder, guest, referrer, disposition
guest,
browserWindowOptions,
windowOpenArgs,
additionalFeatures,
disposition,
postData,
referrer
Expand Down Expand Up @@ -93,7 +92,7 @@ export function openGuestWindow ({ event, embedder, guest, referrer, disposition

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

embedder.emit('did-create-window', window, { url, frameName, options: browserWindowOptions, disposition, additionalFeatures, referrer, postData });
embedder.emit('did-create-window', window, { url, frameName, options: browserWindowOptions, disposition, referrer, postData });

return window;
}
Expand Down Expand Up @@ -134,13 +133,12 @@ const handleWindowLifecycleEvents = function ({ embedder, guest, frameName }: {
* Deprecated in favor of `webContents.setWindowOpenHandler` and
* `did-create-window` in 11.0.0. Will be removed in 12.0.0.
*/
function emitDeprecatedNewWindowEvent ({ event, embedder, guest, windowOpenArgs, browserWindowOptions, additionalFeatures, disposition, referrer, postData }: {
function emitDeprecatedNewWindowEvent ({ event, embedder, guest, windowOpenArgs, browserWindowOptions, disposition, referrer, postData }: {
event: { sender: WebContents, defaultPrevented: boolean, newGuest?: BrowserWindow },
embedder: WebContents,
guest?: WebContents,
windowOpenArgs: WindowOpenArgs,
browserWindowOptions: BrowserWindowConstructorOptions,
additionalFeatures: string[]
disposition: string,
referrer: Referrer,
postData?: PostData,
Expand All @@ -162,7 +160,7 @@ function emitDeprecatedNewWindowEvent ({ event, embedder, guest, windowOpenArgs,
...browserWindowOptions,
webContents: guest
},
additionalFeatures,
[], // additionalFeatures
referrer,
postBody
);
Expand Down Expand Up @@ -200,19 +198,17 @@ const securityWebPreferences: { [key: string]: boolean } = {
enableWebSQL: false
};

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

const deprecatedInheritedOptions = getDeprecatedInheritedOptions(embedder);

return {
additionalFeatures,
options: {
...(useDeprecatedBehaviorForOptionInheritance && deprecatedInheritedOptions),
show: true,
Expand All @@ -232,7 +228,6 @@ export function makeWebPreferences ({ embedder, secureOverrideWebPreferences = {
// 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);
Expand Down
28 changes: 7 additions & 21 deletions lib/common/parse-features-string.ts
Expand Up @@ -56,41 +56,28 @@ function coerce (key: string, value: string): CoercedValue {
}
}

export function parseCommaSeparatedKeyValue (source: string, useSoonToBeDeprecatedBehaviorForBareKeys: boolean) {
const bareKeys = [] as string[];
export function parseCommaSeparatedKeyValue (source: string) {
const parsed = {} as { [key: string]: any };
for (const keyValuePair of source.split(',')) {
const [key, value] = keyValuePair.split('=').map(str => str.trim());
if (useSoonToBeDeprecatedBehaviorForBareKeys && value === undefined) {
if (key) { bareKeys.push(key); }
continue;
}
parsed[key] = coerce(key, value);
if (key) { parsed[key] = coerce(key, value); }
}

return { parsed, bareKeys };
return parsed;
}

export function parseWebViewWebPreferences (preferences: string) {
return parseCommaSeparatedKeyValue(preferences, false).parsed;
return parseCommaSeparatedKeyValue(preferences);
}

const allowedWebPreferences = ['zoomFactor', 'nodeIntegration', 'javascript', 'contextIsolation', 'webviewTag'] as const;
type AllowedWebPreference = (typeof allowedWebPreferences)[number];

/**
* Parses a feature string that has the format used in window.open().
*
* `useSoonToBeDeprecatedBehaviorForBareKeys` - In the html spec, windowFeatures keys
* without values are interpreted as `true`. Previous versions of Electron did
* not respect this. In order to not break any applications, this will be
* flipped in the next major version.
*/
export function parseFeatures (
features: string,
useSoonToBeDeprecatedBehaviorForBareKeys: boolean = true
) {
const { parsed, bareKeys } = parseCommaSeparatedKeyValue(features, useSoonToBeDeprecatedBehaviorForBareKeys);
export function parseFeatures (features: string) {
const parsed = parseCommaSeparatedKeyValue(features);

const webPreferences: { [K in AllowedWebPreference]?: any } = {};
allowedWebPreferences.forEach((key) => {
Expand All @@ -104,7 +91,6 @@ export function parseFeatures (

return {
options: parsed as Omit<BrowserWindowConstructorOptions, 'webPreferences'>,
webPreferences,
additionalFeatures: bareKeys
webPreferences
};
}
8 changes: 4 additions & 4 deletions spec-main/api-browser-window-spec.ts
Expand Up @@ -3003,12 +3003,12 @@ describe('BrowserWindow module', () => {

it('emits when window.open is called', (done) => {
const w = new BrowserWindow({ show: false, webPreferences: { nodeIntegration: true } });
w.webContents.once('new-window', (e, url, frameName, disposition, options, additionalFeatures) => {
w.webContents.once('new-window', (e, url, frameName, disposition, options) => {
e.preventDefault();
try {
expect(url).to.equal('http://host/');
expect(frameName).to.equal('host');
expect(additionalFeatures[0]).to.equal('this-is-not-a-standard-feature');
expect((options as any)['this-is-not-a-standard-feature']).to.equal(true);
done();
} catch (e) {
done(e);
Expand All @@ -3019,12 +3019,12 @@ describe('BrowserWindow module', () => {

it('emits when window.open is called with no webPreferences', (done) => {
const w = new BrowserWindow({ show: false });
w.webContents.once('new-window', function (e, url, frameName, disposition, options, additionalFeatures) {
w.webContents.once('new-window', function (e, url, frameName, disposition, options) {
e.preventDefault();
try {
expect(url).to.equal('http://host/');
expect(frameName).to.equal('host');
expect(additionalFeatures[0]).to.equal('this-is-not-a-standard-feature');
expect((options as any)['this-is-not-a-standard-feature']).to.equal(true);
done();
} catch (e) {
done(e);
Expand Down
2 changes: 1 addition & 1 deletion spec-main/internal-spec.ts
Expand Up @@ -4,7 +4,7 @@ describe('feature-string parsing', () => {
it('is indifferent to whitespace around keys and values', () => {
const { parseCommaSeparatedKeyValue } = require('../lib/common/parse-features-string');
const checkParse = (string: string, parsed: Record<string, string | boolean>) => {
const features = parseCommaSeparatedKeyValue(string, true).parsed;
const features = parseCommaSeparatedKeyValue(string);
expect(features).to.deep.equal(parsed);
};
checkParse('a=yes,c=d', { a: true, c: 'd' });
Expand Down