diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 9401eae5b1a..c5bec7b69a8 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -794,14 +794,10 @@ void AtomBrowserClient::RenderProcessExited( } void OnOpenExternal(const GURL& escaped_url, bool allowed) { - if (allowed) + if (allowed) { platform_util::OpenExternal( -#if defined(OS_WIN) - base::UTF8ToUTF16(escaped_url.spec()), -#else - escaped_url, -#endif - platform_util::OpenExternalOptions()); + escaped_url, platform_util::OpenExternalOptions(), base::DoNothing()); + } } void HandleExternalProtocolInUI( diff --git a/atom/common/api/atom_api_shell.cc b/atom/common/api/atom_api_shell.cc index bd34c42cdf6..e1e61a2ae11 100644 --- a/atom/common/api/atom_api_shell.cc +++ b/atom/common/api/atom_api_shell.cc @@ -52,33 +52,9 @@ void OnOpenExternalFinished(atom::util::Promise promise, promise.RejectWithErrorMessage(error.c_str()); } -bool OpenExternalSync( -#if defined(OS_WIN) - const base::string16& url, -#else - const GURL& url, -#endif - mate::Arguments* args) { - 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); -} - -v8::Local OpenExternal( -#if defined(OS_WIN) - const base::string16& url, -#else - const GURL& url, -#endif - mate::Arguments* args) { +v8::Local OpenExternal(const GURL& url, mate::Arguments* args) { atom::util::Promise promise(args->isolate()); + v8::Local handle = promise.GetHandle(); platform_util::OpenExternalOptions options; if (args->Length() >= 2) { @@ -89,7 +65,6 @@ v8::Local OpenExternal( } } - v8::Local handle = promise.GetHandle(); platform_util::OpenExternal( url, options, base::BindOnce(&OnOpenExternalFinished, std::move(promise))); @@ -158,7 +133,6 @@ 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/atom/common/platform_util.h b/atom/common/platform_util.h index fbdfa2ccf2f..2f3f8b7f1da 100644 --- a/atom/common/platform_util.h +++ b/atom/common/platform_util.h @@ -36,23 +36,9 @@ struct OpenExternalOptions { // Open the given external protocol URL in the desktop's default manner. // (For example, mailto: URLs in the default mail user agent.) -bool OpenExternal( -#if defined(OS_WIN) - const base::string16& url, -#else - const GURL& url, -#endif - const OpenExternalOptions& options); - -// The asynchronous version of OpenExternal. -void OpenExternal( -#if defined(OS_WIN) - const base::string16& url, -#else - const GURL& url, -#endif - const OpenExternalOptions& options, - OpenExternalCallback callback); +void OpenExternal(const GURL& url, + const OpenExternalOptions& options, + OpenExternalCallback callback); // Move a file to trash. bool MoveItemToTrash(const base::FilePath& full_path); diff --git a/atom/common/platform_util_linux.cc b/atom/common/platform_util_linux.cc index 9c061208ef4..899ded59724 100644 --- a/atom/common/platform_util_linux.cc +++ b/atom/common/platform_util_linux.cc @@ -81,20 +81,16 @@ bool OpenItem(const base::FilePath& full_path) { return XDGOpen(full_path.value(), false); } -bool OpenExternal(const GURL& url, const OpenExternalOptions& options) { - // Don't wait for exit, since we don't want to wait for the browser/email - // client window to close before returning - if (url.SchemeIs("mailto")) - return XDGEmail(url.spec(), false); - else - return XDGOpen(url.spec(), false); -} - void OpenExternal(const GURL& url, const OpenExternalOptions& options, OpenExternalCallback callback) { - // TODO(gabriel): Implement async open if callback is specified - std::move(callback).Run(OpenExternal(url, options) ? "" : "Failed to open"); + // Don't wait for exit, since we don't want to wait for the browser/email + // client window to close before returning + if (url.SchemeIs("mailto")) + std::move(callback).Run(XDGEmail(url.spec(), false) ? "" + : "Failed to open"); + else + std::move(callback).Run(XDGOpen(url.spec(), false) ? "" : "Failed to open"); } bool MoveItemToTrash(const base::FilePath& full_path) { diff --git a/atom/common/platform_util_mac.mm b/atom/common/platform_util_mac.mm index 7f83603218c..9f6fa9d3023 100644 --- a/atom/common/platform_util_mac.mm +++ b/atom/common/platform_util_mac.mm @@ -94,17 +94,10 @@ bool OpenItem(const base::FilePath& full_path) { launchIdentifiers:NULL]; } -bool OpenExternal(const GURL& url, const OpenExternalOptions& options) { - DCHECK([NSThread isMainThread]); - NSURL* ns_url = net::NSURLWithGURL(url); - if (ns_url) - return OpenURL(ns_url, options.activate).empty(); - return false; -} - void OpenExternal(const GURL& url, const OpenExternalOptions& options, OpenExternalCallback callback) { + DCHECK([NSThread isMainThread]); NSURL* ns_url = net::NSURLWithGURL(url); if (!ns_url) { std::move(callback).Run("Invalid URL"); diff --git a/atom/common/platform_util_win.cc b/atom/common/platform_util_win.cc index acb19e6b1df..086166bb95e 100644 --- a/atom/common/platform_util_win.cc +++ b/atom/common/platform_util_win.cc @@ -230,6 +230,29 @@ HRESULT DeleteFileProgressSink::ResumeTimer() { return S_OK; } +std::string OpenExternalOnWorkerThread( + const GURL& url, + const platform_util::OpenExternalOptions& options) { + base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, + base::BlockingType::MAY_BLOCK); + // Quote the input scheme to be sure that the command does not have + // parameters unexpected by the external program. This url should already + // have been escaped. + std::string escaped_url = url.spec(); + escaped_url.insert(0, "\""); + escaped_url += "\""; + + std::string working_dir = options.working_dir.AsUTF8Unsafe(); + + if (reinterpret_cast( + ShellExecuteA(nullptr, "open", escaped_url.c_str(), nullptr, + working_dir.empty() ? nullptr : working_dir.c_str(), + SW_SHOWNORMAL)) <= 32) { + return "Failed to open"; + } + return ""; +} + void ShowItemInFolderOnWorkerThread(const base::FilePath& full_path) { base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, base::BlockingType::MAY_BLOCK); @@ -301,31 +324,15 @@ bool OpenItem(const base::FilePath& full_path) { return ui::win::OpenFileViaShell(full_path); } -bool OpenExternal(const base::string16& url, - const OpenExternalOptions& options) { - // Quote the input scheme to be sure that the command does not have - // parameters unexpected by the external program. This url should already - // have been escaped. - base::string16 escaped_url = L"\"" + url + L"\""; - auto working_dir = options.working_dir.value(); - - if (reinterpret_cast( - ShellExecuteW(nullptr, L"open", escaped_url.c_str(), nullptr, - working_dir.empty() ? nullptr : working_dir.c_str(), - SW_SHOWNORMAL)) <= 32) { - // We fail to execute the call. We could display a message to the user. - // TODO(nsylvain): we should also add a dialog to warn on errors. See - // bug 1136923. - return false; - } - return true; -} - -void OpenExternal(const base::string16& url, +void OpenExternal(const GURL& url, const OpenExternalOptions& options, OpenExternalCallback callback) { - // TODO(gabriel): Implement async open if callback is specified - std::move(callback).Run(OpenExternal(url, options) ? "" : "Failed to open"); + base::PostTaskAndReplyWithResult( + base::CreateCOMSTATaskRunnerWithTraits( + {base::MayBlock(), base::TaskPriority::USER_BLOCKING}) + .get(), + FROM_HERE, base::BindOnce(&OpenExternalOnWorkerThread, url, options), + std::move(callback)); } bool MoveItemToTrash(const base::FilePath& path) { diff --git a/docs/api/menu.md b/docs/api/menu.md index 0727d1476ee..ae864eef83f 100644 --- a/docs/api/menu.md +++ b/docs/api/menu.md @@ -240,7 +240,10 @@ const template = [ submenu: [ { label: 'Learn More', - click () { require('electron').shell.openExternalSync('https://electronjs.org') } + click: async () => { + const { shell } = require('electron') + await shell.openExternal('https://electronjs.org') + } } ] } diff --git a/docs/api/shell.md b/docs/api/shell.md index fa06be7780d..ff085164bcc 100644 --- a/docs/api/shell.md +++ b/docs/api/shell.md @@ -32,18 +32,6 @@ Returns `Boolean` - Whether the item was successfully opened. Open the given file in the desktop's default manner. -### `shell.openExternalSync(url[, options])` - -* `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_ - -Returns `Boolean` - Whether an application was available to open the URL. - -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. diff --git a/docs/api/webview-tag.md b/docs/api/webview-tag.md index 523ddaae108..459b2238e30 100644 --- a/docs/api/webview-tag.md +++ b/docs/api/webview-tag.md @@ -774,10 +774,10 @@ The following example code opens the new url in system's default browser. const { shell } = require('electron') const webview = document.querySelector('webview') -webview.addEventListener('new-window', (e) => { +webview.addEventListener('new-window', async (e) => { const protocol = require('url').parse(e.url).protocol if (protocol === 'http:' || protocol === 'https:') { - shell.openExternalSync(e.url) + await shell.openExternal(e.url) } }) ``` diff --git a/docs/tutorial/electron-timelines.md b/docs/tutorial/electron-timelines.md index e9b5f9d06b4..bceab234341 100644 --- a/docs/tutorial/electron-timelines.md +++ b/docs/tutorial/electron-timelines.md @@ -47,4 +47,4 @@ Take a look at 5.0.0 Timeline [blog post](https://electronjs.org/blog/electron-5 | Tue, 2019-Jul-30 | 6.0.0 | ✨ stable ✨ | ## 7.0.0 Release Schedule -TBD \ No newline at end of file +TBD diff --git a/docs/tutorial/security.md b/docs/tutorial/security.md index d97d8918284..93e60c3d204 100644 --- a/docs/tutorial/security.md +++ b/docs/tutorial/security.md @@ -681,12 +681,12 @@ windows, limiting it to only what you need. const { shell } = require('electron') app.on('web-contents-created', (event, contents) => { - contents.on('new-window', (event, navigationUrl) => { + contents.on('new-window', async (event, navigationUrl) => { // In this example, we'll ask the operating system // to open this event's url in the default browser. event.preventDefault() - shell.openExternalSync(navigationUrl) + await shell.openExternal(navigationUrl) }) }) ``` diff --git a/lib/browser/default-menu.ts b/lib/browser/default-menu.ts index eedf8df0246..9f15002ff5a 100644 --- a/lib/browser/default-menu.ts +++ b/lib/browser/default-menu.ts @@ -12,28 +12,27 @@ export const setDefaultApplicationMenu = () => { submenu: [ { label: 'Learn More', - click () { - shell.openExternalSync('https://electronjs.org') + click: async () => { + await shell.openExternal('https://electronjs.org') } }, { label: 'Documentation', - click () { - shell.openExternalSync( - `https://github.com/electron/electron/tree/v${process.versions.electron}/docs#readme` - ) + click: async () => { + const version = process.versions.electron + await shell.openExternal(`https://github.com/electron/electron/tree/v${version}/docs#readme`) } }, { label: 'Community Discussions', - click () { - shell.openExternalSync('https://discuss.atom.io/c/electron') + click: async () => { + await shell.openExternal('https://discuss.atom.io/c/electron') } }, { label: 'Search Issues', - click () { - shell.openExternalSync('https://github.com/electron/electron/issues') + click: async () => { + await shell.openExternal('https://github.com/electron/electron/issues') } } ] diff --git a/spec/api-remote-spec.js b/spec/api-remote-spec.js index b7567fc7742..616efd3ae4d 100644 --- a/spec/api-remote-spec.js +++ b/spec/api-remote-spec.js @@ -198,7 +198,6 @@ 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.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 9b4983b9834..6089cc45124 100644 --- a/spec/api-shell-spec.js +++ b/spec/api-shell-spec.js @@ -36,7 +36,7 @@ describe('shell module', () => { } }) - it('opens an external link asynchronously', done => { + it('opens an external link', done => { const url = 'http://www.example.com' if (process.platform === 'linux') { process.env.BROWSER = '/bin/true' @@ -46,18 +46,6 @@ describe('shell module', () => { 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)', () => { diff --git a/spec/ts-smoke/electron/main.ts b/spec/ts-smoke/electron/main.ts index 7753bbe71dc..9c1a300f06f 100644 --- a/spec/ts-smoke/electron/main.ts +++ b/spec/ts-smoke/electron/main.ts @@ -997,7 +997,7 @@ shell.moveItemToTrash('/home/user/Desktop/test.txt') shell.openExternal('https://github.com', { activate: false -}) +}).then(() => {}) shell.beep() diff --git a/spec/ts-smoke/electron/renderer.ts b/spec/ts-smoke/electron/renderer.ts index a3aa9708cb3..b8d68d2d510 100644 --- a/spec/ts-smoke/electron/renderer.ts +++ b/spec/ts-smoke/electron/renderer.ts @@ -218,7 +218,7 @@ app.on('ready', () => { // shell // https://github.com/atom/electron/blob/master/docs/api/shell.md -shell.openExternal('https://github.com') +shell.openExternal('https://github.com').then(() => {}) // // https://github.com/atom/electron/blob/master/docs/api/web-view-tag.md @@ -238,8 +238,9 @@ webview.addEventListener('found-in-page', function (e) { const requestId = webview.findInPage('test') -webview.addEventListener('new-window', function (e) { - require('electron').shell.openExternal(e.url) +webview.addEventListener('new-window', async e => { + const { shell } = require('electron') + await shell.openExternal(e.url) }) webview.addEventListener('close', function () {