diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts index 99bcd4c1cb7c4..8b6c3b6c36e95 100644 --- a/lib/browser/api/web-contents.ts +++ b/lib/browser/api/web-contents.ts @@ -500,6 +500,7 @@ WebContents.prototype._callWindowOpenHandler = function (event: Electron.Event, if (!this._windowOpenHandler) { return null; } + const response = this._windowOpenHandler(details); if (typeof response !== 'object') { @@ -656,7 +657,15 @@ WebContents.prototype._init = function () { postBody, disposition }; - const options = this._callWindowOpenHandler(event, details); + + let options: ReturnType; + try { + options = this._callWindowOpenHandler(event, details); + } catch (err) { + event.preventDefault(); + throw err; + } + if (!event.defaultPrevented) { openGuestWindow({ event, @@ -684,7 +693,16 @@ WebContents.prototype._init = function () { referrer, postBody }; - windowOpenOverriddenOptions = this._callWindowOpenHandler(event, details); + + let result: ReturnType; + try { + result = this._callWindowOpenHandler(event, details); + } catch (err) { + event.preventDefault(); + throw err; + } + + windowOpenOverriddenOptions = result; if (!event.defaultPrevented) { const secureOverrideWebPreferences = windowOpenOverriddenOptions ? { // Allow setting of backgroundColor as a webPreference even though diff --git a/spec-main/guest-window-manager-spec.ts b/spec-main/guest-window-manager-spec.ts index 7d802709c66f1..974e8fce452c2 100644 --- a/spec-main/guest-window-manager-spec.ts +++ b/spec-main/guest-window-manager-spec.ts @@ -79,6 +79,58 @@ describe('webContents.setWindowOpenHandler', () => { afterEach(closeAllWindows); + it('does not fire window creation events if the handler callback throws an error', (done) => { + const error = new Error('oh no'); + const listeners = process.listeners('uncaughtException'); + process.removeAllListeners('uncaughtException'); + process.on('uncaughtException', (thrown) => { + try { + expect(thrown).to.equal(error); + done(); + } catch (e) { + done(e); + } finally { + process.removeAllListeners('uncaughtException'); + listeners.forEach((listener) => process.on('uncaughtException', listener)); + } + }); + + browserWindow.webContents.on('new-window', () => { + assert.fail('new-window should not be called with an overridden window.open'); + }); + + browserWindow.webContents.on('did-create-window', () => { + assert.fail('did-create-window should not be called with an overridden window.open'); + }); + + browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no') && true"); + + browserWindow.webContents.setWindowOpenHandler(() => { + throw error; + }); + }); + + it('does not fire window creation events if the handler callback returns a bad result', async () => { + const bad = new Promise((resolve) => { + browserWindow.webContents.setWindowOpenHandler(() => { + setTimeout(resolve); + return [1, 2, 3] as any; + }); + }); + + browserWindow.webContents.on('new-window', () => { + assert.fail('new-window should not be called with an overridden window.open'); + }); + + browserWindow.webContents.on('did-create-window', () => { + assert.fail('did-create-window should not be called with an overridden window.open'); + }); + + browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no') && true"); + + await bad; + }); + it('does not fire window creation events if an override returns action: deny', async () => { const denied = new Promise((resolve) => { browserWindow.webContents.setWindowOpenHandler(() => { @@ -87,11 +139,11 @@ describe('webContents.setWindowOpenHandler', () => { }); }); browserWindow.webContents.on('new-window', () => { - assert.fail('new-window should not to be called with an overridden window.open'); + assert.fail('new-window should not be called with an overridden window.open'); }); browserWindow.webContents.on('did-create-window', () => { - assert.fail('did-create-window should not to be called with an overridden window.open'); + assert.fail('did-create-window should not be called with an overridden window.open'); }); browserWindow.webContents.executeJavaScript("window.open('about:blank', '', 'show=no') && true"); @@ -107,11 +159,11 @@ describe('webContents.setWindowOpenHandler', () => { }); }); browserWindow.webContents.on('new-window', () => { - assert.fail('new-window should not to be called with an overridden window.open'); + assert.fail('new-window should not be called with an overridden window.open'); }); browserWindow.webContents.on('did-create-window', () => { - assert.fail('did-create-window should not to be called with an overridden window.open'); + assert.fail('did-create-window should not be called with an overridden window.open'); }); await browserWindow.webContents.loadURL('data:text/html,link'); @@ -129,11 +181,11 @@ describe('webContents.setWindowOpenHandler', () => { }); }); browserWindow.webContents.on('new-window', () => { - assert.fail('new-window should not to be called with an overridden window.open'); + assert.fail('new-window should not be called with an overridden window.open'); }); browserWindow.webContents.on('did-create-window', () => { - assert.fail('did-create-window should not to be called with an overridden window.open'); + assert.fail('did-create-window should not be called with an overridden window.open'); }); await browserWindow.webContents.loadURL('data:text/html,link');