URL bar autocomplete mouseover does not interfere with typed URL (fixes #3012) (take 2)

js/components/urlBar.js

@@ -24,6 +24,9 @@ var searchProviders = require('../data/searchProviders')
 const searchIconSize = 16
 const UrlUtil = require('../lib/urlutil')
 
+const EventUtil = require('../lib/eventUtil')
+const eventElHasAncestorWithClasses = EventUtil.eventElHasAncestorWithClasses
+
 const { isUrl, isIntermediateAboutPage } = require('../lib/appUrlUtil')
 
 class UrlBar extends ImmutableComponent {
@@ -209,8 +212,11 @@ class UrlBar extends ImmutableComponent {
     windowActions.setUrlBarSelected(false)
     // On blur, a user expects the text shown from the last autocomplete suffix
     // to be auto entered as the new location.
-    this.updateLocationToSuggestion()
     this.clearSearchEngine()
+
+    if (!eventElHasAncestorWithClasses(e, ['urlBarSuggestions', 'urlbarForm'])) {
+      this.updateLocationToSuggestion()
+    }
   }
 
   updateLocationToSuggestion () {
@@ -314,14 +320,6 @@ class UrlBar extends ImmutableComponent {
   }
 
   get locationValue () {
-    // If there's a selected autocomplete entry, we just want to show its location
-    if (this.props.suggestionIndex) {
-      const suggestionLocation = this.props.urlbar.getIn(['suggestions', 'suggestionList', this.props.suggestionIndex - 1]).location
-      if (suggestionLocation) {
-        return suggestionLocation
-      }
-    }
-
     let location = this.props.urlbar.get('location')
     const history = this.props.history
     if (isIntermediateAboutPage(location) && history.size > 0) {

js/components/urlBarSuggestions.js

@@ -121,7 +121,7 @@ class UrlBarSuggestions extends ImmutableComponent {
       }
       items = items.concat(suggestions.map((suggestion, i) => {
         const currentIndex = index + i
-        const selected = this.activeIndex === currentIndex + 1 || currentIndex === 0 && this.props.locationValueSuffix
+        const selected = this.activeIndex === currentIndex + 1 || (!this.activeIndex && currentIndex === 0 && this.props.locationValueSuffix)
         return <li data-index={currentIndex + 1}
           onMouseOver={this.onMouseOver.bind(this)}
           onClick={suggestion.onClick}
@@ -205,6 +205,7 @@ class UrlBarSuggestions extends ImmutableComponent {
       } else {
         windowActions.loadUrl(this.activeFrame, location)
         windowActions.setUrlBarActive(false)
+        windowActions.setUrlBarPreview(null)
         this.blur()
       }
     }

js/lib/eventUtil.js

@@ -3,7 +3,28 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const isDarwin = process.platform === 'darwin'
-module.exports.isForSecondaryAction = (e) =>
+module.exports.isForSecondaryAction = (e) => {
   e.ctrlKey && !isDarwin ||
   e.metaKey && isDarwin ||
   e.button === 1
+}
+
+module.exports.eventElHasAncestorWithClasses = (e, classesToCheck) => {
+  let node = e.target
+
+  while (node) {
+    let classMatch = classesToCheck.map(
+      function (className) {
+        return (node.classList ? node.classList.contains(className) : false)
+      }
+    ).includes(true)
+
+    if (classMatch) {
+      return true
+    }
+
+    node = node.parentNode
+  }
+
+  return false
+}

js/stores/windowStore.js

@@ -59,12 +59,20 @@ const updateNavBarInput = (loc, frameStatePath = activeFrameStatePath()) => {
  * @param suggestionList - The suggestion list to use to figure out the suffix.
  */
 const updateUrlSuffix = (suggestionList) => {
-  const suggestion = suggestionList && suggestionList.get(0)
+  let selectedIndex = windowState.getIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']))
+
+  if (!selectedIndex) {
+    selectedIndex = 0
+  } else {
+    selectedIndex--
+  }
+
+  const suggestion = suggestionList && suggestionList.get(selectedIndex)
   let suffix = ''
   if (suggestion) {
-    const selectedIndex = windowState.getIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']))
     const autocompleteEnabled = windowState.getIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'suggestions', 'autocompleteEnabled']))
-    if (!selectedIndex && autocompleteEnabled) {
+
+    if (autocompleteEnabled) {
       const location = windowState.getIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'location']))
       const index = suggestion.location.toLowerCase().indexOf(location.toLowerCase())
       if (index !== -1) {
@@ -512,6 +520,7 @@ const doAction = (action) => {
       break
     case WindowConstants.WINDOW_SET_URL_BAR_SUGGESTIONS:
       windowState = windowState.setIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']), action.selectedIndex)
+
       if (action.suggestionList !== undefined) {
         windowState = windowState.setIn(activeFrameStatePath().concat(['navbar', 'urlbar', 'suggestions', 'suggestionList']), action.suggestionList)
       }

test/components/urlBarSuggestionsTest.js

@@ -17,6 +17,8 @@ describe('urlbarSuggestions', function () {
   Brave.beforeAll(this)
   before(function * () {
     this.page1Url = Brave.server.url('page1.html')
+    this.page2Url = Brave.server.url('page2.html')
+
     yield setup(this.app.client)
     yield this.app.client
       .tabByUrl(Brave.newTabUrl)
@@ -28,6 +30,17 @@ describe('urlbarSuggestions', function () {
       .windowByUrl(Brave.browserWindowUrl)
       .waitForExist('.tab[data-frame-key="2"].active')
       .waitForElementFocus(urlInput)
+
+    // add the page2.html to history (necessary for autocomplete mouseover test)
+    yield this.app.client
+      .tabByUrl(Brave.newTabUrl)
+      .url(this.page2Url)
+      .waitForUrl(this.page2Url)
+      .windowByUrl(Brave.browserWindowUrl)
+      .ipcSend(messages.SHORTCUT_NEW_FRAME)
+      .waitForUrl(Brave.newTabUrl)
+      .windowByUrl(Brave.browserWindowUrl)
+      .waitForElementFocus(urlInput)
   })
 
   it('deactivates suggestions on escape', function * () {
@@ -60,7 +73,7 @@ describe('urlbarSuggestions', function () {
     yield this.app.client.ipcSend(messages.SHORTCUT_NEW_FRAME)
       .waitForUrl(Brave.newTabUrl)
       .windowByUrl(Brave.browserWindowUrl)
-      .waitForExist('.tab[data-frame-key="3"].active')
+      .waitForExist('.tab[data-frame-key="4"].active')
       .ipcSend('shortcut-focus-url')
       .waitForElementFocus(urlInput)
       .setValue(urlInput, 'Page 1')
@@ -81,7 +94,7 @@ describe('urlbarSuggestions', function () {
     yield this.app.client.ipcSend(messages.SHORTCUT_NEW_FRAME)
       .waitForUrl(Brave.newTabUrl)
       .windowByUrl(Brave.browserWindowUrl)
-      .waitForExist('.tab[data-frame-key="4"].active')
+      .waitForExist('.tab[data-frame-key="5"].active')
       .ipcSend('shortcut-focus-url')
       .waitForElementFocus(urlInput)
       .setValue(urlInput, 'http://')
@@ -92,4 +105,38 @@ describe('urlbarSuggestions', function () {
       .setValue(urlInput, 'Page')
       .waitForExist(urlBarSuggestions + ' li.selected', 1000, true)
   })
+
+  it('on suggestion mouseover, appends autocomplete URLs without interrupting typing', function * () {
+    const page1Url = Brave.server.url('page1.html')
+    const page2Url = Brave.server.url('page2.html')
+    const pagePartialUrl = Brave.server.url('page')
+    // test that entering a partial URL which matches two options autocompletes initially to 1st option,
+    // but switches to other option when that is highlighted, while keeping cursor in same position,
+    // so that finally, if the rest of the 1st option is entered via keyboard, it overwrites the suggestion from the mouse
+    yield this.app.client.ipcSend(messages.SHORTCUT_NEW_FRAME)
+      .waitForUrl(Brave.newTabUrl)
+      .windowByUrl(Brave.browserWindowUrl)
+      .waitForExist('.tab[data-frame-key="6"].active')
+      .ipcSend('shortcut-focus-url')
+      .waitForElementFocus(urlInput)
+      .keys(pagePartialUrl)
+      .waitUntil(function () {
+        return this.getValue(urlInput).then(function (val) {
+          return val === page1Url // after entering partial URL matching two options, 1st is tentatively filled in (_without_ moving cursor to end)
+        })
+      })
+      .waitForExist(urlBarSuggestions + ' li.suggestionItem')
+      .moveToObject(urlBarSuggestions + ' li.suggestionItem:not(.selected)')
+      .waitUntil(function () {
+        return this.getValue(urlInput).then(function (val) {
+          return val === page2Url // mousing over 2nd option tentatively completes URL with 2nd option (_without_ moving cursor to end)
+        })
+      })
+      .keys('1.html')
+      .waitUntil(function () {
+        return this.getValue(urlInput).then(function (val) {
+          return val === page1Url // without moving mouse, typing rest of 1st option URL overwrites the autocomplete from mouseover
+        })
+      })
+  })
 })