diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts index e9aab2cf829..8cd01fa134e 100644 --- a/lib/browser/api/web-contents.ts +++ b/lib/browser/api/web-contents.ts @@ -433,7 +433,11 @@ WebContents.prototype._callWindowOpenHandler = function (event: Electron.Event, event.preventDefault(); return null; } else if (response.action === 'allow') { - if (typeof response.overrideBrowserWindowOptions === 'object' && response.overrideBrowserWindowOptions !== null) { return response.overrideBrowserWindowOptions; } else { return {}; } + if (typeof response.overrideBrowserWindowOptions === 'object' && response.overrideBrowserWindowOptions !== null) { + return response.overrideBrowserWindowOptions; + } else { + return {}; + } } else { event.preventDefault(); console.error('The window open handler response must be an object with an \'action\' property of \'allow\' or \'deny\'.'); @@ -565,19 +569,22 @@ WebContents.prototype._init = function () { // Make new windows requested by links behave like "window.open". this.on('-new-window' as any, (event: ElectronInternal.Event, url: string, frameName: string, disposition: string, rawFeatures: string, referrer: Electron.Referrer, postData: PostData) => { - openGuestWindow({ - event, - embedder: event.sender, - disposition, - referrer, - postData, - overrideBrowserWindowOptions: {}, - windowOpenArgs: { - url, - frameName, - features: rawFeatures - } - }); + const options = this._callWindowOpenHandler(event, url, frameName, rawFeatures); + if (!event.defaultPrevented) { + openGuestWindow({ + event, + embedder: event.sender, + disposition, + referrer, + postData, + overrideBrowserWindowOptions: options || {}, + windowOpenArgs: { + url, + frameName, + features: rawFeatures + } + }); + } }); let windowOpenOverriddenOptions: BrowserWindowConstructorOptions | null = null; diff --git a/lib/browser/guest-window-manager.ts b/lib/browser/guest-window-manager.ts index 0200151447b..c97c8edd025 100644 --- a/lib/browser/guest-window-manager.ts +++ b/lib/browser/guest-window-manager.ts @@ -42,7 +42,6 @@ export function openGuestWindow ({ event, embedder, guest, referrer, disposition windowOpenArgs: WindowOpenArgs, }): BrowserWindow | undefined { const { url, frameName, features } = windowOpenArgs; - const isNativeWindowOpen = !!guest; const { options: browserWindowOptions, additionalFeatures } = makeBrowserWindowOptions({ embedder, features, @@ -74,11 +73,14 @@ export function openGuestWindow ({ event, embedder, guest, referrer, disposition webContents: guest, ...browserWindowOptions }); - if (!isNativeWindowOpen) { + if (!guest) { // We should only call `loadURL` if the webContents was constructed by us in // the case of BrowserWindowProxy (non-sandboxed, nativeWindowOpen: false), // as navigating to the url when creating the window from an existing // webContents is not necessary (it will navigate there anyway). + // This can also happen if we enter this function from OpenURLFromTab, in + // which case the browser process is responsible for initiating navigation + // in the new window. window.loadURL(url, { httpReferrer: referrer, ...(postData && { diff --git a/spec-main/guest-window-manager-spec.ts b/spec-main/guest-window-manager-spec.ts index 20e640f02a6..01f7914d86b 100644 --- a/spec-main/guest-window-manager-spec.ts +++ b/spec-main/guest-window-manager-spec.ts @@ -112,16 +112,21 @@ describe('webContents.setWindowOpenHandler', () => { const { browserWindowOptions } = testConfig[testName]; describe(testName, () => { - beforeEach((done) => { + beforeEach(async () => { browserWindow = new BrowserWindow(browserWindowOptions); - browserWindow.loadURL('about:blank'); - browserWindow.on('ready-to-show', () => { browserWindow.show(); done(); }); + await browserWindow.loadURL('about:blank'); + browserWindow.show(); }); afterEach(closeAllWindows); - it('does not fire window creation events if an override returns action: deny', (done) => { - browserWindow.webContents.setWindowOpenHandler(() => ({ action: 'deny' })); + it('does not fire window creation events if an override returns action: deny', async () => { + const denied = new Promise((resolve) => { + browserWindow.webContents.setWindowOpenHandler(() => { + setTimeout(resolve); + return { action: 'deny' }; + }); + }); browserWindow.webContents.on('new-window', () => { assert.fail('new-window should not to be called with an overridden window.open'); }); @@ -132,9 +137,51 @@ describe('webContents.setWindowOpenHandler', () => { browserWindow.webContents.executeJavaScript("window.open('about:blank') && true"); - setTimeout(() => { - done(); - }, 500); + await denied; + }); + + it('is called when clicking on a target=_blank link', async () => { + const denied = new Promise((resolve) => { + browserWindow.webContents.setWindowOpenHandler(() => { + setTimeout(resolve); + return { action: 'deny' }; + }); + }); + browserWindow.webContents.on('new-window', () => { + assert.fail('new-window should not to 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'); + }); + + await browserWindow.webContents.loadURL('data:text/html,link'); + browserWindow.webContents.sendInputEvent({ type: 'mouseDown', x: 10, y: 10, button: 'left', clickCount: 1 }); + browserWindow.webContents.sendInputEvent({ type: 'mouseUp', x: 10, y: 10, button: 'left', clickCount: 1 }); + + await denied; + }); + + it('is called when shift-clicking on a link', async () => { + const denied = new Promise((resolve) => { + browserWindow.webContents.setWindowOpenHandler(() => { + setTimeout(resolve); + return { action: 'deny' }; + }); + }); + browserWindow.webContents.on('new-window', () => { + assert.fail('new-window should not to 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'); + }); + + await browserWindow.webContents.loadURL('data:text/html,link'); + browserWindow.webContents.sendInputEvent({ type: 'mouseDown', x: 10, y: 10, button: 'left', clickCount: 1, modifiers: ['shift'] }); + browserWindow.webContents.sendInputEvent({ type: 'mouseUp', x: 10, y: 10, button: 'left', clickCount: 1, modifiers: ['shift'] }); + + await denied; }); it('fires handler with correct params', (done) => {