From d3706d6b6d80d55581e579df7778ddb3dae5b026 Mon Sep 17 00:00:00 2001 From: Vishwas Shashidhar Date: Fri, 6 Oct 2017 15:58:26 +0530 Subject: [PATCH 1/5] electron-135: fixes the raised issue --- js/windowMgr.js | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/js/windowMgr.js b/js/windowMgr.js index e591e36e..228d28a4 100644 --- a/js/windowMgr.js +++ b/js/windowMgr.js @@ -250,17 +250,7 @@ function doCreateMainWindow(initialUrl, initialBounds) { webContents.send('downloadCompleted', data); } }); - }); - - // bug in electron is preventing this from working in sandboxed evt... - // https://github.com/electron/electron/issues/8841 - mainWindow.webContents.on('will-navigate', function(event, willNavUrl) { - if (!sandboxed) { - return; - } - event.preventDefault(); - openUrlInDefaultBrower(willNavUrl); - }); + }); // open external links in default browser - a tag with href='_blank' or window.open mainWindow.webContents.on('new-window', function (event, newWinUrl, @@ -365,6 +355,11 @@ function doCreateMainWindow(initialUrl, initialBounds) { }); }); + browserWin.webContents.on('new-window', (childEvent, childWinUrl) => { + childEvent.preventDefault(); + openUrlInDefaultBrower(childWinUrl); + }); + addWindowKey(newWinKey, browserWin); // Method that sends bound changes as soon @@ -381,7 +376,7 @@ function doCreateMainWindow(initialUrl, initialBounds) { }); } else { event.preventDefault(); - openUrlInDefaultBrower(newWinUrl) + openUrlInDefaultBrower(newWinUrl); } }); @@ -524,7 +519,7 @@ function sendChildWinBoundsChange(window) { * @param urlToOpen */ function openUrlInDefaultBrower(urlToOpen) { - if (urlToOpen) { + if (urlToOpen) { electron.shell.openExternal(urlToOpen); } } From 49775e543dfacf98c6ea643fcc459f03059e9aa6 Mon Sep 17 00:00:00 2001 From: Vishwas Shashidhar Date: Fri, 6 Oct 2017 16:00:50 +0530 Subject: [PATCH 2/5] electron-135: added code comments --- js/windowMgr.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/js/windowMgr.js b/js/windowMgr.js index 228d28a4..0d7ce691 100644 --- a/js/windowMgr.js +++ b/js/windowMgr.js @@ -355,6 +355,9 @@ function doCreateMainWindow(initialUrl, initialBounds) { }); }); + // In case we navigate to an external link from inside a pop-out, + // we open that link in an external browser rather than creating + // a new window browserWin.webContents.on('new-window', (childEvent, childWinUrl) => { childEvent.preventDefault(); openUrlInDefaultBrower(childWinUrl); From 9dfbdaa1bb04255f5ada071902926927edeb77da Mon Sep 17 00:00:00 2001 From: Vishwas Shashidhar Date: Fri, 6 Oct 2017 16:07:13 +0530 Subject: [PATCH 3/5] electron-135: fixed typo in the method name --- js/windowMgr.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/windowMgr.js b/js/windowMgr.js index 0d7ce691..19a7bd8d 100644 --- a/js/windowMgr.js +++ b/js/windowMgr.js @@ -360,7 +360,7 @@ function doCreateMainWindow(initialUrl, initialBounds) { // a new window browserWin.webContents.on('new-window', (childEvent, childWinUrl) => { childEvent.preventDefault(); - openUrlInDefaultBrower(childWinUrl); + openUrlInDefaultBrowser(childWinUrl); }); addWindowKey(newWinKey, browserWin); @@ -379,7 +379,7 @@ function doCreateMainWindow(initialUrl, initialBounds) { }); } else { event.preventDefault(); - openUrlInDefaultBrower(newWinUrl); + openUrlInDefaultBrowser(newWinUrl); } }); @@ -521,7 +521,7 @@ function sendChildWinBoundsChange(window) { * Opens an external url in the system's default browser * @param urlToOpen */ -function openUrlInDefaultBrower(urlToOpen) { +function openUrlInDefaultBrowser(urlToOpen) { if (urlToOpen) { electron.shell.openExternal(urlToOpen); } From a2918460019fdbf494cb3ccff4bde100c171c6df Mon Sep 17 00:00:00 2001 From: Vishwas Shashidhar Date: Fri, 6 Oct 2017 23:44:16 +0530 Subject: [PATCH 4/5] electron-135: fixes as per PR comments to avoid leakages --- js/windowMgr.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/js/windowMgr.js b/js/windowMgr.js index 19a7bd8d..c29fd2c1 100644 --- a/js/windowMgr.js +++ b/js/windowMgr.js @@ -347,10 +347,10 @@ function doCreateMainWindow(initialUrl, initialBounds) { electron.dialog.showMessageBox(options, function (index) { if (index === 0) { - mainWindow.reload(); + browserWin.reload(); } else { - mainWindow.close(); + browserWin.close(); } }); }); @@ -363,6 +363,12 @@ function doCreateMainWindow(initialUrl, initialBounds) { openUrlInDefaultBrowser(childWinUrl); }); + // Clear up the browser window once the window is closed + // to avoid leakage + browserWin.on('closed', () => { + browserWin = null; + }); + addWindowKey(newWinKey, browserWin); // Method that sends bound changes as soon From 99dd5d90ff0db4b94fb71dc8735e43b3b8a0c37c Mon Sep 17 00:00:00 2001 From: Vishwas Shashidhar Date: Sun, 8 Oct 2017 19:32:44 +0530 Subject: [PATCH 5/5] electron-135: added logic to clean up event listeners --- js/windowMgr.js | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/js/windowMgr.js b/js/windowMgr.js index c29fd2c1..1be85b4b 100644 --- a/js/windowMgr.js +++ b/js/windowMgr.js @@ -331,13 +331,22 @@ function doCreateMainWindow(initialUrl, initialBounds) { browserWin.winName = frameName; browserWin.setAlwaysOnTop(alwaysOnTop); - browserWin.once('closed', function () { + let handleChildWindowClosed = () => { removeWindowKey(newWinKey); browserWin.removeListener('move', throttledBoundsChange); - browserWin.removeListener('resize', throttledBoundsChange); + browserWin.removeListener('resize', throttledBoundsChange); + }; + + browserWin.once('closed', () => { + handleChildWindowClosed(); }); - browserWin.webContents.on('crashed', function () { + browserWin.on('close', () => { + browserWin.webContents.removeListener('new-window', handleChildNewWindowEvent); + browserWin.webContents.removeListener('crashed', handleChildWindowCrashEvent); + }); + + let handleChildWindowCrashEvent = () => { const options = { type: 'error', title: 'Renderer Process Crashed', @@ -353,21 +362,19 @@ function doCreateMainWindow(initialUrl, initialBounds) { browserWin.close(); } }); - }); + }; + browserWin.webContents.on('crashed', handleChildWindowCrashEvent); + + let handleChildNewWindowEvent = (childEvent, childWinUrl) => { + childEvent.preventDefault(); + openUrlInDefaultBrowser(childWinUrl); + }; + // In case we navigate to an external link from inside a pop-out, // we open that link in an external browser rather than creating // a new window - browserWin.webContents.on('new-window', (childEvent, childWinUrl) => { - childEvent.preventDefault(); - openUrlInDefaultBrowser(childWinUrl); - }); - - // Clear up the browser window once the window is closed - // to avoid leakage - browserWin.on('closed', () => { - browserWin = null; - }); + browserWin.webContents.on('new-window', handleChildNewWindowEvent); addWindowKey(newWinKey, browserWin); @@ -481,7 +488,7 @@ function setIsOnline(status) { } /** - * Tries finding a window we have created with given name. If founds then + * Tries finding a window we have created with given name. If found, then * brings to front and gives focus. * @param {String} windowName Name of target window. Note: main window has * name 'main'.