diff --git a/js/components/urlBar.js b/js/components/urlBar.js index 950fa40bedd..6b7ab7783c0 100644 --- a/js/components/urlBar.js +++ b/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) { diff --git a/js/components/urlBarSuggestions.js b/js/components/urlBarSuggestions.js index 69807cd4cae..d1a9ad74dd0 100644 --- a/js/components/urlBarSuggestions.js +++ b/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
  • +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 +} diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 75abc2f675e..b5bb6eaf2a8 100644 --- a/js/stores/windowStore.js +++ b/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) } diff --git a/test/components/urlBarSuggestionsTest.js b/test/components/urlBarSuggestionsTest.js index fe94ba34a6c..e414dc27cb7 100644 --- a/test/components/urlBarSuggestionsTest.js +++ b/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 + }) + }) + }) })