Set URL only for navigatable protocols

Now we will call webview.loadURL and let it handle actions for non-navigatable URLs.  For navigatable ones we go through the normal route.

Fix #1922

Auditors: @diracdeltas

docs/state.md

@@ -177,7 +177,6 @@ WindowStore
     tabId: number, // session tab id not persisted
     closedAtIndex: number, // Index the frame was last closed at, cleared unless the frame is inside of closedFrames
     activeShortcut: string, // Set by the application store when the component should react to a shortcut
-    bookmarklet: string, // JavaScript to load for activeShortcut === 'bookmarklet'
     activeShortcutDetails: object, // Additional parameters for the active shortcut action if any
     adblock: {
       blocked: Array<string>

js/components/frame.js

@@ -292,8 +292,8 @@ class Frame extends ImmutableComponent {
       case 'focus-webview':
         setImmediate(() => this.webview.focus())
         break
-      case 'bookmarklet':
-        this.webview.loadURL(this.props.frame.get('bookmarklet'))
+      case 'load-non-navigatable-url':
+        this.webview.loadURL(this.props.frame.get('activeShortcutDetails'))
         break
     }
     if (activeShortcut) {
@@ -467,30 +467,33 @@ class Frame extends ImmutableComponent {
       this.webview = false
     })
     this.webview.addEventListener('did-fail-load', (e) => {
-      if (e.isMainFrame && isFrameError(e.errorCode)) {
-        // temporary workaround for https://github.com/brave/browser-laptop/issues/1817
-        if (e.validatedURL === aboutUrls.get('about:newtab') ||
-            e.validatedURL === aboutUrls.get('about:blank') ||
-            e.validatedURL === aboutUrls.get('about:certerror') ||
-            e.validatedURL === aboutUrls.get('about:error') ||
-            e.validatedURL === aboutUrls.get('about:safebrowsing')) {
-          // this will just display a blank page for errors
-          // but we don't want to take the user out of the private tab
-          return
-        } else if (isTargetAboutUrl(e.validatedURL)) {
-          // open a new tab for other about urls
-          // and send this tab back to wherever it came from
-          this.goBack()
-          windowActions.newFrame({location: e.validatedURL}, true)
-          return
-        }
+      if (e.isMainFrame) {
+        loadEnd()
+        if (isFrameError(e.errorCode)) {
+          // temporary workaround for https://github.com/brave/browser-laptop/issues/1817
+          if (e.validatedURL === aboutUrls.get('about:newtab') ||
+              e.validatedURL === aboutUrls.get('about:blank') ||
+              e.validatedURL === aboutUrls.get('about:certerror') ||
+              e.validatedURL === aboutUrls.get('about:error') ||
+              e.validatedURL === aboutUrls.get('about:safebrowsing')) {
+            // this will just display a blank page for errors
+            // but we don't want to take the user out of the private tab
+            return
+          } else if (isTargetAboutUrl(e.validatedURL)) {
+            // open a new tab for other about urls
+            // and send this tab back to wherever it came from
+            this.goBack()
+            windowActions.newFrame({location: e.validatedURL}, true)
+            return
+          }
 
-        windowActions.setFrameError(this.props.frame, {
-          event_type: 'did-fail-load',
-          errorCode: e.errorCode,
-          url: e.validatedURL
-        })
-        windowActions.loadUrl(this.props.frame, 'about:error')
+          windowActions.setFrameError(this.props.frame, {
+            event_type: 'did-fail-load',
+            errorCode: e.errorCode,
+            url: e.validatedURL
+          })
+          windowActions.loadUrl(this.props.frame, 'about:error')
+        }
       }
     })
     this.webview.addEventListener('did-finish-load', () => {

js/stores/windowStore.js

@@ -16,6 +16,7 @@ const debounce = require('../lib/debounce.js')
 const getSetting = require('../settings').getSetting
 const importFromHTML = require('../lib/importer').importFromHTML
 const UrlUtil = require('../lib/urlutil')
+const urlParse = require('url').parse
 
 const { l10nErrorText } = require('../lib/errorUtil')
 const { aboutUrls, getSourceAboutUrl, isIntermediateAboutPage } = require('../lib/appUrlUtil')
@@ -130,11 +131,18 @@ const doAction = (action) => {
     case WindowConstants.WINDOW_SET_URL:
       const frame = FrameStateUtil.getFrameByKey(windowState, action.key)
       const currentLocation = frame.get('location')
-      if (action.location.substring(0, 11).toLowerCase() === 'javascript:') {
-        if (currentLocation.substring(0, 6).toLowerCase() !== 'about:') {
+      const parsedUrl = urlParse(action.location)
+
+      const navigatableTypes = ['http:', 'https:', 'about:', 'chrome:',
+        'chrome-extension:', 'file:', 'view-source:', 'ftp:', 'data:']
+
+      // For types that are not navigatable, just do a loadUrl on them
+      if (!navigatableTypes.includes(parsedUrl.protocol)) {
+        if (parsedUrl.protocol !== 'javascript:' ||
+            currentLocation.substring(0, 6).toLowerCase() !== 'about:') {
           windowState = windowState.mergeIn(frameStatePath(action.key), {
-            activeShortcut: 'bookmarklet',
-            bookmarklet: action.location
+            activeShortcut: 'load-non-navigatable-url',
+            activeShortcutDetails: action.location
           })
         }
         updateNavBarInput(frame.get('location'), frameStatePath(action.key))