Skip to content

Commit

Permalink
fix: properly pass openExternal activate option (#18721)
Browse files Browse the repository at this point in the history
A reference to an OpenExternalOptions structure was being captured by an Objective-C block that
outlived the object that was being referenced.
  • Loading branch information
jeremyspiegel authored and codebytere committed Jun 13, 2019
1 parent eaa22b4 commit e0d566a
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 10 deletions.
15 changes: 8 additions & 7 deletions atom/common/platform_util_mac.mm
Expand Up @@ -110,14 +110,15 @@ void OpenExternal(const GURL& url,
return;
}

bool activate = options.activate;
__block OpenExternalCallback c = std::move(callback);
dispatch_async(
dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
__block std::string error = OpenURL(ns_url, options.activate);
dispatch_async(dispatch_get_main_queue(), ^{
std::move(c).Run(error);
});
});
dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0),
^{
__block std::string error = OpenURL(ns_url, activate);
dispatch_async(dispatch_get_main_queue(), ^{
std::move(c).Run(error);
});
});
}

bool MoveItemToTrash(const base::FilePath& full_path) {
Expand Down
29 changes: 26 additions & 3 deletions spec/api-shell-spec.js
Expand Up @@ -2,7 +2,10 @@ const assert = require('assert')
const fs = require('fs')
const path = require('path')
const os = require('os')
const { shell } = require('electron')
const { shell, remote } = require('electron')
const { BrowserWindow } = remote

const { closeWindow } = require('./window-helpers')

describe('shell module', () => {
const fixtures = path.resolve(__dirname, 'fixtures')
Expand All @@ -18,6 +21,7 @@ describe('shell module', () => {

describe('shell.openExternal()', () => {
let envVars = {}
let w

beforeEach(function () {
envVars = {
Expand All @@ -27,7 +31,9 @@ describe('shell module', () => {
}
})

afterEach(() => {
afterEach(async () => {
await closeWindow(w)
w = null
// reset env vars to prevent side effects
if (process.platform === 'linux') {
process.env.DE = envVars.de
Expand All @@ -44,7 +50,24 @@ describe('shell module', () => {
process.env.DISPLAY = ''
}

shell.openExternal(url).then(() => done())
// Ensure an external window is activated via a new window's blur event
w = new BrowserWindow()
let promiseResolved = false
let blurEventEmitted = false

w.on('blur', () => {
blurEventEmitted = true
if (promiseResolved) {
done()
}
})

shell.openExternal(url).then(() => {
promiseResolved = true
if (blurEventEmitted || process.platform === 'linux') {
done()
}
})
})

it('opens an external link synchronously', () => {
Expand Down

0 comments on commit e0d566a

Please sign in to comment.