diff --git a/atom/common/api/atom_api_shell.cc b/atom/common/api/atom_api_shell.cc index 7c67c7a3a7a64..0e8d89584e6df 100644 --- a/atom/common/api/atom_api_shell.cc +++ b/atom/common/api/atom_api_shell.cc @@ -10,6 +10,7 @@ #include "atom/common/native_mate_converters/string16_converter.h" #include "atom/common/node_includes.h" #include "atom/common/platform_util.h" +#include "atom/common/promise_util.h" #include "native_mate/dictionary.h" #if defined(OS_WIN) @@ -43,17 +44,16 @@ struct Converter { namespace { -void OnOpenExternalFinished( - v8::Isolate* isolate, - const base::Callback)>& callback, - const std::string& error) { +void OnOpenExternalFinished(v8::Isolate* isolate, + scoped_refptr promise, + const std::string& error) { if (error.empty()) - callback.Run(v8::Null(isolate)); + promise->Resolve(); else - callback.Run(v8::String::NewFromUtf8(isolate, error.c_str())); + promise->RejectWithErrorMessage(error.c_str()); } -bool OpenExternal( +bool OpenExternalSync( #if defined(OS_WIN) const base::string16& url, #else @@ -69,17 +69,36 @@ bool OpenExternal( } } - if (args->Length() >= 3) { - base::Callback)> callback; - if (args->GetNext(&callback)) { - platform_util::OpenExternal( - url, options, - base::Bind(&OnOpenExternalFinished, args->isolate(), callback)); - return true; + return platform_util::OpenExternal(url, options); +} + +v8::Local OpenExternal( +#if defined(OS_WIN) + const base::string16& url, +#else + const GURL& url, +#endif + mate::Arguments* args) { + + v8::Locker locker(args->isolate()); + v8::HandleScope handle_scope(args->isolate()); + scoped_refptr promise = + new atom::util::Promise(args->isolate()); + + platform_util::OpenExternalOptions options; + if (args->Length() >= 2) { + mate::Dictionary obj; + if (args->GetNext(&obj)) { + obj.Get("activate", &options.activate); + obj.Get("workingDirectory", &options.working_dir); } } - return platform_util::OpenExternal(url, options); + platform_util::OpenExternal( + url, options, + base::Bind(&OnOpenExternalFinished, args->isolate(), promise)); + + return promise->GetHandle(); } #if defined(OS_WIN) @@ -144,6 +163,7 @@ void Initialize(v8::Local exports, mate::Dictionary dict(context->GetIsolate(), exports); dict.SetMethod("showItemInFolder", &platform_util::ShowItemInFolder); dict.SetMethod("openItem", &platform_util::OpenItem); + dict.SetMethod("openExternalSync", &OpenExternalSync); dict.SetMethod("openExternal", &OpenExternal); dict.SetMethod("moveItemToTrash", &platform_util::MoveItemToTrash); dict.SetMethod("beep", &platform_util::Beep); diff --git a/default_app/menu.js b/default_app/menu.js index 138734b565cb4..7dd71017791a0 100644 --- a/default_app/menu.js +++ b/default_app/menu.js @@ -13,13 +13,13 @@ const setDefaultApplicationMenu = () => { { label: 'Learn More', click () { - shell.openExternal('https://electronjs.org') + shell.openExternalSync('https://electronjs.org') } }, { label: 'Documentation', click () { - shell.openExternal( + shell.openExternalSync( `https://github.com/electron/electron/tree/v${process.versions.electron}/docs#readme` ) } @@ -27,13 +27,13 @@ const setDefaultApplicationMenu = () => { { label: 'Community Discussions', click () { - shell.openExternal('https://discuss.atom.io/c/electron') + shell.openExternalSync('https://discuss.atom.io/c/electron') } }, { label: 'Search Issues', click () { - shell.openExternal('https://github.com/electron/electron/issues') + shell.openExternalSync('https://github.com/electron/electron/issues') } } ] diff --git a/default_app/renderer.js b/default_app/renderer.js index 2107d798132c8..6d26007b7a8e2 100644 --- a/default_app/renderer.js +++ b/default_app/renderer.js @@ -21,7 +21,7 @@ function initialize () { const openLinkExternally = (e) => { e.preventDefault() - shell.openExternal(url) + shell.openExternalSync(url) } link.addEventListener('click', openLinkExternally) diff --git a/docs/api/menu.md b/docs/api/menu.md index d2c7b872c871e..4e31779f7982e 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -245,7 +245,7 @@ const template = [ submenu: [ { label: 'Learn More', - click () { require('electron').shell.openExternal('https://electronjs.org') } + click () { require('electron').shell.openExternalSync('https://electronjs.org') } } ] } diff --git a/docs/api/promisification.md b/docs/api/promisification.md index 921f8e0f393c6..c1794a7e54459 100644 --- a/docs/api/promisification.md +++ b/docs/api/promisification.md @@ -8,7 +8,6 @@ When a majority of affected functions are migrated, this flag will be enabled by ### Candidate Functions -- [ ] [app.getFileIcon(path[, options], callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#getFileIcon) - [ ] [app.importCertificate(options, callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#importCertificate) - [ ] [request.write(chunk[, encoding][, callback])](https://github.com/electron/electron/blob/master/docs/api/client-request.md#write) - [ ] [request.end([chunk][, encoding][, callback])](https://github.com/electron/electron/blob/master/docs/api/client-request.md#end) @@ -41,7 +40,6 @@ When a majority of affected functions are migrated, this flag will be enabled by - [ ] [ses.clearHostResolverCache([callback])](https://github.com/electron/electron/blob/master/docs/api/session.md#clearHostResolverCache) - [ ] [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) -- [ ] [shell.openExternal(url[, options, callback])](https://github.com/electron/electron/blob/master/docs/api/shell.md#openExternal) - [ ] [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) @@ -61,4 +59,6 @@ When a majority of affected functions are migrated, this flag will be enabled by - [ ] [win.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/browser-window.md#capturePage) - [ ] [webviewTag.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/webview-tag.md#capturePage) -- [ ] [contents.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#capturePage) \ No newline at end of file +- [ ] [contents.capturePage([rect, ]callback)](https://github.com/electron/electron/blob/master/docs/api/web-contents.md#capturePage) +- [ ] [app.getFileIcon(path[, options], callback)](https://github.com/electron/electron/blob/master/docs/api/app.md#getFileIcon) +- [ ] [shell.openExternal(url[, options, callback])](https://github.com/electron/electron/blob/master/docs/api/shell.md#openExternal) \ No newline at end of file diff --git a/docs/api/shell.md b/docs/api/shell.md index b38348ab6bdb4..e2c6781830af7 100644 --- a/docs/api/shell.md +++ b/docs/api/shell.md @@ -34,21 +34,29 @@ Returns `Boolean` - Whether the item was successfully opened. Open the given file in the desktop's default manner. -### `shell.openExternal(url[, options, callback])` +### `shell.openExternalSync(url[, options])` -* `url` String - Max 2081 characters on windows, or the function returns false. +* `url` String - Max 2081 characters on Windows, or the function returns false. * `options` Object (optional) * `activate` Boolean (optional) - `true` to bring the opened application to the foreground. The default is `true`. _macOS_ * `workingDirectory` String (optional) - The working directory. _Windows_ -* `callback` Function (optional) _macOS_ - If specified will perform the open asynchronously. - * `error` Error Returns `Boolean` - Whether an application was available to open the URL. -If callback is specified, always returns true. -Open the given external protocol URL in the desktop's default manner. (For -example, mailto: URLs in the user's default mail agent). +Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent). + +### `shell.openExternal(url[, options])` + +* `url` String - Max 2081 characters on windows. +* `options` Object (optional) + * `activate` Boolean (optional) - `true` to bring the opened application to the + foreground. The default is `true`. _macOS_ + * `workingDirectory` String (optional) - The working directory. _Windows_ + +Returns `Promise` + +Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent). ### `shell.moveItemToTrash(fullPath)` diff --git a/docs/api/webview-tag.md b/docs/api/webview-tag.md index ad0afaa27f7d7..bab79f4086b41 100644 --- a/docs/api/webview-tag.md +++ b/docs/api/webview-tag.md @@ -782,7 +782,7 @@ const webview = document.querySelector('webview') webview.addEventListener('new-window', (e) => { const protocol = require('url').parse(e.url).protocol if (protocol === 'http:' || protocol === 'https:') { - shell.openExternal(e.url) + shell.openExternalSync(e.url) } }) ``` diff --git a/docs/tutorial/security.md b/docs/tutorial/security.md index b9e4a0fd28d4c..4a3300ee7f243 100644 --- a/docs/tutorial/security.md +++ b/docs/tutorial/security.md @@ -639,7 +639,7 @@ app.on('web-contents-created', (event, contents) => { // to open this event's url in the default browser. event.preventDefault() - shell.openExternal(navigationUrl) + shell.openExternalSync(navigationUrl) }) }) ``` diff --git a/spec/api-remote-spec.js b/spec/api-remote-spec.js index 6b6a21c002a80..b7567fc77424e 100644 --- a/spec/api-remote-spec.js +++ b/spec/api-remote-spec.js @@ -198,7 +198,7 @@ describe('remote module', () => { assert.strictEqual(typeof remote.app.getPath, 'function') assert.strictEqual(typeof remote.webContents.getFocusedWebContents, 'function') assert.strictEqual(typeof remote.clipboard.readText, 'function') - assert.strictEqual(typeof remote.shell.openExternal, 'function') + assert.strictEqual(typeof remote.shell.openExternalSync, 'function') }) it('returns toString() of original function via toString()', () => { diff --git a/spec/api-shell-spec.js b/spec/api-shell-spec.js index 720b75da4a9de..9b4983b9834f3 100644 --- a/spec/api-shell-spec.js +++ b/spec/api-shell-spec.js @@ -16,14 +16,55 @@ describe('shell module', () => { iconIndex: 1 } - // (alexeykuzmin): `.skip()` in `before` doesn't work for nested `describe`s. - beforeEach(function () { - if (process.platform !== 'win32') { - this.skip() - } + describe('shell.openExternal()', () => { + let envVars = {} + + beforeEach(function () { + envVars = { + display: process.env.DISPLAY, + de: process.env.DE, + browser: process.env.BROWSER + } + }) + + afterEach(() => { + // reset env vars to prevent side effects + if (process.platform === 'linux') { + process.env.DE = envVars.de + process.env.BROWSER = envVars.browser + process.env.DISPLAY = envVars.display + } + }) + + it('opens an external link asynchronously', done => { + const url = 'http://www.example.com' + if (process.platform === 'linux') { + process.env.BROWSER = '/bin/true' + process.env.DE = 'generic' + process.env.DISPLAY = '' + } + + shell.openExternal(url).then(() => done()) + }) + + it('opens an external link synchronously', () => { + const url = 'http://www.example.com' + if (process.platform === 'linux') { + process.env.DE = 'generic' + process.env.DE = '/bin/true' + process.env.DISPLAY = '' + } + + const success = shell.openExternalSync(url) + assert.strictEqual(true, success) + }) }) describe('shell.readShortcutLink(shortcutPath)', () => { + beforeEach(function () { + if (process.platform !== 'win32') this.skip() + }) + it('throws when failed', () => { assert.throws(() => { shell.readShortcutLink('not-exist') @@ -37,6 +78,10 @@ describe('shell module', () => { }) describe('shell.writeShortcutLink(shortcutPath[, operation], options)', () => { + beforeEach(function () { + if (process.platform !== 'win32') this.skip() + }) + const tmpShortcut = path.join(os.tmpdir(), `${Date.now()}.lnk`) afterEach(() => { diff --git a/spec/package-lock.json b/spec/package-lock.json index 2717758d64ae6..737fa4094fec2 100644 --- a/spec/package-lock.json +++ b/spec/package-lock.json @@ -72,7 +72,7 @@ }, "bl": { "version": "1.2.2", - "resolved": "http://registry.npmjs.org/bl/-/bl-1.2.2.tgz", + "resolved": "https://registry.npmjs.org/bl/-/bl-1.2.2.tgz", "integrity": "sha512-e8tQYnZodmebYDWGH7KMRvtzKXaJHx3BbilrgZCfvyLUYdKpK1t5PSPmpkny/SgiTSCnjfLW7v5rlONXVFkQEA==", "optional": true, "requires": { @@ -228,7 +228,7 @@ }, "commander": { "version": "2.15.1", - "resolved": "http://registry.npmjs.org/commander/-/commander-2.15.1.tgz", + "resolved": "https://registry.npmjs.org/commander/-/commander-2.15.1.tgz", "integrity": "sha512-VlfT9F3V0v+jr4yxPc5gg9s62/fIVWsd2Bk2iD435um1NlGMYdVCq+MjcXnhYq2icNOizHr1kK+5TI6H0Hy0ag==", "dev": true }, @@ -267,7 +267,7 @@ }, "dbus-native": { "version": "0.2.5", - "resolved": "http://registry.npmjs.org/dbus-native/-/dbus-native-0.2.5.tgz", + "resolved": "https://registry.npmjs.org/dbus-native/-/dbus-native-0.2.5.tgz", "integrity": "sha512-ocxMKCV7QdiNhzhFSeEMhj258OGtvpANSb3oWGiotmI5h1ZIse0TMPcSLiXSpqvbYvQz2Y5RsYPMNYLWhg9eBw==", "dev": true, "requires": { @@ -358,7 +358,7 @@ }, "duplexer": { "version": "0.1.1", - "resolved": "http://registry.npmjs.org/duplexer/-/duplexer-0.1.1.tgz", + "resolved": "https://registry.npmjs.org/duplexer/-/duplexer-0.1.1.tgz", "integrity": "sha1-rOb/gIwc5mtX0ev5eXessCM0z8E=", "dev": true }, @@ -508,7 +508,7 @@ }, "get-stream": { "version": "3.0.0", - "resolved": "http://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz", + "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz", "integrity": "sha1-jpQ9E1jcN1VQVOy+LtsFqhdO3hQ=", "dev": true }, @@ -749,7 +749,7 @@ }, "mkdirp": { "version": "0.5.1", - "resolved": "http://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", + "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "requires": { "minimist": "0.0.8" @@ -757,7 +757,7 @@ "dependencies": { "minimist": { "version": "0.0.8", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=" } } @@ -937,7 +937,7 @@ }, "os-homedir": { "version": "1.0.2", - "resolved": "http://registry.npmjs.org/os-homedir/-/os-homedir-1.0.2.tgz", + "resolved": "https://registry.npmjs.org/os-homedir/-/os-homedir-1.0.2.tgz", "integrity": "sha1-/7xJiDNuDoM94MFox+8VISGqf7M=", "optional": true }, @@ -954,7 +954,7 @@ }, "os-tmpdir": { "version": "1.0.2", - "resolved": "http://registry.npmjs.org/os-tmpdir/-/os-tmpdir-1.0.2.tgz", + "resolved": "https://registry.npmjs.org/os-tmpdir/-/os-tmpdir-1.0.2.tgz", "integrity": "sha1-u+Z0BseaqFxc/sdm/lc0VV36EnQ=", "dev": true }, @@ -996,7 +996,7 @@ }, "path-is-absolute": { "version": "1.0.1", - "resolved": "http://registry.npmjs.org/path-is-absolute/-/path-is-absolute-1.0.1.tgz", + "resolved": "https://registry.npmjs.org/path-is-absolute/-/path-is-absolute-1.0.1.tgz", "integrity": "sha1-F0uSaHNVNP+8es5r9TpanhtcX18=", "dev": true }, @@ -1014,7 +1014,7 @@ }, "pause-stream": { "version": "0.0.11", - "resolved": "http://registry.npmjs.org/pause-stream/-/pause-stream-0.0.11.tgz", + "resolved": "https://registry.npmjs.org/pause-stream/-/pause-stream-0.0.11.tgz", "integrity": "sha1-/lo0sMvOErWqaitAPuLnO2AvFEU=", "dev": true, "requires": { @@ -1135,7 +1135,7 @@ }, "rimraf": { "version": "2.2.8", - "resolved": "http://registry.npmjs.org/rimraf/-/rimraf-2.2.8.tgz", + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.2.8.tgz", "integrity": "sha1-5Dm+Kq7jJzIZUnMPmaiSnk/FBYI=", "dev": true }, @@ -1310,7 +1310,7 @@ }, "string_decoder": { "version": "1.1.1", - "resolved": "http://registry.npmjs.org/string_decoder/-/string_decoder-1.1.1.tgz", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.1.1.tgz", "integrity": "sha512-n/ShnvDi6FHbbVfviro+WojiFzv+s8MPMHBczVePfUpDJLwoLT0ht1l4YwBCbi8pJAveEEdnkHyPyTP/mzRfwg==", "requires": { "safe-buffer": "~5.1.0" @@ -1326,7 +1326,7 @@ }, "strip-eof": { "version": "1.0.0", - "resolved": "http://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", + "resolved": "https://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", "integrity": "sha1-u0P/VZim6wXYm1n80SnJgzE2Br8=", "dev": true }, @@ -1396,7 +1396,7 @@ }, "through": { "version": "2.3.8", - "resolved": "http://registry.npmjs.org/through/-/through-2.3.8.tgz", + "resolved": "https://registry.npmjs.org/through/-/through-2.3.8.tgz", "integrity": "sha1-DdTJ/6q8NXlgsbckEV1+Doai4fU=", "dev": true }, @@ -1491,7 +1491,7 @@ }, "wrap-ansi": { "version": "2.1.0", - "resolved": "http://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", + "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", "integrity": "sha1-2Pw9KE3QV5T+hJc8rs3Rz4JP3YU=", "dev": true, "requires": {