Fix sites that use about:blank redirect technique (#623)

* Fix sites that use about:blank redirect technique

When you open some links with Google Calendar, instead of opening the link directly, the site opens a new window with the location 'about:blank' and then sets the new window's document content to include a refresh directive to open the actual link. This change causes the 'about:blank' links to be handled internally so that the technique can actually work.

* Hide 'about:blank' windows while they perform the redirect

After a new window is created for an 'about:blank' link, the redirect occurs, which causes another window to be opened. This change causes the 'about:blank' to be created hidden, and then closed entirely once the redirect finishes.

* Add tests for `linkIsInternal`

* Refactor onNewWindow to make it testable

app/src/components/mainWindow/mainWindow.js

@@ -2,6 +2,7 @@ import fs from 'fs';
 import path from 'path';
 import { BrowserWindow, shell, ipcMain, dialog } from 'electron';
 import windowStateKeeper from 'electron-window-state';
+import mainWindowHelpers from './mainWindowHelpers';
 import helpers from './../../helpers/helpers';
 import createMenu from './../menu/menu';
 import initContextMenu from './../contextMenu/contextMenu';
@@ -15,6 +16,8 @@ const {
   nativeTabsSupported,
 } = helpers;
 
+const { onNewWindowHelper } = mainWindowHelpers;
+
 const ZOOM_INTERVAL = 0.1;
 
 function maybeHideWindow(window, event, fastQuit, tray) {
@@ -216,6 +219,19 @@ function createMainWindow(inpOptions, onAppQuit, setDockBadge) {
     return undefined;
   };
 
+  const createAboutBlankWindow = () => {
+    const window = createNewWindow('about:blank');
+    window.hide();
+    window.webContents.once('did-stop-loading', () => {
+      if (window.webContents.getURL() === 'about:blank') {
+        window.close();
+      } else {
+        window.show();
+      }
+    });
+    return window;
+  };
+
   const onNewWindow = (event, urlToGo, _, disposition) => {
     const preventDefault = (newGuest) => {
       event.preventDefault();
@@ -224,18 +240,17 @@ function createMainWindow(inpOptions, onAppQuit, setDockBadge) {
         event.newGuest = newGuest;
       }
     };
-    if (!linkIsInternal(options.targetUrl, urlToGo, options.internalUrls)) {
+    onNewWindowHelper(
-      shell.openExternal(urlToGo);
+      urlToGo,
-      preventDefault();
+      disposition,
-    } else if (nativeTabsSupported()) {
+      options.targetUrl,
-      if (disposition === 'background-tab') {
+      options.internalUrls,
-        const newTab = createNewTab(urlToGo, false);
+      preventDefault,
-        preventDefault(newTab);
+      shell.openExternal,
-      } else if (disposition === 'foreground-tab') {
+      createAboutBlankWindow,
-        const newTab = createNewTab(urlToGo, true);
+      nativeTabsSupported,
-        preventDefault(newTab);
+      createNewTab,
-      }
+    );
-    }
   };
 
   const sendParamsOnDidFinishLoad = (window) => {

app/src/components/mainWindow/mainWindowHelpers.js

@@ -0,0 +1,33 @@
+import helpers from './../../helpers/helpers';
+
+const { linkIsInternal } = helpers;
+
+function onNewWindowHelper(
+  urlToGo,
+  disposition,
+  targetUrl,
+  internalUrls,
+  preventDefault,
+  openExternal,
+  createAboutBlankWindow,
+  nativeTabsSupported,
+  createNewTab,
+) {
+  if (!linkIsInternal(targetUrl, urlToGo, internalUrls)) {
+    openExternal(urlToGo);
+    preventDefault();
+  } else if (urlToGo === 'about:blank') {
+    const newWindow = createAboutBlankWindow();
+    preventDefault(newWindow);
+  } else if (nativeTabsSupported()) {
+    if (disposition === 'background-tab') {
+      const newTab = createNewTab(urlToGo, false);
+      preventDefault(newTab);
+    } else if (disposition === 'foreground-tab') {
+      const newTab = createNewTab(urlToGo, true);
+      preventDefault(newTab);
+    }
+  }
+}
+
+export default { onNewWindowHelper };

app/src/components/mainWindow/mainWindowHelpers.test.js

@@ -0,0 +1,168 @@
+import mainWindowHelpers from './mainWindowHelpers';
+
+const { onNewWindowHelper } = mainWindowHelpers;
+
+const originalUrl = 'https://medium.com/';
+const internalUrl = 'https://medium.com/topics/technology';
+const externalUrl = 'https://www.wikipedia.org/wiki/Electron';
+const foregroundDisposition = 'foreground-tab';
+const backgroundDisposition = 'background-tab';
+
+const nativeTabsSupported = () => true;
+const nativeTabsNotSupported = () => false;
+
+test('internal urls should not be handled', () => {
+  const preventDefault = jest.fn();
+  const openExternal = jest.fn();
+  const createAboutBlankWindow = jest.fn();
+  const createNewTab = jest.fn();
+  onNewWindowHelper(
+    internalUrl,
+    undefined,
+    originalUrl,
+    undefined,
+    preventDefault,
+    openExternal,
+    createAboutBlankWindow,
+    nativeTabsNotSupported,
+    createNewTab,
+  );
+  expect(openExternal.mock.calls.length).toBe(0);
+  expect(createAboutBlankWindow.mock.calls.length).toBe(0);
+  expect(createNewTab.mock.calls.length).toBe(0);
+  expect(preventDefault.mock.calls.length).toBe(0);
+});
+
+test('external urls should be opened externally', () => {
+  const openExternal = jest.fn();
+  const createAboutBlankWindow = jest.fn();
+  const createNewTab = jest.fn();
+  const preventDefault = jest.fn();
+  onNewWindowHelper(
+    externalUrl,
+    undefined,
+    originalUrl,
+    undefined,
+    preventDefault,
+    openExternal,
+    createAboutBlankWindow,
+    nativeTabsNotSupported,
+    createNewTab,
+  );
+  expect(openExternal.mock.calls.length).toBe(1);
+  expect(createAboutBlankWindow.mock.calls.length).toBe(0);
+  expect(createNewTab.mock.calls.length).toBe(0);
+  expect(preventDefault.mock.calls.length).toBe(1);
+});
+
+test('tab disposition should be ignored if tabs are not enabled', () => {
+  const preventDefault = jest.fn();
+  const openExternal = jest.fn();
+  const createAboutBlankWindow = jest.fn();
+  const createNewTab = jest.fn();
+  onNewWindowHelper(
+    internalUrl,
+    foregroundDisposition,
+    originalUrl,
+    undefined,
+    preventDefault,
+    openExternal,
+    createAboutBlankWindow,
+    nativeTabsNotSupported,
+    createNewTab,
+  );
+  expect(openExternal.mock.calls.length).toBe(0);
+  expect(createAboutBlankWindow.mock.calls.length).toBe(0);
+  expect(createNewTab.mock.calls.length).toBe(0);
+  expect(preventDefault.mock.calls.length).toBe(0);
+});
+
+test('tab disposition should be ignored if url is external', () => {
+  const openExternal = jest.fn();
+  const createAboutBlankWindow = jest.fn();
+  const createNewTab = jest.fn();
+  const preventDefault = jest.fn();
+  onNewWindowHelper(
+    externalUrl,
+    foregroundDisposition,
+    originalUrl,
+    undefined,
+    preventDefault,
+    openExternal,
+    createAboutBlankWindow,
+    nativeTabsSupported,
+    createNewTab,
+  );
+  expect(openExternal.mock.calls.length).toBe(1);
+  expect(createAboutBlankWindow.mock.calls.length).toBe(0);
+  expect(createNewTab.mock.calls.length).toBe(0);
+  expect(preventDefault.mock.calls.length).toBe(1);
+});
+
+test('foreground tabs with internal urls should be opened in the foreground', () => {
+  const openExternal = jest.fn();
+  const createAboutBlankWindow = jest.fn();
+  const createNewTab = jest.fn();
+  const preventDefault = jest.fn();
+  onNewWindowHelper(
+    internalUrl,
+    foregroundDisposition,
+    originalUrl,
+    undefined,
+    preventDefault,
+    openExternal,
+    createAboutBlankWindow,
+    nativeTabsSupported,
+    createNewTab,
+  );
+  expect(openExternal.mock.calls.length).toBe(0);
+  expect(createAboutBlankWindow.mock.calls.length).toBe(0);
+  expect(createNewTab.mock.calls.length).toBe(1);
+  expect(createNewTab.mock.calls[0][1]).toBe(true);
+  expect(preventDefault.mock.calls.length).toBe(1);
+});
+
+test('background tabs with internal urls should be opened in background tabs', () => {
+  const openExternal = jest.fn();
+  const createAboutBlankWindow = jest.fn();
+  const createNewTab = jest.fn();
+  const preventDefault = jest.fn();
+  onNewWindowHelper(
+    internalUrl,
+    backgroundDisposition,
+    originalUrl,
+    undefined,
+    preventDefault,
+    openExternal,
+    createAboutBlankWindow,
+    nativeTabsSupported,
+    createNewTab,
+  );
+  expect(openExternal.mock.calls.length).toBe(0);
+  expect(createAboutBlankWindow.mock.calls.length).toBe(0);
+  expect(createNewTab.mock.calls.length).toBe(1);
+  expect(createNewTab.mock.calls[0][1]).toBe(false);
+  expect(preventDefault.mock.calls.length).toBe(1);
+});
+
+test('about:blank urls should be handled', () => {
+  const preventDefault = jest.fn();
+  const openExternal = jest.fn();
+  const createAboutBlankWindow = jest.fn();
+  const createNewTab = jest.fn();
+  onNewWindowHelper(
+    'about:blank',
+    undefined,
+    originalUrl,
+    undefined,
+    preventDefault,
+    openExternal,
+    createAboutBlankWindow,
+    nativeTabsNotSupported,
+    createNewTab,
+  );
+  expect(openExternal.mock.calls.length).toBe(0);
+  expect(createAboutBlankWindow.mock.calls.length).toBe(1);
+  expect(createNewTab.mock.calls.length).toBe(0);
+  expect(preventDefault.mock.calls.length).toBe(1);
+});

app/src/helpers/helpers.js

@@ -19,6 +19,10 @@ function isWindows() {
 }
 
 function linkIsInternal(currentUrl, newUrl, internalUrlRegex) {
+  if (newUrl === 'about:blank') {
+    return true;
+  }
+
   if (internalUrlRegex) {
     const regex = RegExp(internalUrlRegex);
     return regex.test(newUrl);

app/src/helpers/helpers.test.js

@@ -0,0 +1,30 @@
+import helpers from './helpers';
+
+const { linkIsInternal } = helpers;
+
+const internalUrl = 'https://medium.com/';
+const internalUrlSubPath = 'topic/technology';
+const externalUrl = 'https://www.wikipedia.org/wiki/Electron';
+const wildcardRegex = /.*/;
+
+test('the original url should be internal', () => {
+  expect(linkIsInternal(internalUrl, internalUrl, undefined)).toEqual(true);
+});
+
+test('sub-paths of the original url should be internal', () => {
+  expect(
+    linkIsInternal(internalUrl, internalUrl + internalUrlSubPath, undefined),
+  ).toEqual(true);
+});
+
+test("'about:blank' should be internal", () => {
+  expect(linkIsInternal(internalUrl, 'about:blank', undefined)).toEqual(true);
+});
+
+test('urls from different sites should not be internal', () => {
+  expect(linkIsInternal(internalUrl, externalUrl, undefined)).toEqual(false);
+});
+
+test('all urls should be internal with wildcard regex', () => {
+  expect(linkIsInternal(internalUrl, externalUrl, wildcardRegex)).toEqual(true);
+});