From 5c9b19b508045df6070f53422b154490be2796d7 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 16 Mar 2016 09:24:57 -0700 Subject: [PATCH 01/13] web-preferences -> webPreferences --- spec/static/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/static/main.js b/spec/static/main.js index 9a049e3f1078..77a3c3927228 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -82,8 +82,8 @@ app.on('ready', function() { show: false, width: 800, height: 600, - 'web-preferences': { - javascript: true // Test whether web-preferences crashes. + webPreferences: { + javascript: true // Test whether web preferences crashes. }, }); window.loadURL(url.format({ From 1e8e8f18b42f4a442e5932d20c2c8b3dd295e823 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 16 Mar 2016 09:31:32 -0700 Subject: [PATCH 02/13] Add failing specs for deprecated options usage --- spec/api-browser-window-spec.js | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index c6deb38684c4..e0b3c75437e2 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -759,4 +759,25 @@ describe('browser-window module', function() { }); }); }); + + describe('deprecated options', function() { + it('throws a deprecation error for option keys using hyphens instead of camel case', function() { + assert.throws(function () { + new BrowserWindow({'min-width': 500}) + }, 'min-width is deprecated. Use minWidth instead.'); + }); + + it('throws a deprecation error for webPreference keys using hyphens instead of camel case', function() { + assert.throws(function () { + new BrowserWindow({webPreferences: {'node-integration': false}}) + }, 'node-integration is deprecated. Use nodeIntegration instead.'); + }); + + it('throws a deprecation error for option keys that should be set on webPreferences', function() { + assert.throws(function () { + new BrowserWindow({zoomFactor: 1}) + }, 'Setting zoomFactor on options is deprecated. Set it on options.webPreferences instead.'); + }); + }) + }); From dfd13cf4ca797afc0027368640a2a65123df9e1d Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 16 Mar 2016 09:33:19 -0700 Subject: [PATCH 03/13] persistented -> persisted --- lib/browser/chrome-extension.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/browser/chrome-extension.js b/lib/browser/chrome-extension.js index fcbb8c7f15fd..054a0cd6c675 100644 --- a/lib/browser/chrome-extension.js +++ b/lib/browser/chrome-extension.js @@ -132,7 +132,7 @@ app.once('ready', function() { return delete extensionInfoMap[name]; }; - // Load persistented extensions when devtools is opened. + // Load persisted extensions when devtools is opened. init = BrowserWindow.prototype._init; return BrowserWindow.prototype._init = function() { init.call(this); From 1b6e01ce6de14f4df6e57d81ceb99b9714bd6159 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 16 Mar 2016 09:35:18 -0700 Subject: [PATCH 04/13] Add missing semicolons --- spec/api-browser-window-spec.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index e0b3c75437e2..00bc2fb9f18e 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -763,21 +763,20 @@ describe('browser-window module', function() { describe('deprecated options', function() { it('throws a deprecation error for option keys using hyphens instead of camel case', function() { assert.throws(function () { - new BrowserWindow({'min-width': 500}) + new BrowserWindow({'min-width': 500}); }, 'min-width is deprecated. Use minWidth instead.'); }); it('throws a deprecation error for webPreference keys using hyphens instead of camel case', function() { assert.throws(function () { - new BrowserWindow({webPreferences: {'node-integration': false}}) + new BrowserWindow({webPreferences: {'node-integration': false}}); }, 'node-integration is deprecated. Use nodeIntegration instead.'); }); it('throws a deprecation error for option keys that should be set on webPreferences', function() { assert.throws(function () { - new BrowserWindow({zoomFactor: 1}) + new BrowserWindow({zoomFactor: 1}); }, 'Setting zoomFactor on options is deprecated. Set it on options.webPreferences instead.'); }); - }) - + }); }); From 15397bf879b26e56df4d28453a30f04ea915b984 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 16 Mar 2016 09:38:03 -0700 Subject: [PATCH 05/13] Report deprecated BrowserWindow options --- atom/browser/api/atom_api_window.cc | 19 +++++++++++ lib/browser/api/browser-window.js | 53 ++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 6c0f0f6f762a..87f6411e6658 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -52,6 +52,11 @@ namespace api { namespace { +// The DeprecatedOptionsCheckCallback funtion which is implemented in JavaScript +using DeprecatedOptionsCheckCallback = + base::Callback)>; +DeprecatedOptionsCheckCallback g_deprecated_options_check; + void OnCapturePageDone( v8::Isolate* isolate, const base::Callback& callback, @@ -291,6 +296,13 @@ mate::Wrappable* Window::New(v8::Isolate* isolate, mate::Arguments* args) { options = mate::Dictionary::CreateEmpty(isolate); } + std::string deprecation_message = g_deprecated_options_check.Run( + options.GetHandle()); + if (deprecation_message.length() > 0) { + args->ThrowError(deprecation_message); + return nullptr; + } + return new Window(isolate, options); } @@ -797,6 +809,10 @@ v8::Local Window::From(v8::Isolate* isolate, return v8::Null(isolate); } +void SetDeprecatedOptionsCheck(const DeprecatedOptionsCheckCallback& callback) { + g_deprecated_options_check = callback; +} + } // namespace api } // namespace atom @@ -816,6 +832,9 @@ void Initialize(v8::Local exports, v8::Local unused, &mate::TrackableObject::FromWeakMapID); browser_window.SetMethod("getAllWindows", &mate::TrackableObject::GetAll); + browser_window.SetMethod("_setDeprecatedOptionsCheck", + &atom::api::SetDeprecatedOptionsCheck); + mate::Dictionary dict(isolate, exports); dict.Set("BrowserWindow", browser_window); diff --git a/lib/browser/api/browser-window.js b/lib/browser/api/browser-window.js index 7e2a8d38c31f..9645f855eed1 100644 --- a/lib/browser/api/browser-window.js +++ b/lib/browser/api/browser-window.js @@ -8,7 +8,6 @@ const BrowserWindow = process.atomBinding('window').BrowserWindow; BrowserWindow.prototype.__proto__ = EventEmitter.prototype; BrowserWindow.prototype._init = function() { - // avoid recursive require. var app, menu; app = require('electron').app; @@ -240,4 +239,56 @@ BrowserWindow.prototype.getPageTitle = deprecate('getPageTitle', 'webContents.ge return (ref1 = this.webContents) != null ? ref1.getTitle() : void 0; }); +const isDeprecatedKey = function(key) { + return key.indexOf('-') >= 0; +}; + +// Map deprecated key with hyphens to camel case key +const getNonDeprecatedKey = function(deprecatedKey) { + return deprecatedKey.replace(/-./g, function(match) { + return match[1].toUpperCase(); + }); +}; + +// TODO Remove for 1.0 +const checkForDeprecatedOptions = function(options) { + if (!options) return ''; + + let keysToCheck = Object.keys(options); + if (options.webPreferences) { + keysToCheck = keysToCheck.concat(Object.keys(options.webPreferences)); + } + + // Check options for keys with hypens in them + let deprecatedKey = keysToCheck.filter(isDeprecatedKey)[0]; + if (deprecatedKey) { + try { + deprecate.warn(deprecatedKey, getNonDeprecatedKey(deprecatedKey)); + } catch (error) { + // Return error message so it can be rethrown via C++ + return error.message; + } + } + + let webPreferenceOption; + if (options.hasOwnProperty('nodeIntegration')) { + webPreferenceOption = 'nodeIntegration'; + } else if (options.hasOwnProperty('preload')) { + webPreferenceOption = 'preload'; + } else if (options.hasOwnProperty('zoomFactor')) { + webPreferenceOption = 'zoomFactor'; + } + if (webPreferenceOption) { + try { + deprecate.log(`Setting ${webPreferenceOption} on options is deprecated. Set it on options.webPreferences instead.`); + } catch (error) { + // Return error message so it can be rethrown via C++ + return error.message; + } + } + + return ''; +}; +BrowserWindow._setDeprecatedOptionsCheck(checkForDeprecatedOptions); + module.exports = BrowserWindow; From 2acfb8ad82ee149bec56254c2f61f7fc4b69fcba Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 16 Mar 2016 10:47:55 -0700 Subject: [PATCH 06/13] node-integration -> nodeIntegration --- spec/chromium-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/chromium-spec.js b/spec/chromium-spec.js index 71cef1256187..ac9403a7ecd4 100644 --- a/spec/chromium-spec.js +++ b/spec/chromium-spec.js @@ -155,7 +155,7 @@ describe('chromium feature', function() { b.close(); }); - it('accepts "node-integration" as feature', function(done) { + it('accepts "nodeIntegration" as feature', function(done) { var b; listener = function(event) { assert.equal(event.data, 'undefined'); From a14380ed01c7bf5d8535bd9515a3964d2a71db63 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 16 Mar 2016 10:59:47 -0700 Subject: [PATCH 07/13] Set webPrereferences from features tring --- lib/renderer/override.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/renderer/override.js b/lib/renderer/override.js index 2aceec5353d3..d9e00122870d 100644 --- a/lib/renderer/override.js +++ b/lib/renderer/override.js @@ -101,6 +101,7 @@ window.open = function(url, frameName, features) { } options = {}; ints = ['x', 'y', 'width', 'height', 'min-width', 'max-width', 'min-height', 'max-height', 'zoom-factor']; + const webPreferences = ['zoom-factor', 'zoomFactor', 'node-integration', 'nodeIntegration', 'preload']; // Make sure to get rid of excessive whitespace in the property name ref1 = features.split(/,\s*/); @@ -109,7 +110,15 @@ window.open = function(url, frameName, features) { ref2 = feature.split(/\s*=/); name = ref2[0]; value = ref2[1]; - options[name] = value === 'yes' || value === '1' ? true : value === 'no' || value === '0' ? false : value; + value = value === 'yes' || value === '1' ? true : value === 'no' || value === '0' ? false : value; + if (webPreferences.includes(name)) { + if (options.webPreferences == null) { + options.webPreferences = {}; + } + options.webPreferences[name] = value; + } else { + options[name] = value; + } } if (options.left) { if (options.x == null) { From 3e7501579f7644e3e914bacb8ddff4b5291eb302 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 16 Mar 2016 11:00:15 -0700 Subject: [PATCH 08/13] Add camel case versions to ints array --- lib/renderer/override.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/renderer/override.js b/lib/renderer/override.js index d9e00122870d..5e9d42adf61d 100644 --- a/lib/renderer/override.js +++ b/lib/renderer/override.js @@ -100,7 +100,7 @@ window.open = function(url, frameName, features) { features = ''; } options = {}; - ints = ['x', 'y', 'width', 'height', 'min-width', 'max-width', 'min-height', 'max-height', 'zoom-factor']; + ints = ['x', 'y', 'width', 'height', 'min-width', 'max-width', 'min-height', 'minHeight', 'max-height', 'maxHeight', 'zoom-factor', 'zoomFactor']; const webPreferences = ['zoom-factor', 'zoomFactor', 'node-integration', 'nodeIntegration', 'preload']; // Make sure to get rid of excessive whitespace in the property name From 90d815ce6ca2d0e28bc29477e5df9ed6798f2b06 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 16 Mar 2016 11:03:50 -0700 Subject: [PATCH 09/13] Add todo about removing hyphenated options --- lib/renderer/override.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/renderer/override.js b/lib/renderer/override.js index 5e9d42adf61d..69295d1632be 100644 --- a/lib/renderer/override.js +++ b/lib/renderer/override.js @@ -100,6 +100,8 @@ window.open = function(url, frameName, features) { features = ''; } options = {}; + + // TODO remove hyphenated options in both of the following arrays for 1.0 ints = ['x', 'y', 'width', 'height', 'min-width', 'max-width', 'min-height', 'minHeight', 'max-height', 'maxHeight', 'zoom-factor', 'zoomFactor']; const webPreferences = ['zoom-factor', 'zoomFactor', 'node-integration', 'nodeIntegration', 'preload']; From c31882749d303ef56deadeb8d061641ea1f3f0ee Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Wed, 16 Mar 2016 15:03:53 -0700 Subject: [PATCH 10/13] Correct typos in comments --- atom/browser/api/atom_api_window.cc | 2 +- lib/browser/api/browser-window.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 87f6411e6658..a41b8fae9f22 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -52,7 +52,7 @@ namespace api { namespace { -// The DeprecatedOptionsCheckCallback funtion which is implemented in JavaScript +// This function is implemented in JavaScript using DeprecatedOptionsCheckCallback = base::Callback)>; DeprecatedOptionsCheckCallback g_deprecated_options_check; diff --git a/lib/browser/api/browser-window.js b/lib/browser/api/browser-window.js index 9645f855eed1..fe9d4415d534 100644 --- a/lib/browser/api/browser-window.js +++ b/lib/browser/api/browser-window.js @@ -259,7 +259,7 @@ const checkForDeprecatedOptions = function(options) { keysToCheck = keysToCheck.concat(Object.keys(options.webPreferences)); } - // Check options for keys with hypens in them + // Check options for keys with hyphens in them let deprecatedKey = keysToCheck.filter(isDeprecatedKey)[0]; if (deprecatedKey) { try { From 737ffd8d7c1e7a3a7d67a237b7bde566e071c2f2 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Thu, 17 Mar 2016 13:34:30 -0700 Subject: [PATCH 11/13] Improve deprecated message on webPreferences options --- lib/browser/api/browser-window.js | 2 +- spec/api-browser-window-spec.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/browser/api/browser-window.js b/lib/browser/api/browser-window.js index fe9d4415d534..87077ffd2d94 100644 --- a/lib/browser/api/browser-window.js +++ b/lib/browser/api/browser-window.js @@ -280,7 +280,7 @@ const checkForDeprecatedOptions = function(options) { } if (webPreferenceOption) { try { - deprecate.log(`Setting ${webPreferenceOption} on options is deprecated. Set it on options.webPreferences instead.`); + deprecate.log(`options.${webPreferenceOption} is deprecated. Use options.webPreferences.${webPreferenceOption} instead.`); } catch (error) { // Return error message so it can be rethrown via C++ return error.message; diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 00bc2fb9f18e..beb84a8d7486 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -776,7 +776,7 @@ describe('browser-window module', function() { it('throws a deprecation error for option keys that should be set on webPreferences', function() { assert.throws(function () { new BrowserWindow({zoomFactor: 1}); - }, 'Setting zoomFactor on options is deprecated. Set it on options.webPreferences instead.'); + }, 'options.zoomFactor is deprecated. Use options.webPreferences.zoomFactor instead.'); }); }); }); From 7668c1ea0b012e4712aa16369ea723a093d7f1d1 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 18 Mar 2016 09:02:58 -0700 Subject: [PATCH 12/13] Use deprecate.warn instead of deprecate.log --- lib/browser/api/browser-window.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/browser/api/browser-window.js b/lib/browser/api/browser-window.js index 87077ffd2d94..ed6cbc4b80a7 100644 --- a/lib/browser/api/browser-window.js +++ b/lib/browser/api/browser-window.js @@ -280,7 +280,7 @@ const checkForDeprecatedOptions = function(options) { } if (webPreferenceOption) { try { - deprecate.log(`options.${webPreferenceOption} is deprecated. Use options.webPreferences.${webPreferenceOption} instead.`); + deprecate.warn(`options.${webPreferenceOption}`, `options.webPreferences.${webPreferenceOption}`); } catch (error) { // Return error message so it can be rethrown via C++ return error.message; From 6aa452cda4e3ec6b975bdc9fb3ad043de5bee167 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 18 Mar 2016 09:06:03 -0700 Subject: [PATCH 13/13] Set _setDeprecatedOptionsCheck on exports --- atom/browser/api/atom_api_window.cc | 5 ++--- lib/browser/api/browser-window.js | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index a41b8fae9f22..ccc3c8695d51 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -832,12 +832,11 @@ void Initialize(v8::Local exports, v8::Local unused, &mate::TrackableObject::FromWeakMapID); browser_window.SetMethod("getAllWindows", &mate::TrackableObject::GetAll); - browser_window.SetMethod("_setDeprecatedOptionsCheck", - &atom::api::SetDeprecatedOptionsCheck); - mate::Dictionary dict(isolate, exports); dict.Set("BrowserWindow", browser_window); + dict.SetMethod("_setDeprecatedOptionsCheck", + &atom::api::SetDeprecatedOptionsCheck); } } // namespace diff --git a/lib/browser/api/browser-window.js b/lib/browser/api/browser-window.js index ed6cbc4b80a7..244e87797a5b 100644 --- a/lib/browser/api/browser-window.js +++ b/lib/browser/api/browser-window.js @@ -3,7 +3,7 @@ const ipcMain = require('electron').ipcMain; const deprecate = require('electron').deprecate; const EventEmitter = require('events').EventEmitter; -const BrowserWindow = process.atomBinding('window').BrowserWindow; +const {BrowserWindow, _setDeprecatedOptionsCheck} = process.atomBinding('window'); BrowserWindow.prototype.__proto__ = EventEmitter.prototype; @@ -289,6 +289,6 @@ const checkForDeprecatedOptions = function(options) { return ''; }; -BrowserWindow._setDeprecatedOptionsCheck(checkForDeprecatedOptions); +_setDeprecatedOptionsCheck(checkForDeprecatedOptions); module.exports = BrowserWindow;