Command+L when in url bar with URL bar suggestion should select full URL

There was also a lot of other fixes which moves things out of the
component and into the store.

Note that there are no state functions in use yet for the window
renderer so I didn't create them yet as part of this task and just
used immutableJS directly.

I also fixed some intermittents with this fix.

Fix #9914

Auditors: @bsclifton

Test Plan:
1. Fully test url bar and suggestions thoroughly
2. Type in some chars that lead to a autocomplete selection, press
command+L and make sure it selects all text.

app/renderer/components/navigation/urlBar.js

@@ -29,7 +29,6 @@ const siteSettingsState = require('../../../common/state/siteSettingsState')
 
 // Utils
 const cx = require('../../../../js/lib/classSet')
-const debounce = require('../../../../js/lib/debounce')
 const {getSetting} = require('../../../../js/settings')
 const contextMenus = require('../../../../js/contextMenus')
 const {eventElHasAncestorWithClasses, isForSecondaryAction} = require('../../../../js/lib/eventUtil')
@@ -55,12 +54,6 @@ class UrlBar extends React.Component {
     this.onKeyPress = this.onKeyPress.bind(this)
     this.onClick = this.onClick.bind(this)
     this.onContextMenu = this.onContextMenu.bind(this)
-    this.showAutocompleteResult = debounce(() => {
-      if (!this.urlInput) {
-        return
-      }
-      this.updateAutocomplete(this.lastVal)
-    }, 10)
   }
 
   maybeUrlBarTextChanged (value) {
@@ -111,7 +104,7 @@ class UrlBar extends React.Component {
         let location = this.urlInput ? this.getValue() : this.props.urlbarLocation
 
         if (location === null || location.length === 0) {
-          windowActions.setUrlBarSelected(true)
+          windowActions.urlBarSelected(true)
         } else {
           // Filter javascript URLs to prevent self-XSS
           location = location.replace(/^(\s*javascript:)+/i, '')
@@ -298,12 +291,13 @@ class UrlBar extends React.Component {
         return
     }
     if (this.props.isSelected) {
-      windowActions.setUrlBarSelected(false)
+      windowActions.urlBarSelected(false)
     }
     this.maybeUrlBarTextChanged(this.lastVal)
   }
 
   select () {
+    windowActions.urlBarSelected(true)
     setImmediate(() => {
       if (this.urlInput) {
         this.urlInput.select()
@@ -356,17 +350,18 @@ class UrlBar extends React.Component {
       if (!(prevProps.frameLocation === 'about:blank' && this.props.frameLocation === 'about:newtab' && this.props.urlbarLocation !== 'about:blank')) {
         this.setValue(this.props.displayURL)
       }
-    } else if (this.props.isActive &&
-              this.props.urlbarLocationSuffix !== this.lastSuffix) {
-      this.showAutocompleteResult()
+    } else if (this.props.autocompleteEnabled &&
+        this.props.normalizedSuggestion !== prevProps.normalizedSuggestion) {
+      this.updateAutocomplete(this.lastVal)
+    // This case handles when entering urlmode from tilemode
     } else if ((this.props.titleMode !== prevProps.titleMode) ||
-        (!this.props.isActive && !this.props.isFocused)) {
+         (!this.props.isActive && !this.props.isFocused)) {
       this.setValue(this.props.urlbarLocation)
     }
 
     if (this.props.isSelected && !prevProps.isSelected) {
       this.select()
-      windowActions.setUrlBarSelected(false)
+      windowActions.urlBarSelected(false)
     }
 
     if (this.props.noScriptIsVisible && !this.props.showNoScriptInfo) {

app/renderer/reducers/urlBarReducer.js

@@ -97,6 +97,22 @@ const updateUrlSuffix = (state, suggestionList, framePath) => {
   return state
 }
 
+/**
+ * Accepts whatever suffix is present as part of the user input
+ */
+const acceptUrlSuffix = (state, framePath) => {
+  const lastSuffix = state.getIn(framePath.concat(['navbar', 'urlbar', 'suggestions', 'urlSuffix']))
+  if (lastSuffix) {
+    const input = state.getIn(framePath.concat(['navbar', 'urlbar', 'location']))
+    state = setNavBarUserInput(state, input + lastSuffix, framePath)
+  }
+  state = state.mergeIn(framePath.concat(['navbar', 'urlbar', 'suggestions']), {
+    selectedIndex: null,
+    suggestionList: null
+  })
+  return state
+}
+
 const updateNavBarInput = (state, loc, framePath) => {
   if (framePath === undefined) {
     framePath = activeFrameStatePath(state)
@@ -131,15 +147,18 @@ const setActive = (state, isActive) => {
 }
 
 const setUrlBarSelected = (state, selected) => {
-  const urlBarPath = activeFrameStatePath(state).concat(['navbar', 'urlbar'])
+  const framePath = activeFrameStatePath(state)
+  const urlBarPath = framePath.concat(['navbar', 'urlbar'])
+  state = acceptUrlSuffix(state, framePath)
   state = state.mergeIn(urlBarPath, {
     selected: selected
   })
   // selection implies focus
   if (selected) {
     state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'focused']), true)
   }
-
+  state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'autocompleteEnabled']), false)
+  state = setRenderUrlBarSuggestions(state, false, framePath)
   return state
 }
 
@@ -152,9 +171,8 @@ const urlBarReducer = (state, action) => {
     const displayURL = navigationState.getIn(['visibleEntry', 'virtualURL'])
     const frame = getFrameByTabId(state, tabId)
     if (frame) {
-      state = updateNavBarInput(state, displayURL, frameStatePath(state, frame.get('key')))
+      state = setNavBarUserInput(state, displayURL, frameStatePath(state, frame.get('key')))
       state = state.setIn(frameStatePath(state, frame.get('key')).concat(['navbar', 'urlbar', 'suggestions', 'shouldRender']), false)
-      state = updateSearchEngineInfoFromInput(state, frame)
     }
     return state
   }
@@ -185,6 +203,7 @@ const urlBarReducer = (state, action) => {
         state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']), action.selectedIndex)
       }
       state = setUrlSuggestions(state, action.suggestionList)
+      state = updateUrlSuffix(state, action.suggestionList)
       break
     case windowConstants.WINDOW_URL_BAR_ON_FOCUS:
       state = navigationBarState.setFocused(state, tabId, true)
@@ -201,7 +220,7 @@ const urlBarReducer = (state, action) => {
       state = navigationBarState.setFocused(state, tabId, false)
       state = setActive(state, false)
       break
-    case windowConstants.WINDOW_SET_URL_BAR_SELECTED:
+    case windowConstants.WINDOW_URL_BAR_SELECTED:
       state = setUrlBarSelected(state, action.selected)
       break
     case windowConstants.WINDOW_SET_FINDBAR_SHOWN:

docs/windowActions.md

@@ -336,13 +336,9 @@ This is sometimes only temporarily disabled, e.g. a user is pressing backspace.
 
 
 
-### setUrlBarSelected(isSelected) 
+### urlBarSelected() 
 
-Marks the URL bar text as selected or not
-
-**Parameters**
-
-**isSelected**: `boolean`, Whether or not the URL bar text input should be selected
+Indicates the URLbar has been selected
 
 
 

js/actions/windowActions.js

@@ -411,14 +411,11 @@ const windowActions = {
   },
 
   /**
-   * Marks the URL bar text as selected or not
-   *
-   * @param {boolean} isSelected - Whether or not the URL bar text input should be selected
+   * Indicates the URLbar has been selected
    */
-  setUrlBarSelected: function (selected) {
+  urlBarSelected: function (selected) {
     dispatch({
-      actionType: windowConstants.WINDOW_SET_URL_BAR_SELECTED,
-      selected
+      actionType: windowConstants.WINDOW_URL_BAR_SELECTED
     })
   },
 

js/constants/windowConstants.js

@@ -40,7 +40,7 @@ const windowConstants = {
   WINDOW_URL_BAR_ON_BLUR: _,
   WINDOW_URL_BAR_ON_FOCUS: _,
   WINDOW_TAB_ON_FOCUS: _,
-  WINDOW_SET_URL_BAR_SELECTED: _,
+  WINDOW_URL_BAR_SELECTED: _,
   WINDOW_SET_FIND_DETAIL: _,
   WINDOW_SET_BOOKMARK_DETAIL: _, // If set, also indicates that add/edit is shown
   WINDOW_SET_CONTEXT_MENU_DETAIL: _, // If set, also indicates that the context menu is shown

test/navbar-components/navigationBarTest.js

@@ -98,7 +98,7 @@ describe('navigationBar tests', function () {
           .newTab({ url: page1Url })
           .waitUntil(function () {
             return this.getWindowState().then((val) => {
-              return val.value.frames.length === 2
+              return val.value.frames.length === 3
             })
           })
           .waitForElementFocus(activeWebview)
@@ -214,6 +214,7 @@ describe('navigationBar tests', function () {
       it('remains cleared when onChange is fired but not onKeyUp', function * () {
         yield this.app.client
           .windowByUrl(Brave.browserWindowUrl)
+          .activateURLMode()
           .waitForVisible(urlInput)
           .setValue(urlInput, '')
           .moveToObject(activeWebview)
@@ -610,7 +611,7 @@ describe('navigationBar tests', function () {
         })
         .windowByUrl(Brave.browserWindowUrl)
         .waitForExist(urlbarIcon + '.fa-lock')
-        .click(urlbarIcon)
+        .click(urlbarIcon + '.fa-lock')
         .waitForVisible('[data-test-id="secureConnection"]')
         .waitForVisible('[data-test-id="runInsecureContentWarning"]')
         .waitForVisible(dismissAllowRunInsecureContentButton)
@@ -913,10 +914,9 @@ describe('navigationBar tests', function () {
         yield setup(this.app.client)
         yield this.app.client
           .waitForExist(urlInput)
-        yield this.app.client
           .keys(this.page1)
-        yield this.app.client
           .keys(Brave.keys.ENTER)
+          .activateURLMode()
       })
 
       it('shows search icon when input is empty', function * () {
@@ -1002,6 +1002,7 @@ describe('navigationBar tests', function () {
           .keys(Brave.keys.ENTER)
           .tabByUrl('about:about')
           .windowByUrl(Brave.browserWindowUrl)
+          .activateURLMode()
           .waitForInputText(urlInput, 'about:about')
           .waitForExist('.urlbarIcon.fa-list')
       })
@@ -1223,20 +1224,25 @@ describe('navigationBar tests', function () {
   })
 
   describe('shortcut-focus-url', function () {
-    Brave.beforeAll(this)
+    Brave.beforeEach(this)
 
-    before(function * () {
+    beforeEach(function * () {
       yield setup(this.app.client)
       yield blur(this.app.client)
       yield this.app.client.ipcSend('shortcut-focus-url')
+      yield this.app.client.waitForElementFocus(urlInput)
     })
 
     it('has an empty url with placeholder', function * () {
       yield defaultUrlInputValue(this.app.client)
     })
 
-    it('has focus', function * () {
-      yield this.app.client.waitForElementFocus(urlInput)
+    it('selects full url when autocompleting with partial selection', function * () {
+      yield this.app.client
+        .keys('about:bra')
+      yield selectsText(this.app.client, 've')
+      yield this.app.client.ipcSend('shortcut-focus-url')
+      yield selectsText(this.app.client, 'about:brave')
     })
   })
 

test/navbar-components/urlBarTest.js

@@ -5,6 +5,7 @@ const Brave = require('../lib/brave')
 const {urlInput, urlBarSuggestions, urlbarIcon, reloadButton} = require('../lib/selectors')
 const searchProviders = require('../../js/data/searchProviders')
 const config = require('../../js/constants/config')
+const settings = require('../../js/constants/settings')
 
 describe('urlBar tests', function () {
   function * setup (client) {
@@ -708,6 +709,7 @@ describe('urlBar tests', function () {
       this.page1Url = Brave.server.url('page1.html')
       yield setup(this.app.client)
       yield this.app.client
+        .changeSetting(settings.DISABLE_TITLE_MODE, false)
         .waitForExist(urlInput)
         .waitForElementFocus(urlInput)
         .tabByIndex(0)
@@ -767,7 +769,7 @@ describe('urlBar tests', function () {
 
     it('changes only the selection', function * () {
       yield this.app.client
-        .setInputText(urlInput, 'git')
+        .keys('git')
         .waitForSelectedText('hub.com')
         // Select next suggestion
         .keys(Brave.keys.DOWN)