From 5454cd01125b9abcf89a6ec1f8622bbe2107ea81 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Fri, 18 Jan 2019 08:41:52 -0800 Subject: [PATCH 1/2] feat: make zoomLevel/zoomFactor sync --- atom/browser/api/atom_api_web_contents.cc | 4 +- docs/api/promisification.md | 4 - docs/api/web-contents.md | 16 +-- docs/api/webview-tag.md | 16 +-- lib/browser/api/menu-item-roles.js | 10 +- lib/browser/api/web-contents.js | 45 +++++---- lib/browser/guest-view-manager.js | 2 +- lib/common/web-view-methods.js | 4 +- spec/api-web-contents-spec.js | 98 ++++++++++--------- spec/fixtures/api/execute-javascript.html | 0 .../pages/webview-custom-zoom-level.html | 20 ++-- .../pages/webview-in-page-navigate.html | 10 +- .../pages/webview-origin-zoom-level.html | 5 +- 13 files changed, 111 insertions(+), 123 deletions(-) create mode 100644 spec/fixtures/api/execute-javascript.html diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index 8416b9ad37e03..93b2ccb361b37 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -2159,9 +2159,9 @@ void WebContents::BuildPrototype(v8::Isolate* isolate, #endif .SetMethod("invalidate", &WebContents::Invalidate) .SetMethod("setZoomLevel", &WebContents::SetZoomLevel) - .SetMethod("_getZoomLevel", &WebContents::GetZoomLevel) + .SetMethod("getZoomLevel", &WebContents::GetZoomLevel) .SetMethod("setZoomFactor", &WebContents::SetZoomFactor) - .SetMethod("_getZoomFactor", &WebContents::GetZoomFactor) + .SetMethod("getZoomFactor", &WebContents::GetZoomFactor) .SetMethod("getType", &WebContents::GetType) .SetMethod("_getPreloadPath", &WebContents::GetPreloadPath) .SetMethod("getWebPreferences", &WebContents::GetWebPreferences) diff --git a/docs/api/promisification.md b/docs/api/promisification.md index 55717d55a655a..4363afb6c7cb3 100644 --- a/docs/api/promisification.md +++ b/docs/api/promisification.md @@ -41,8 +41,6 @@ When a majority of affected functions are migrated, this flag will be enabled by - [ses.getBlobData(identifier, callback)](https://github.com/electron/electron/blob/master/docs/api/session.md#getBlobData) - [ses.clearAuthCache(options[, callback])](https://github.com/electron/electron/blob/master/docs/api/session.md#clearAuthCache) - [contents.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#executeJavaScript) -- [contents.getZoomFactor(callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#getZoomFactor) -- [contents.getZoomLevel(callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#getZoomLevel) - [contents.hasServiceWorker(callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#hasServiceWorker) - [contents.unregisterServiceWorker(callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#unregisterServiceWorker) - [contents.print([options], [callback])](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#print) @@ -52,8 +50,6 @@ When a majority of affected functions are migrated, this flag will be enabled by - [webFrame.executeJavaScriptInIsolatedWorld(worldId, scripts[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/web-frame.md#executeJavaScriptInIsolatedWorld) - [webviewTag.executeJavaScript(code[, userGesture, callback])](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#executeJavaScript) - [webviewTag.printToPDF(options, callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#printToPDF) -- [webviewTag.getZoomFactor(callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#getZoomFactor) -- [webviewTag.getZoomLevel(callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#getZoomLevel) ### Converted Functions diff --git a/docs/api/web-contents.md b/docs/api/web-contents.md index 846cc39ff27f5..73d4875b99a87 100644 --- a/docs/api/web-contents.md +++ b/docs/api/web-contents.md @@ -977,13 +977,9 @@ Returns `Boolean` - Whether audio is currently playing. Changes the zoom factor to the specified factor. Zoom factor is zoom percent divided by 100, so 300% = 3.0. -#### `contents.getZoomFactor(callback)` +#### `contents.getZoomFactor()` -* `callback` Function - * `zoomFactor` Number - -Sends a request to get current zoom factor, the `callback` will be called with -`callback(zoomFactor)`. +Returns `Number` - the current zoom factor. #### `contents.setZoomLevel(level)` @@ -994,13 +990,9 @@ increment above or below represents zooming 20% larger or smaller to default limits of 300% and 50% of original size, respectively. The formula for this is `scale := 1.2 ^ level`. -#### `contents.getZoomLevel(callback)` - -* `callback` Function - * `zoomLevel` Number +#### `contents.getZoomLevel()` -Sends a request to get current zoom level, the `callback` will be called with -`callback(zoomLevel)`. +Returns `Number` - the current zoom level. #### `contents.setVisualZoomLevelLimits(minimumLevel, maximumLevel)` diff --git a/docs/api/webview-tag.md b/docs/api/webview-tag.md index bab79f4086b41..406613f876784 100644 --- a/docs/api/webview-tag.md +++ b/docs/api/webview-tag.md @@ -595,21 +595,13 @@ increment above or below represents zooming 20% larger or smaller to default limits of 300% and 50% of original size, respectively. The formula for this is `scale := 1.2 ^ level`. -### `.getZoomFactor(callback)` +### `.getZoomFactor()` -* `callback` Function - * `zoomFactor` Number - -Sends a request to get current zoom factor, the `callback` will be called with -`callback(zoomFactor)`. +Returns `Number` - the current zoom factor. -### `.getZoomLevel(callback)` - -* `callback` Function - * `zoomLevel` Number +### `.getZoomLevel()` -Sends a request to get current zoom level, the `callback` will be called with -`callback(zoomLevel)`. +Returns `Number` - the current zoom level. ### `.setVisualZoomLevelLimits(minimumLevel, maximumLevel)` diff --git a/lib/browser/api/menu-item-roles.js b/lib/browser/api/menu-item-roles.js index e5d47d665cafd..22949b324cdb6 100644 --- a/lib/browser/api/menu-item-roles.js +++ b/lib/browser/api/menu-item-roles.js @@ -156,9 +156,8 @@ const roles = { accelerator: 'CommandOrControl+Plus', nonNativeMacOSRole: true, webContentsMethod: (webContents) => { - webContents.getZoomLevel((zoomLevel) => { - webContents.setZoomLevel(zoomLevel + 0.5) - }) + const zoomLevel = webContents.getZoomLevel() + webContents.setZoomLevel(zoomLevel + 0.5) } }, zoomout: { @@ -166,9 +165,8 @@ const roles = { accelerator: 'CommandOrControl+-', nonNativeMacOSRole: true, webContentsMethod: (webContents) => { - webContents.getZoomLevel((zoomLevel) => { - webContents.setZoomLevel(zoomLevel - 0.5) - }) + const zoomLevel = webContents.getZoomLevel() + webContents.setZoomLevel(zoomLevel - 0.5) } }, // App submenu should be used for Mac only diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index 26c4d49577d2f..10a506fac0e45 100644 --- a/lib/browser/api/web-contents.js +++ b/lib/browser/api/web-contents.js @@ -111,6 +111,7 @@ WebContents.prototype.send = function (channel, ...args) { return this._send(internal, sendToAll, channel, args) } + WebContents.prototype.sendToAll = function (channel, ...args) { if (typeof channel !== 'string') { throw new Error('Missing required channel argument') @@ -209,6 +210,30 @@ WebContents.prototype.executeJavaScript = function (code, hasUserGesture, callba } } +// TODO(codebytere): remove when promisifications is complete +const nativeZoomLevel = WebContents.prototype.getZoomLevel +WebContents.prototype.getZoomLevel = function (callback) { + if (callback == null) { + return nativeZoomLevel.call(this) + } else { + process.nextTick(() => { + callback(nativeZoomLevel.call(this)) + }) + } +} + +// TODO(codebytere): remove when promisifications is complete +const nativeZoomFactor = WebContents.prototype.getZoomFactor +WebContents.prototype.getZoomFactor = function (callback) { + if (callback == null) { + return nativeZoomFactor.call(this) + } else { + process.nextTick(() => { + callback(nativeZoomFactor.call(this)) + }) + } +} + WebContents.prototype.takeHeapSnapshot = function (filePath) { return new Promise((resolve, reject) => { const channel = `ELECTRON_TAKE_HEAP_SNAPSHOT_RESULT_${getNextId()}` @@ -289,16 +314,6 @@ WebContents.prototype.getPrinters = function () { } } -WebContents.prototype.getZoomLevel = function (callback) { - if (typeof callback !== 'function') { - throw new Error('Must pass function as an argument') - } - process.nextTick(() => { - const zoomLevel = this._getZoomLevel() - callback(zoomLevel) - }) -} - WebContents.prototype.loadFile = function (filePath, options = {}) { if (typeof filePath !== 'string') { throw new Error('Must pass filePath as a string') @@ -315,16 +330,6 @@ WebContents.prototype.loadFile = function (filePath, options = {}) { })) } -WebContents.prototype.getZoomFactor = function (callback) { - if (typeof callback !== 'function') { - throw new Error('Must pass function as an argument') - } - process.nextTick(() => { - const zoomFactor = this._getZoomFactor() - callback(zoomFactor) - }) -} - // Add JavaScript wrappers for WebContents class. WebContents.prototype._init = function () { // The navigation controller. diff --git a/lib/browser/guest-view-manager.js b/lib/browser/guest-view-manager.js index bc23335c63044..378f07500ddee 100644 --- a/lib/browser/guest-view-manager.js +++ b/lib/browser/guest-view-manager.js @@ -212,7 +212,7 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn nodeIntegration: params.nodeintegration != null ? params.nodeintegration : false, enableRemoteModule: params.enableremotemodule, plugins: params.plugins, - zoomFactor: embedder._getZoomFactor(), + zoomFactor: embedder.getZoomFactor(), webSecurity: !params.disablewebsecurity, enableBlinkFeatures: params.blinkfeatures, disableBlinkFeatures: params.disableblinkfeatures diff --git a/lib/common/web-view-methods.js b/lib/common/web-view-methods.js index 860eab00474da..fd8ba2d7de2a0 100644 --- a/lib/common/web-view-methods.js +++ b/lib/common/web-view-methods.js @@ -46,6 +46,8 @@ exports.syncMethods = new Set([ 'downloadURL', 'inspectServiceWorker', 'showDefinitionForSelection', + 'getZoomFactor', + 'getZoomLevel', 'setZoomFactor', 'setZoomLevel' ]) @@ -57,8 +59,6 @@ exports.asyncCallbackMethods = new Set([ 'sendInputEvent', 'setLayoutZoomLevelLimits', 'setVisualZoomLevelLimits', - 'getZoomFactor', - 'getZoomLevel', 'print', 'printToPDF' ]) diff --git a/spec/api-web-contents-spec.js b/spec/api-web-contents-spec.js index 0a3bc85dd9954..4344a7f6a0f44 100644 --- a/spec/api-web-contents-spec.js +++ b/spec/api-web-contents-spec.js @@ -569,9 +569,8 @@ describe('webContents module', () => { const {ipcRenderer, remote} = require('electron') ipcRenderer.send('set-zoom', window.location.hostname) ipcRenderer.on(window.location.hostname + '-zoom-set', () => { - remote.getCurrentWebContents().getZoomLevel((zoomLevel) => { - ipcRenderer.send(window.location.hostname + '-zoom-level', zoomLevel) - }) + const zoomLevel = remote.getCurrentWebContents().getZoomLevel() + ipcRenderer.send(window.location.hostname + '-zoom-level', zoomLevel) }) ` callback({ data: response, mimeType: 'text/html' }) @@ -584,6 +583,19 @@ describe('webContents module', () => { }) it('can set the correct zoom level', async () => { + try { + await w.loadURL('about:blank') + const zoomLevel = w.webContents.getZoomLevel() + expect(zoomLevel).to.eql(0.0) + w.webContents.setZoomLevel(0.5) + const newZoomLevel = w.webContents.getZoomLevel() + expect(newZoomLevel).to.eql(0.5) + } finally { + w.webContents.setZoomLevel(0) + } + }) + + it('can set the correct zoom level (callback)', async () => { try { await w.loadURL('about:blank') const zoomLevel = await new Promise(resolve => w.webContents.getZoomLevel(resolve)) @@ -626,15 +638,14 @@ describe('webContents module', () => { show: false }) w2.webContents.on('did-finish-load', () => { - w.webContents.getZoomLevel((zoomLevel1) => { - assert.strictEqual(zoomLevel1, hostZoomMap.host3) - w2.webContents.getZoomLevel((zoomLevel2) => { - assert.strictEqual(zoomLevel1, zoomLevel2) - w2.setClosable(true) - w2.close() - done() - }) - }) + const zoomLevel1 = w.webContents.getZoomLevel() + assert.strictEqual(zoomLevel1, hostZoomMap.host3) + + const zoomLevel2 = w2.webContents.getZoomLevel() + assert.strictEqual(zoomLevel1, zoomLevel2) + w2.setClosable(true) + w2.close() + done() }) w.webContents.on('did-finish-load', () => { w.webContents.setZoomLevel(hostZoomMap.host3) @@ -656,18 +667,18 @@ describe('webContents module', () => { }, (error) => { if (error) return done(error) w2.webContents.on('did-finish-load', () => { - w.webContents.getZoomLevel((zoomLevel1) => { - assert.strictEqual(zoomLevel1, hostZoomMap.host3) - w2.webContents.getZoomLevel((zoomLevel2) => { - assert.strictEqual(zoomLevel2, 0) - assert.notStrictEqual(zoomLevel1, zoomLevel2) - protocol.unregisterProtocol(zoomScheme, (error) => { - if (error) return done(error) - w2.setClosable(true) - w2.close() - done() - }) - }) + const zoomLevel1 = w.webContents.getZoomLevel() + assert.strictEqual(zoomLevel1, hostZoomMap.host3) + + const zoomLevel2 = w2.webContents.getZoomLevel() + assert.strictEqual(zoomLevel2, 0) + assert.notStrictEqual(zoomLevel1, zoomLevel2) + + protocol.unregisterProtocol(zoomScheme, (error) => { + if (error) return done(error) + w2.setClosable(true) + w2.close() + done() }) }) w.webContents.on('did-finish-load', () => { @@ -689,12 +700,12 @@ describe('webContents module', () => { const content = `` w.webContents.on('did-frame-finish-load', (e, isMainFrame) => { if (!isMainFrame) { - w.webContents.getZoomLevel((zoomLevel) => { - assert.strictEqual(zoomLevel, 2.0) - w.webContents.setZoomLevel(0) - server.close() - done() - }) + const zoomLevel = w.webContents.getZoomLevel() + assert.strictEqual(zoomLevel, 2.0) + + w.webContents.setZoomLevel(0) + server.close() + done() } }) w.webContents.on('dom-ready', () => { @@ -710,16 +721,16 @@ describe('webContents module', () => { show: false }) w2.webContents.on('did-finish-load', () => { - w.webContents.getZoomLevel((zoomLevel1) => { - assert.strictEqual(zoomLevel1, finalZoomLevel) - w2.webContents.getZoomLevel((zoomLevel2) => { - assert.strictEqual(zoomLevel2, 0) - assert.notStrictEqual(zoomLevel1, zoomLevel2) - w2.setClosable(true) - w2.close() - done() - }) - }) + const zoomLevel1 = w.webContents.getZoomLevel() + assert.strictEqual(zoomLevel1, finalZoomLevel) + + const zoomLevel2 = w2.webContents.getZoomLevel() + assert.strictEqual(zoomLevel2, 0) + assert.notStrictEqual(zoomLevel1, zoomLevel2) + + w2.setClosable(true) + w2.close() + done() }) ipcMain.once('temporary-zoom-set', (e, zoomLevel) => { w2.loadFile(path.join(fixtures, 'pages', 'c.html')) @@ -739,10 +750,9 @@ describe('webContents module', () => { if (initialNavigation) { w.webContents.executeJavaScript(source, () => {}) } else { - w.webContents.getZoomLevel((zoomLevel) => { - assert.strictEqual(zoomLevel, 0) - done() - }) + const zoomLevel = w.webContents.getZoomLevel() + assert.strictEqual(zoomLevel, 0) + done() } }) ipcMain.once('zoom-level-set', (e, zoomLevel) => { diff --git a/spec/fixtures/api/execute-javascript.html b/spec/fixtures/api/execute-javascript.html new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/spec/fixtures/pages/webview-custom-zoom-level.html b/spec/fixtures/pages/webview-custom-zoom-level.html index 24f246fdd2926..fdfa482e2b10b 100644 --- a/spec/fixtures/pages/webview-custom-zoom-level.html +++ b/spec/fixtures/pages/webview-custom-zoom-level.html @@ -10,17 +10,15 @@ if (!finalNavigation && !view.canGoBack()) { view.setZoomLevel(2.0) } - view.getZoomLevel((zoomLevel) => { - view.getZoomFactor((zoomFactor) => { - ipcRenderer.send('webview-zoom-level', zoomLevel, zoomFactor, view.canGoBack(), finalNavigation) - if (!view.canGoBack() && !finalNavigation) { - view.src = 'zoom://host2' - } else if (!finalNavigation) { - finalNavigation = true - view.goBack() - } - }) - }) + const zoomLevel = view.getZoomLevel() + const zoomFactor = view.getZoomFactor() + ipcRenderer.send('webview-zoom-level', zoomLevel, zoomFactor, view.canGoBack(), finalNavigation) + if (!view.canGoBack() && !finalNavigation) { + view.src = 'zoom://host2' + } else if (!finalNavigation) { + finalNavigation = true + view.goBack() + } }) diff --git a/spec/fixtures/pages/webview-in-page-navigate.html b/spec/fixtures/pages/webview-in-page-navigate.html index 5b061e387b99b..a0acd5e2ccfaa 100644 --- a/spec/fixtures/pages/webview-in-page-navigate.html +++ b/spec/fixtures/pages/webview-in-page-navigate.html @@ -8,12 +8,10 @@ let finalNavigation = false function SendZoomLevel() { return new Promise((resolve, reject) => { - view.getZoomLevel((zoomLevel) => { - view.getZoomFactor((zoomFactor) => { - ipcRenderer.send('webview-zoom-in-page', zoomLevel, zoomFactor, finalNavigation) - resolve() - }) - }) + const zoomLevel = view.getZoomLevel() + const zoomFactor = view.getZoomFactor() + ipcRenderer.send('webview-zoom-in-page', zoomLevel, zoomFactor, finalNavigation) + resolve() }) } view.addEventListener('dom-ready', () => { diff --git a/spec/fixtures/pages/webview-origin-zoom-level.html b/spec/fixtures/pages/webview-origin-zoom-level.html index 2c83f7ae56ea4..9dda7b824a37e 100644 --- a/spec/fixtures/pages/webview-origin-zoom-level.html +++ b/spec/fixtures/pages/webview-origin-zoom-level.html @@ -13,8 +13,7 @@ document.body.appendChild(view2) }) view2.addEventListener('dom-ready', () => { - view2.getZoomLevel((level) => { - ipcRenderer.send('webview-origin-zoom-level', level) - }) + const zoomLevel = view2.getZoomLevel() + ipcRenderer.send('webview-origin-zoom-level', zoomLevel) }) From 8826080f02f1e96149f868f504079c852e139b09 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Fri, 18 Jan 2019 13:47:42 -0800 Subject: [PATCH 2/2] update ts defs dep --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index f8ca95d0602b5..c0a5c6497e804 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,7 @@ "dotenv-safe": "^4.0.4", "dugite": "^1.45.0", "electron-docs-linter": "^2.4.0", - "electron-typescript-definitions": "^3.0.0", + "electron-typescript-definitions": "^4.0.0", "eslint": "^5.6.0", "eslint-config-standard": "^12.0.0", "eslint-plugin-mocha": "^5.2.0",