Support forking nav from navigation buttons

Fix #2753

Audiors: @bridiver

js/components/frame.js

@@ -25,7 +25,7 @@ const debounce = require('../lib/debounce.js')
 const getSetting = require('../settings').getSetting
 const settings = require('../constants/settings')
 const FindBar = require('./findbar.js')
-const { aboutUrls, isSourceAboutUrl, isTargetAboutUrl, getTargetAboutUrl, getBaseUrl } = require('../lib/appUrlUtil')
+const { aboutUrls, isSourceAboutUrl, isTargetAboutUrl, getTargetAboutUrl, getBaseUrl, isNavigatableAboutPage } = require('../lib/appUrlUtil')
 const { isFrameError } = require('../lib/errorUtil')
 const locale = require('../l10n')
 const appConfig = require('../constants/appConfig')
@@ -307,6 +307,24 @@ class Frame extends ImmutableComponent {
     }
   }
 
+  clone (args) {
+    if (!isNavigatableAboutPage(getBaseUrl(this.props.frame.get('location')))) {
+      return
+    }
+    const newGuest = this.webview.clone()
+    const newGuestInstanceId = newGuest.getWebPreferences().guestInstanceId
+    let cloneAction
+    if (args && args.get('back')) {
+      cloneAction = newGuest.goBack
+    } else if (args && args.get('forward')) {
+      cloneAction = () => newGuest.goForward
+    }
+    if (cloneAction) {
+      newGuest.once('did-attach', cloneAction.bind(newGuest))
+    }
+    windowActions.cloneFrame(this.props.frame, newGuestInstanceId, args && args.get('openInForeground'))
+  }
+
   handleShortcut () {
     const activeShortcut = this.props.frame.get('activeShortcut')
     const activeShortcutDetails = this.props.frame.get('activeShortcutDetails')
@@ -336,12 +354,7 @@ class Frame extends ImmutableComponent {
         this.webview.reloadIgnoringCache()
         break
       case 'clone':
-        if (this.isAboutPage()) {
-          break
-        }
-        const newGuest = this.webview.clone()
-        const newGuestInstanceId = newGuest.getWebPreferences().guestInstanceId
-        windowActions.cloneFrame(this.props.frame, newGuestInstanceId)
+        this.clone(activeShortcutDetails)
         break
       case 'explicitLoadURL':
         this.webview.loadURL(location)

js/components/longPressButton.js

@@ -49,7 +49,7 @@ class LongPressButton extends ImmutableComponent {
       this.isLocked = false
       return
     }
-    this.props.onClick()
+    this.props.onClick(e)
   }
 
   render () {

js/components/main.js

@@ -52,7 +52,7 @@ const FrameStateUtil = require('../state/frameStateUtil')
 // Util
 const cx = require('../lib/classSet.js')
 const eventUtil = require('../lib/eventUtil')
-const { isIntermediateAboutPage, getBaseUrl } = require('../lib/appUrlUtil')
+const { isIntermediateAboutPage, getBaseUrl, isNavigatableAboutPage } = require('../lib/appUrlUtil')
 const siteSettings = require('../state/siteSettings')
 const urlParse = require('url').parse
 const debounce = require('../lib/debounce.js')
@@ -457,16 +457,32 @@ class Main extends ImmutableComponent {
     return location
   }
 
-  onBack () {
-    this.activeFrame.goBack()
+  onNav (e, navCheckProp, navType, navAction) {
+    const activeFrame = FrameStateUtil.getActiveFrame(this.props.windowState)
+    const isNavigatable = isNavigatableAboutPage(getBaseUrl(activeFrame.get('location')))
+    if (e && eventUtil.isForSecondaryAction(e) && isNavigatable) {
+      if (activeFrame && activeFrame.get(navCheckProp)) {
+        const win = remote.getCurrentWindow()
+        win.webContents.send(messages.SHORTCUT_ACTIVE_FRAME_CLONE, {
+          [navType]: true,
+          openInForeground: !!e.shiftKey
+        })
+      }
+    } else {
+      navAction.call(this.activeFrame)
+    }
   }
 
-  onBackLongPress (rect) {
-    contextMenus.onBackButtonHistoryMenu(this.activeFrame, this.activeFrame.getHistory(this.props.appState), rect)
+  onBack (e) {
+    this.onNav(e, 'canGoBack', 'back', this.activeFrame.goBack)
+  }
+
+  onForward (e) {
+    this.onNav(e, 'canGoForward', 'forward', this.activeFrame.goForward)
   }
 
-  onForward () {
-    this.activeFrame.goForward()
+  onBackLongPress (rect) {
+    contextMenus.onBackButtonHistoryMenu(this.activeFrame, this.activeFrame.getHistory(this.props.appState), rect)
   }
 
   onForwardLongPress (rect) {

js/components/navigationBar.js

@@ -50,7 +50,7 @@ class NavigationBar extends ImmutableComponent {
 
   onReload (e) {
     if (eventUtil.isForSecondaryAction(e)) {
-      windowActions.cloneFrame(this.props.activeFrame)
+      ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_CLONE, {}, { openInForeground: !!e.shiftKey })
     } else {
       ipc.emit(messages.SHORTCUT_ACTIVE_FRAME_RELOAD)
     }

js/contextMenus.js

@@ -368,7 +368,9 @@ function tabTemplateInit (frameProps) {
       label: locale.translation('clone'),
       click: (item, focusedWindow) => {
         if (focusedWindow) {
-          focusedWindow.webContents.send(messages.SHORTCUT_ACTIVE_FRAME_CLONE)
+          focusedWindow.webContents.send(messages.SHORTCUT_ACTIVE_FRAME_CLONE, {
+            openInForeground: true
+          })
         }
       }
     })

js/stores/windowStore.js

@@ -653,10 +653,10 @@ ipc.on(messages.IMPORT_BOOKMARKS, () => {
 const frameShortcuts = ['stop', 'reload', 'zoom-in', 'zoom-out', 'zoom-reset', 'toggle-dev-tools', 'clean-reload', 'view-source', 'mute', 'save', 'print', 'show-findbar', 'copy', 'find-next', 'find-prev', 'clone']
 frameShortcuts.forEach((shortcut) => {
   // Listen for actions on the active frame
-  ipc.on(`shortcut-active-frame-${shortcut}`, () => {
+  ipc.on(`shortcut-active-frame-${shortcut}`, (e, args) => {
     windowState = windowState.mergeIn(activeFrameStatePath(), {
       activeShortcut: shortcut,
-      activeShortcutDetails: null
+      activeShortcutDetails: args
     })
     emitChanges()
   })

test/components/frameTest.js

@@ -1,7 +1,7 @@
 /* global describe, it, before */
 
 const Brave = require('../lib/brave')
-const { activeWebview, findBarInput, findBarMatches, findBarNextButton, urlInput, titleBar } = require('../lib/selectors')
+const { activeWebview, findBarInput, findBarMatches, findBarNextButton, urlInput, titleBar, backButton, forwardButton } = require('../lib/selectors')
 const messages = require('../../js/constants/messages')
 const assert = require('assert')
 
@@ -170,15 +170,47 @@ describe('clone tab', function () {
     })
 
     it('preserves the history', function * () {
-      let backButton = '.backforward .back'
       yield this.app.client
         .windowByUrl(Brave.browserWindowUrl)
         .waitForExist(this.webview2 + '[src="' + this.url2 + '"]')
         .waitForExist(backButton + ':not([disabled])')
+        .waitForExist(forwardButton + '[disabled]')
         .click(backButton)
         .waitForExist(this.webview2 + '[src="' + this.url1 + '"]')
     })
   })
+
+  describe('back clone', function () {
+    Brave.beforeAll(this)
+
+    before(function * () {
+      this.url1 = Brave.server.url('page1.html')
+      this.url2 = Brave.server.url('page2.html')
+      this.webview1 = '.frameWrapper:nth-child(1) webview'
+      this.webview2 = '.frameWrapper.isActive:nth-child(2) webview'
+
+      yield setup(this.app.client)
+      yield this.app.client
+        .tabByIndex(0)
+        // add some history
+        .loadUrl(this.url1)
+        .loadUrl(this.url2)
+        .windowByUrl(Brave.browserWindowUrl)
+        .ipcSend(messages.SHORTCUT_ACTIVE_FRAME_CLONE, { back: true })
+    })
+
+    it('preserves proper navigation', function * () {
+      yield this.app.client
+        .windowByUrl(Brave.browserWindowUrl)
+        .waitForExist(this.webview2 + '[src="' + this.url1 + '"]')
+        .waitForExist(backButton + ':not([disabled])')
+        .waitForExist(forwardButton + ':not([disabled])')
+        .click(forwardButton)
+        .waitForExist(this.webview2 + '[src="' + this.url2 + '"]')
+        .waitForExist(backButton + ':not([disabled])')
+        .waitForExist(forwardButton + '[disabled]')
+    })
+  })
 })
 
 describe('view source', function () {

test/lib/selectors.js

@@ -38,5 +38,7 @@ module.exports = {
   fpStat: '.braveryStat.fpStat',
   fpSwitch: '.fingerprintingProtectionSwitch .switchMiddle',
   noScriptSwitch: '.noScriptSwitch .switchMiddle',
-  noScriptNavButton: '#navigator .noScript'
+  noScriptNavButton: '#navigator .noScript',
+  backButton: '.backforward .back',
+  forwardButton: '.backforward .forward'
 }