From 258b937b18d31fe92419732c9e414fbd8d27f68f Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Wed, 10 May 2017 13:07:42 -0400 Subject: [PATCH 01/22] Tokenize urlbar suggestions and use bloodhound --- app/common/lib/siteSuggestions.js | 61 +++++++++ package.json | 1 + .../app/common/lib/siteSuggestionsTest.js | 116 ++++++++++++++++++ 3 files changed, 178 insertions(+) create mode 100644 app/common/lib/siteSuggestions.js create mode 100644 test/unit/app/common/lib/siteSuggestionsTest.js diff --git a/app/common/lib/siteSuggestions.js b/app/common/lib/siteSuggestions.js new file mode 100644 index 00000000000..ea0b80d4614 --- /dev/null +++ b/app/common/lib/siteSuggestions.js @@ -0,0 +1,61 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const Bloodhound = require('bloodhound-js') +let initialized = false +let engine + +const init = (sites) => { + if (initialized) { + return Promise.resolve() + } + engine = new Bloodhound({ + local: sites, + queryTokenizer: tokenizeInput, + datumTokenizer: tokenizeInput + }) + const promise = engine.initialize() + promise.then(() => { + initialized = true + }) + return promise +} + +const tokenizeInput = (data) => { + let url = data || '' + let parts = [] + + if (typeof data === 'object' && data !== null) { + url = data.location + if (data.title) { + parts = data.title.toLowerCase().split(/\s/) + } + } + + if (url) { + url = url.toLowerCase().replace(/^https?:\/\//i, '') + parts = parts.concat(url.split(/[.\s\\/?&]/)) + } + + return parts +} + +const query = (input) => { + if (!initialized) { + return Promise.resolve([]) + } + return new Promise((resolve, reject) => { + engine.search(input, function (results) { + resolve(results) + }, function (err) { + reject(err) + }) + }) +} + +module.exports = { + init, + tokenizeInput, + query +} diff --git a/package.json b/package.json index 6887629df43..78aab259ba7 100644 --- a/package.json +++ b/package.json @@ -89,6 +89,7 @@ "ad-block": "^3.0.2", "aphrodite": "^1.0.0", "async": "^2.0.1", + "bloodhound-js": "^1.2.1", "clipboard-copy": "^1.0.0", "electron-localshortcut": "^0.6.0", "electron-squirrel-startup": "brave/electron-squirrel-startup", diff --git a/test/unit/app/common/lib/siteSuggestionsTest.js b/test/unit/app/common/lib/siteSuggestionsTest.js new file mode 100644 index 00000000000..1ba15b5fc42 --- /dev/null +++ b/test/unit/app/common/lib/siteSuggestionsTest.js @@ -0,0 +1,116 @@ +/* global describe, before, it */ +const {tokenizeInput, init, query} = require('../../../../../app/common/lib/siteSuggestions') +const assert = require('assert') + +const site1 = { + location: 'https://www.bradrichter.co/bad_numbers/3', + title: 'Do not use 3 for items because it is prime' +} +const site2 = { + location: 'https://www.brave.com', + title: 'No really, take back the web' +} +const site3 = { + location: 'https://www.bradrichter.co/bad_numbers/5', + title: 'Do not use 5 it is so bad, try 6 instead. Much better.' +} + +const site4 = { + location: 'https://www.designers.com/brad', + title: 'Brad Saves The World!' +} + +require('../../../braveUnit') + +describe('siteSuggestions lib', function () { + describe('tokenizeInput', function () { + it('empty string has no tokens', function () { + assert.deepEqual(tokenizeInput(''), []) + }) + it('undefined has no tokens', function () { + assert.deepEqual(tokenizeInput(null), []) + }) + it('null has no tokens', function () { + assert.deepEqual(tokenizeInput(undefined), []) + }) + it('lowercases tokens', function () { + assert.deepEqual(tokenizeInput('BRaD HaTES PRIMES'), ['brad', 'hates', 'primes']) + }) + it('does not include http', function () { + assert.deepEqual(tokenizeInput('http://bradrichter.co/I/hate/primes.html'), ['bradrichter', 'co', 'i', 'hate', 'primes', 'html']) + }) + it('does not include https as a token', function () { + assert.deepEqual(tokenizeInput('https://bradrichter.co/I/hate/primes.html'), ['bradrichter', 'co', 'i', 'hate', 'primes', 'html']) + }) + it('spaces get tokenized', function () { + assert.deepEqual(tokenizeInput('brad\thates primes'), ['brad', 'hates', 'primes']) + }) + it('periods get tokenized', function () { + assert.deepEqual(tokenizeInput('brad.hates.primes'), ['brad', 'hates', 'primes']) + }) + it('/ gets tokenized', function () { + assert.deepEqual(tokenizeInput('brad/hates/primes'), ['brad', 'hates', 'primes']) + }) + it('\\ gets tokenized', function () { + assert.deepEqual(tokenizeInput('brad\\hates\\primes'), ['brad', 'hates', 'primes']) + }) + it('? gets tokenized', function () { + assert.deepEqual(tokenizeInput('brad?hates?primes'), ['brad', 'hates', 'primes']) + }) + it('& gets tokenized', function () { + assert.deepEqual(tokenizeInput('brad&hates&primes'), ['brad', 'hates', 'primes']) + }) + it('can tokenize site objects', function () { + assert.deepEqual(tokenizeInput(site1), ['do', 'not', 'use', '3', 'for', 'items', 'because', 'it', 'is', 'prime', 'www', 'bradrichter', 'co', 'bad_numbers', '3']) + }) + }) + + const checkResult = (inputQuery, expectedResults, cb) => { + query(inputQuery).then((results) => { + assert.deepEqual(results, expectedResults) + cb() + }) + } + + describe('not initialized query', function () { + it('returns no results if not initialized', function (cb) { + checkResult('hello', [], cb) + }) + }) + describe('query', function () { + before(function (cb) { + const sites = [site1, site2, site3, site4] + init(sites).then(cb.bind(null, null)) + }) + it('can query with empty string', function (cb) { + checkResult('', [], cb) + }) + it('can query with null', function (cb) { + checkResult(null, [], cb) + }) + it('can query with undefined', function (cb) { + checkResult(undefined, [], cb) + }) + it('returns an empty array when there are no matches', function (cb) { + checkResult('hello', [], cb) + }) + it('returns matched result on an exact token', function (cb) { + checkResult('bradrichter', [site1, site3], cb) + }) + it('returns matched result on a token prefix', function (cb) { + checkResult('brad', [site1, site3, site4], cb) + }) + it('returns no results on input that has a token as a prefix', function (cb) { + checkResult('bradrichterhatesprimes.com', [], cb) + }) + it('can query on title', function (cb) { + checkResult('back', [site2], cb) + }) + it('can query on multiple tokens in different order', function (cb) { + checkResult('back really', [site2], cb) + }) + it('all tokens must match, not just some', function (cb) { + checkResult('brave brad', [], cb) + }) + }) +}) From 5b59bfe3079f824084cca0ef37de751badec5442 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Wed, 10 May 2017 13:29:59 -0400 Subject: [PATCH 02/22] Change URL input entered into an app action --- app/renderer/components/navigation/urlBar.js | 6 ++--- app/renderer/reducers/urlBarReducer.js | 5 +++-- docs/appActions.md | 22 +++++++++++++++++++ docs/windowActions.md | 11 ---------- js/actions/appActions.js | 19 ++++++++++++++-- js/actions/windowActions.js | 13 ----------- js/constants/appConstants.js | 1 + .../renderer/reducers/urlBarReducerTest.js | 7 +++--- 8 files changed, 50 insertions(+), 34 deletions(-) diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 3b113af4c56..0b2d3504c06 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -69,7 +69,7 @@ class UrlBar extends React.Component { if (this.urlInput) { this.setValue(location) } - windowActions.setNavBarUserInput(location) + appActions.urlBarTextChanged(getCurrentWindowId(), location) } /** @@ -292,7 +292,7 @@ class UrlBar extends React.Component { if (!this.keyPress) { // if this is a key press don't sent the update until keyUp so // showAutocompleteResult can handle the result - windowActions.setNavBarUserInput(val) + appActions.urlBarTextChanged(getCurrentWindowId(), val) } } } @@ -310,7 +310,7 @@ class UrlBar extends React.Component { // clear any current arrow or mouse hover selection windowActions.setUrlBarSuggestions(undefined, null) this.keyPressed = false - windowActions.setNavBarUserInput(this.lastVal) + appActions.urlBarTextChanged(getCurrentWindowId(), this.lastVal) } select () { diff --git a/app/renderer/reducers/urlBarReducer.js b/app/renderer/reducers/urlBarReducer.js index 1df5cd82a6b..8a98928bd4f 100644 --- a/app/renderer/reducers/urlBarReducer.js +++ b/app/renderer/reducers/urlBarReducer.js @@ -5,6 +5,7 @@ 'use strict' const windowConstants = require('../../../js/constants/windowConstants') +const appConstants = require('../../../js/constants/appConstants') const {aboutUrls, isNavigatableAboutPage, isSourceAboutUrl, isUrl, getSourceAboutUrl, getSourceMagnetUrl} = require('../../../js/lib/appUrlUtil') const {isURL, isPotentialPhishingUrl, getUrlFromInput} = require('../../../js/lib/urlutil') const {getFrameByKey, getFrameKeyByTabId, activeFrameStatePath, frameStatePath, getActiveFrame, getFrameByTabId} = require('../../../js/state/frameStateUtil') @@ -388,8 +389,8 @@ const urlBarReducer = (state, action) => { const tabId = state.getIn(activeFrameStatePath(state).concat(['tabId']), tabState.TAB_ID_NONE) switch (action.actionType) { - case windowConstants.WINDOW_SET_NAVBAR_INPUT: - state = setNavBarUserInput(state, action.location) + case appConstants.APP_URL_BAR_TEXT_CHANGED: + state = setNavBarUserInput(state, action.input) break case windowConstants.WINDOW_SET_NAVIGATED: // For about: URLs, make sure we store the URL as about:something diff --git a/docs/appActions.md b/docs/appActions.md index fbe64eda399..b35447ca642 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -1023,6 +1023,18 @@ Notifies autoplay has been blocked +### savePassword() + +Handle 'save-password' event from muon + + + +### updatePassword() + +Handle 'update-password' event from muon + + + ### deletePassword(passwordDetail) Deletes login credentials @@ -1045,6 +1057,16 @@ Delete legacy "never saved password" list +### urlBarTextChanged(location) + +Indicates that the urlbar text has changed, usually from user input + +**Parameters** + +**location**: `string`, The text to set as the new navbar URL input + + + * * * diff --git a/docs/windowActions.md b/docs/windowActions.md index b7dc3227e48..f233b92136e 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -98,17 +98,6 @@ Dispatches a message to set the frame error state -### setNavBarUserInput(location) - -Dispatches a message to the store to set the user entered text for the URL bar. -Unlike setLocation and loadUrl, this does not modify the state of src and location. - -**Parameters** - -**location**: `string`, The text to set as the new navbar URL input - - - ### setFindbarShown(frameKey, shown) Shows/hides the find-in-page bar. diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 702047dc026..19d718ecf40 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -1284,7 +1284,7 @@ const appActions = { }) }, - /* + /** * Handle 'save-password' event from muon */ savePassword: function (username, origin, tabId) { @@ -1296,7 +1296,7 @@ const appActions = { }) }, - /* + /** * Handle 'update-password' event from muon */ updatePassword: function (username, origin, tabId) { @@ -1337,6 +1337,21 @@ const appActions = { hostPattern: origin, key: 'savePasswords' }) + }, + + /** + * Indicates that the urlbar text has changed, usually from user input + * + * @param {string} location - The text to set as the new navbar URL input + */ + urlBarTextChanged: function (windowId, input) { + AppDispatcher.dispatch({ + actionType: appConstants.APP_URL_BAR_TEXT_CHANGED, + input, + queryInfo: { + windowId + } + }) } } diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index ad3413cad4a..9dccf96c2fa 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -115,19 +115,6 @@ const windowActions = { }) }, - /** - * Dispatches a message to the store to set the user entered text for the URL bar. - * Unlike setLocation and loadUrl, this does not modify the state of src and location. - * - * @param {string} location - The text to set as the new navbar URL input - */ - setNavBarUserInput: function (location) { - dispatch({ - actionType: windowConstants.WINDOW_SET_NAVBAR_INPUT, - location - }) - }, - /** * Shows/hides the find-in-page bar. * @param {number} frameKey - Key of the frame that we want to modify diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index d103b4b858b..f127eccdda3 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -107,6 +107,7 @@ const appConstants = { APP_TAB_MESSAGE_BOX_UPDATED: _, APP_NAVIGATOR_HANDLER_REGISTERED: _, APP_NAVIGATOR_HANDLER_UNREGISTERED: _, + APP_URL_BAR_TEXT_CHANGED: _, APP_CHANGE_LEDGER_PINNED_PERCENTAGES: _, APP_ENABLE_UNDEFINED_PUBLISHERS: _, APP_TAB_PINNED: _, diff --git a/test/unit/app/renderer/reducers/urlBarReducerTest.js b/test/unit/app/renderer/reducers/urlBarReducerTest.js index 1baa6dd982d..a055b43563d 100644 --- a/test/unit/app/renderer/reducers/urlBarReducerTest.js +++ b/test/unit/app/renderer/reducers/urlBarReducerTest.js @@ -5,6 +5,7 @@ const assert = require('assert') const fakeElectron = require('../../../lib/fakeElectron') const windowConstants = require('../../../../../js/constants/windowConstants') +const appConstants = require('../../../../../js/constants/appConstants') require('../../../braveUnit') const windowState = Immutable.fromJS({ @@ -111,10 +112,10 @@ describe('urlBarReducer', function () { mockery.disable() }) - describe('WINDOW_SET_NAVBAR_INPUT', function () { + describe('APP_URL_BAR_TEXT_CHANGED', function () { before(function () { this.location = 'this test is brought to you by coffee.' - this.newState = urlBarReducer(windowState, {actionType: windowConstants.WINDOW_SET_NAVBAR_INPUT, location: this.location}) + this.newState = urlBarReducer(windowState, {actionType: appConstants.APP_URL_BAR_TEXT_CHANGED, input: this.location}) }) it('Changes urlbar state for active frame key', function () { @@ -288,7 +289,7 @@ describe('urlBarReducer', function () { }) }) - describe('WINDOW_SET_NAVBAR_INPUT', function () { + describe('APP_URL_BAR_TEXT_CHANGED', function () { // TODO }) From cfeb861dc9d9fe76782069c3b2b562e5a790954b Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Wed, 10 May 2017 17:29:58 -0400 Subject: [PATCH 03/22] Don't store suggestion click handlers in state --- app/renderer/reducers/urlBarReducer.js | 49 ++++++++----------------- app/renderer/suggestionClickHandlers.js | 34 +++++++++++++---- 2 files changed, 42 insertions(+), 41 deletions(-) diff --git a/app/renderer/reducers/urlBarReducer.js b/app/renderer/reducers/urlBarReducer.js index 8a98928bd4f..413e24a148c 100644 --- a/app/renderer/reducers/urlBarReducer.js +++ b/app/renderer/reducers/urlBarReducer.js @@ -19,7 +19,7 @@ const config = require('../../../js/constants/config') const top500 = require('../../../js/data/top500') const suggestion = require('../lib/suggestion') const suggestionTypes = require('../../../js/constants/suggestionTypes') -const {navigateSiteClickHandler, frameClickHandler} = require('../suggestionClickHandlers') +const {navigateSiteClickHandler} = require('../suggestionClickHandlers') const appStoreRenderer = require('../../../js/stores/appStoreRenderer') const navigationBarState = require('../../common/state/navigationBarState') @@ -133,8 +133,6 @@ const generateNewSuggestionsList = (state) => { const activeFrameKey = state.get('activeFrameKey') const urlLocation = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'location'])) const searchResults = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'searchResults'])) - const frameSearchDetail = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'searchDetail'])) - const searchDetail = state.get('searchDetail') if (!urlLocation) { return state @@ -142,10 +140,11 @@ const generateNewSuggestionsList = (state) => { const urlLocationLower = urlLocation.toLowerCase() let suggestionsList = new Immutable.List() - const defaultme = (x) => x - const mapListToElements = ({data, maxResults, type, clickHandler = navigateSiteClickHandler.bind(this), - sortHandler = defaultme, formatTitle = defaultme, formatUrl = defaultme, - filterValue = (site) => { + const formatUrl = (x) => typeof x === 'object' && x !== null ? x.get('location') : x + const formatTitle = (x) => typeof x === 'object' && x !== null ? x.get('title') : x + const formatTabId = (x) => typeof x === 'object' && x !== null ? x.get('tabId') : x + const mapListToElements = ({data, maxResults, type, + sortHandler = (x) => x, filterValue = (site) => { return site.toLowerCase().indexOf(urlLocationLower) !== -1 } }) => { @@ -165,9 +164,9 @@ const generateNewSuggestionsList = (state) => { .take(maxResults) .map((site) => { return { - onClick: clickHandler.bind(null, site), title: formatTitle(site), location: formatUrl(site), + tabId: formatTabId(site), type } }) @@ -247,12 +246,7 @@ const generateNewSuggestionsList = (state) => { data: historySites, maxResults: config.urlBarSuggestions.maxHistorySites, type: suggestionTypes.HISTORY, - clickHandler: navigateSiteClickHandler((site) => { - return site.get('location') - }), sortHandler: sortBasedOnLocationPos, - formatTitle: (site) => site.get('title'), - formatUrl: (site) => site.get('location'), filterValue: null })) } @@ -262,12 +256,7 @@ const generateNewSuggestionsList = (state) => { data: bookmarkSites, maxResults: config.urlBarSuggestions.maxBookmarkSites, type: suggestionTypes.BOOKMARK, - clickHandler: navigateSiteClickHandler((site) => { - return site.get('location') - }), sortHandler: sortBasedOnLocationPos, - formatTitle: (site) => site.get('title'), - formatUrl: (site) => site.get('location'), filterValue: null })) } @@ -277,8 +266,8 @@ const generateNewSuggestionsList = (state) => { suggestionsList = suggestionsList.concat(mapListToElements({ data: aboutUrls.keySeq().filter((x) => isNavigatableAboutPage(x)), maxResults: config.urlBarSuggestions.maxAboutPages, - type: suggestionTypes.ABOUT_PAGES, - clickHandler: navigateSiteClickHandler((x) => x)})) + type: suggestionTypes.ABOUT_PAGES + })) // opened frames if (getSetting(settings.OPENED_TAB_SUGGESTIONS)) { @@ -286,10 +275,7 @@ const generateNewSuggestionsList = (state) => { data: state.get('frames'), maxResults: config.urlBarSuggestions.maxOpenedFrames, type: suggestionTypes.TAB, - clickHandler: frameClickHandler, sortHandler: sortBasedOnLocationPos, - formatTitle: (frame) => frame.get('title'), - formatUrl: (frame) => frame.get('location'), filterValue: (frame) => !isSourceAboutUrl(frame.get('location')) && frame.get('key') !== activeFrameKey && ( @@ -304,12 +290,7 @@ const generateNewSuggestionsList = (state) => { suggestionsList = suggestionsList.concat(mapListToElements({ data: searchResults, maxResults: config.urlBarSuggestions.maxSearch, - type: suggestionTypes.SEARCH, - clickHandler: navigateSiteClickHandler((searchTerms) => { - let searchURL = frameSearchDetail - ? frameSearchDetail.get('search') : searchDetail.get('searchURL') - return searchURL.replace('{searchTerms}', encodeURIComponent(searchTerms)) - }) + type: suggestionTypes.SEARCH })) } @@ -317,8 +298,8 @@ const generateNewSuggestionsList = (state) => { suggestionsList = suggestionsList.concat(mapListToElements({ data: top500, maxResults: config.urlBarSuggestions.maxTopSites, - type: suggestionTypes.TOP_SITE, - clickHandler: navigateSiteClickHandler((x) => x)})) + type: suggestionTypes.TOP_SITE + })) return setUrlSuggestions(state, suggestionsList) } @@ -519,8 +500,10 @@ const urlBarReducer = (state, action) => { const suggestionList = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'suggestionList'])) if (suggestionList.size > 0) { // It's important this doesn't run sync or else the returned state below will overwrite anything done in the click handler - setImmediate(() => - suggestionList.get(selectedIndex).onClick(action.isForSecondaryAction, action.shiftKey)) + setImmediate(() => { + const suggestion = suggestionList.get(selectedIndex) + navigateSiteClickHandler(suggestion, action.isForSecondaryAction, action.shiftKey) + }) } break } diff --git a/app/renderer/suggestionClickHandlers.js b/app/renderer/suggestionClickHandlers.js index 72c41b6038e..129e08f7683 100644 --- a/app/renderer/suggestionClickHandlers.js +++ b/app/renderer/suggestionClickHandlers.js @@ -7,16 +7,38 @@ const appActions = require('../../js/actions/appActions') const windowActions = require('../../js/actions/windowActions') const windowStore = require('../../js/stores/windowStore') +const suggestionTypes = require('../../js/constants/suggestionTypes') +const {makeImmutable} = require('../common/state/immutableUtil') const {getActiveFrame} = require('../../js/state/frameStateUtil') -const navigateSiteClickHandler = (formatUrl) => (site, isForSecondaryAction, shiftKey) => { - const url = formatUrl(site) +const navigateSiteClickHandler = (suggestionData, isForSecondaryAction, shiftKey) => { // When clicked make sure to hide autocomplete windowActions.setRenderUrlBarSuggestions(false) + + suggestionData = makeImmutable(suggestionData) + const type = suggestionData.get('type') + let partitionNumber + let url + if (type === suggestionTypes.SEARCH) { + const frameSearchDetail = suggestionData.getIn(['navbar', 'urlbar', 'searchDetail']) + const searchDetail = windowStore.state.get('searchDetail') + let searchURL = frameSearchDetail + ? frameSearchDetail.get('search') : searchDetail.get('searchURL') + url = searchURL.replace('{searchTerms}', encodeURIComponent(suggestionData.get('location'))) + } else if (type === suggestionTypes.TAB) { + appActions.tabActivateRequested(suggestionData.get('tabId')) + return + } else if (type === suggestionTypes.TOP_SITE) { + url = suggestionData.get('location') + } else { + url = suggestionData.get('location') + partitionNumber = (suggestionData && suggestionData.get && suggestionData.get('partitionNumber')) || undefined + } + if (isForSecondaryAction) { appActions.createTabRequested({ url, - partitionNumber: (site && site.get && site.get('partitionNumber')) || undefined, + partitionNumber, active: !!shiftKey }) } else { @@ -26,10 +48,6 @@ const navigateSiteClickHandler = (formatUrl) => (site, isForSecondaryAction, shi } } -const frameClickHandler = (frameProps) => - appActions.tabActivateRequested(frameProps.get('tabId')) - module.exports = { - navigateSiteClickHandler, - frameClickHandler + navigateSiteClickHandler } From dd1bb2224ef68a167762bacbf028e7a3f3feb72f Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Wed, 10 May 2017 22:44:31 -0400 Subject: [PATCH 04/22] Change URL suggestion generation to async --- app/renderer/components/navigation/urlBar.js | 8 +++---- .../navigation/urlBarSuggestions.js | 5 +++-- app/renderer/reducers/urlBarReducer.js | 22 ++++++++++++------- docs/appActions.md | 12 ++++++++++ docs/windowActions.md | 12 ---------- js/actions/appActions.js | 15 +++++++++++++ js/actions/windowActions.js | 14 ------------ js/constants/appConstants.js | 1 + js/constants/windowConstants.js | 1 - .../renderer/reducers/urlBarReducerTest.js | 4 ++-- 10 files changed, 51 insertions(+), 43 deletions(-) diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 0b2d3504c06..1d2ff0c58d4 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -94,7 +94,7 @@ class UrlBar extends React.Component { if (this.props.autocompleteEnabled) { windowActions.urlBarAutocompleteEnabled(false) } - windowActions.setUrlBarSuggestions(undefined, null) + appActions.urlBarSuggestionsChanged(undefined, null) windowActions.setRenderUrlBarSuggestions(false) } @@ -270,7 +270,7 @@ class UrlBar extends React.Component { if (e.target.value !== this.lastVal + this.lastSuffix) { e.preventDefault() // clear any current arrow or mouse hover selection - windowActions.setUrlBarSuggestions(undefined, null) + appActions.urlBarSuggestionsChanged(undefined, null) this.setValue(e.target.value) } } @@ -308,7 +308,7 @@ class UrlBar extends React.Component { windowActions.setUrlBarSelected(false) } // clear any current arrow or mouse hover selection - windowActions.setUrlBarSuggestions(undefined, null) + appActions.urlBarSuggestionsChanged(undefined, null) this.keyPressed = false appActions.urlBarTextChanged(getCurrentWindowId(), this.lastVal) } @@ -365,7 +365,7 @@ class UrlBar extends React.Component { if (this.props.isFocused) { this.focus() } - windowActions.setUrlBarSuggestions(undefined, null) + appActions.urlBarSuggestionsChanged(undefined, null) windowActions.setRenderUrlBarSuggestions(false) } else if (this.props.location !== prevProps.location) { // This is a url nav change diff --git a/app/renderer/components/navigation/urlBarSuggestions.js b/app/renderer/components/navigation/urlBarSuggestions.js index 4f06ea0ca7d..4f56f9c2bb1 100644 --- a/app/renderer/components/navigation/urlBarSuggestions.js +++ b/app/renderer/components/navigation/urlBarSuggestions.js @@ -7,6 +7,7 @@ const ImmutableComponent = require('../immutableComponent') const UrlBarSuggestionItem = require('./urlBarSuggestionItem') const windowActions = require('../../../../js/actions/windowActions') +const appActions = require('../../../../js/actions/appActions') const suggestionTypes = require('../../../../js/constants/suggestionTypes') const cx = require('../../../../js/lib/classSet') const locale = require('../../../../js/l10n') @@ -27,7 +28,7 @@ class UrlBarSuggestions extends ImmutableComponent { } blur () { - windowActions.setUrlBarSuggestions(null, null) + appActions.urlBarSuggestionsChanged(null, null) } onSuggestionClicked (e) { @@ -105,7 +106,7 @@ class UrlBarSuggestions extends ImmutableComponent { if (newIndex === 0 || newIndex > suggestions.size) { newIndex = null } - windowActions.setUrlBarSuggestions(suggestions, newIndex) + appActions.urlBarSuggestionsChanged(suggestions, newIndex) } } diff --git a/app/renderer/reducers/urlBarReducer.js b/app/renderer/reducers/urlBarReducer.js index 413e24a148c..13423d9db72 100644 --- a/app/renderer/reducers/urlBarReducer.js +++ b/app/renderer/reducers/urlBarReducer.js @@ -301,7 +301,8 @@ const generateNewSuggestionsList = (state) => { type: suggestionTypes.TOP_SITE })) - return setUrlSuggestions(state, suggestionsList) + const appActions = require('../../../js/actions/appActions') + appActions.urlBarSuggestionsChanged(suggestionsList) } const getLocation = (location) => { @@ -346,8 +347,9 @@ const setNavBarUserInput = (state, location) => { const activeFrameProps = getActiveFrame(state) state = updateSearchEngineInfoFromInput(state, activeFrameProps) state = searchXHR(state, activeFrameProps, true) - state = generateNewSuggestionsList(state) - state = updateUrlSuffix(state, state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'suggestionList']), Immutable.Map())) + setImmediate(() => { + generateNewSuggestionsList(state) + }) if (!location) { state = setRenderUrlBarSuggestions(state, false) } @@ -373,6 +375,12 @@ const urlBarReducer = (state, action) => { case appConstants.APP_URL_BAR_TEXT_CHANGED: state = setNavBarUserInput(state, action.input) break + case appConstants.APP_URL_BAR_SUGGESTIONS_CHANGED: + if (action.selectedIndex !== undefined) { + state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']), action.selectedIndex) + } + state = setUrlSuggestions(state, action.suggestionList) + break case windowConstants.WINDOW_SET_NAVIGATED: // For about: URLs, make sure we store the URL as about:something // and not what we map to. @@ -480,15 +488,13 @@ const urlBarReducer = (state, action) => { case windowConstants.WINDOW_SEARCH_SUGGESTION_RESULTS_AVAILABLE: const frameKey = getFrameKeyByTabId(state, action.tabId) state = state.setIn(frameStatePath(state, frameKey).concat(['navbar', 'urlbar', 'suggestions', 'searchResults']), action.searchResults) - state = generateNewSuggestionsList(state) + setImmediate(() => { + generateNewSuggestionsList(state) + }) break case windowConstants.WINDOW_URL_BAR_AUTOCOMPLETE_ENABLED: state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'autocompleteEnabled']), action.enabled) break - case windowConstants.WINDOW_SET_URL_BAR_SUGGESTIONS: - state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']), action.selectedIndex) - state = setUrlSuggestions(state, action.suggestionList) - break case windowConstants.WINDOW_SET_URL_BAR_ACTIVE: state = setActive(state, action.isActive) break diff --git a/docs/appActions.md b/docs/appActions.md index b35447ca642..f13421222ce 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -1067,6 +1067,18 @@ Indicates that the urlbar text has changed, usually from user input +### urlBarSuggestionsChanged(suggestionList, selectedIndex) + +Indicates URL bar suggestions and selected index. + +**Parameters** + +**suggestionList**: `Array.<Object>`, The list of suggestions for the entered URL bar text. This can be generated from history, bookmarks, etc. + +**selectedIndex**: `number`, The index for the selected item (users can select items with down arrow on their keyboard) + + + * * * diff --git a/docs/windowActions.md b/docs/windowActions.md index f233b92136e..46f95910a44 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -280,18 +280,6 @@ Dispatches a message to the store to indicate that the specified frame should mo -### setUrlBarSuggestions(suggestionList, selectedIndex) - -Sets the URL bar suggestions and selected index. - -**Parameters** - -**suggestionList**: `Array.<Object>`, The list of suggestions for the entered URL bar text. This can be generated from history, bookmarks, etc. - -**selectedIndex**: `number`, The index for the selected item (users can select items with down arrow on their keyboard) - - - ### activeSuggestionClicked(isForSecondaryAction, shiftKey) The active URL bar suggestion was clicked diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 19d718ecf40..0747b85f7ce 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -1352,7 +1352,22 @@ const appActions = { windowId } }) + }, + + /** + * Indicates URL bar suggestions and selected index. + * + * @param {Object[]} suggestionList - The list of suggestions for the entered URL bar text. This can be generated from history, bookmarks, etc. + * @param {number} selectedIndex - The index for the selected item (users can select items with down arrow on their keyboard) + */ + urlBarSuggestionsChanged: function (suggestionList, selectedIndex) { + AppDispatcher.dispatch({ + actionType: appConstants.APP_URL_BAR_SUGGESTIONS_CHANGED, + suggestionList, + selectedIndex + }) } + } module.exports = appActions diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 9dccf96c2fa..ceca582cb8e 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -339,20 +339,6 @@ const windowActions = { }) }, - /** - * Sets the URL bar suggestions and selected index. - * - * @param {Object[]} suggestionList - The list of suggestions for the entered URL bar text. This can be generated from history, bookmarks, etc. - * @param {number} selectedIndex - The index for the selected item (users can select items with down arrow on their keyboard) - */ - setUrlBarSuggestions: function (suggestionList, selectedIndex) { - dispatch({ - actionType: windowConstants.WINDOW_SET_URL_BAR_SUGGESTIONS, - suggestionList, - selectedIndex - }) - }, - /** * The active URL bar suggestion was clicked * @param {boolean} isForSecondaryAction - Whether the secondary action is expected diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index f127eccdda3..9152153b13a 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -108,6 +108,7 @@ const appConstants = { APP_NAVIGATOR_HANDLER_REGISTERED: _, APP_NAVIGATOR_HANDLER_UNREGISTERED: _, APP_URL_BAR_TEXT_CHANGED: _, + APP_URL_BAR_SUGGESTIONS_CHANGED: _, APP_CHANGE_LEDGER_PINNED_PERCENTAGES: _, APP_ENABLE_UNDEFINED_PUBLISHERS: _, APP_TAB_PINNED: _, diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index 094c82bd44c..4c2db17cf64 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -20,7 +20,6 @@ const windowConstants = { WINDOW_WEBVIEW_LOAD_END: _, WINDOW_SET_FULL_SCREEN: _, WINDOW_SET_LINK_HOVER_PREVIEW: _, - WINDOW_SET_URL_BAR_SUGGESTIONS: _, WINDOW_SET_RENDER_URL_BAR_SUGGESTIONS: _, WINDOW_URL_BAR_AUTOCOMPLETE_ENABLED: _, WINDOW_SEARCH_SUGGESTION_RESULTS_AVAILABLE: _, diff --git a/test/unit/app/renderer/reducers/urlBarReducerTest.js b/test/unit/app/renderer/reducers/urlBarReducerTest.js index a055b43563d..be22d690e30 100644 --- a/test/unit/app/renderer/reducers/urlBarReducerTest.js +++ b/test/unit/app/renderer/reducers/urlBarReducerTest.js @@ -300,10 +300,10 @@ describe('urlBarReducer', function () { }) }) - describe('WINDOW_SET_URL_BAR_SUGGESTIONS', function () { + describe('APP_URL_BAR_SUGGESTIONS_CHANGED', function () { it('suggestion results can be updated', function () { const suggestionList = Immutable.fromJS(['0.207879576']) - const newState = urlBarReducer(windowState, {actionType: windowConstants.WINDOW_SET_URL_BAR_SUGGESTIONS, suggestionList, selectedIndex: null}) + const newState = urlBarReducer(windowState, {actionType: appConstants.APP_URL_BAR_SUGGESTIONS_CHANGED, suggestionList, selectedIndex: null}) assert.equal(newState.getIn(['frames', 1, 'navbar', 'urlbar', 'suggestions', 'suggestionList']), suggestionList) assert.equal(newState.getIn(['frames', 1, 'navbar', 'urlbar', 'suggestions', 'selectedIndex']), null) }) From be7ad9fe799802a52223e9ac433eba572691fd68 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Fri, 12 May 2017 08:54:27 -0400 Subject: [PATCH 05/22] Split out suggestion generation into parts --- .../navigation/urlBarSuggestionItem.js | 12 +- .../navigation/urlBarSuggestions.js | 12 +- app/renderer/lib/suggestion.js | 259 +++++++++++++++++- app/renderer/reducers/urlBarReducer.js | 197 +------------ 4 files changed, 272 insertions(+), 208 deletions(-) diff --git a/app/renderer/components/navigation/urlBarSuggestionItem.js b/app/renderer/components/navigation/urlBarSuggestionItem.js index 027277a9e65..ed7b9655244 100644 --- a/app/renderer/components/navigation/urlBarSuggestionItem.js +++ b/app/renderer/components/navigation/urlBarSuggestionItem.js @@ -20,21 +20,21 @@ class UrlBarSuggestionItem extends ImmutableComponent { return
  • { this.node = node }} className={cx({ selected: this.props.selected, suggestionItem: true, - [this.props.suggestion.type]: true + [this.props.suggestion.get('type')]: true })}> { - this.props.suggestion.type !== suggestionTypes.TOP_SITE && this.props.suggestion.title - ?
    {this.props.suggestion.title}
    + this.props.suggestion.get('type') !== suggestionTypes.TOP_SITE && this.props.suggestion.get('title') + ?
    {this.props.suggestion.get('title')}
    : null } { - this.props.suggestion.type !== suggestionTypes.SEARCH && this.props.suggestion.type !== suggestionTypes.ABOUT_PAGES - ?
    {this.props.suggestion.location}
    + this.props.suggestion.get('type') !== suggestionTypes.SEARCH && this.props.suggestion.get('type') !== suggestionTypes.ABOUT_PAGES + ?
    {this.props.suggestion.get('location')}
    : null }
    • diff --git a/app/renderer/components/navigation/urlBarSuggestions.js b/app/renderer/components/navigation/urlBarSuggestions.js index 4f56f9c2bb1..f1fc02b4e75 100644 --- a/app/renderer/components/navigation/urlBarSuggestions.js +++ b/app/renderer/components/navigation/urlBarSuggestions.js @@ -37,12 +37,12 @@ class UrlBarSuggestions extends ImmutableComponent { render () { const suggestions = this.props.suggestionList - const bookmarkSuggestions = suggestions.filter((s) => s.type === suggestionTypes.BOOKMARK) - const historySuggestions = suggestions.filter((s) => s.type === suggestionTypes.HISTORY) - const aboutPagesSuggestions = suggestions.filter((s) => s.type === suggestionTypes.ABOUT_PAGES) - const tabSuggestions = suggestions.filter((s) => s.type === suggestionTypes.TAB) - const searchSuggestions = suggestions.filter((s) => s.type === suggestionTypes.SEARCH) - const topSiteSuggestions = suggestions.filter((s) => s.type === suggestionTypes.TOP_SITE) + const bookmarkSuggestions = suggestions.filter((s) => s.get('type') === suggestionTypes.BOOKMARK) + const historySuggestions = suggestions.filter((s) => s.get('type') === suggestionTypes.HISTORY) + const aboutPagesSuggestions = suggestions.filter((s) => s.get('type') === suggestionTypes.ABOUT_PAGES) + const tabSuggestions = suggestions.filter((s) => s.get('type') === suggestionTypes.TAB) + const searchSuggestions = suggestions.filter((s) => s.get('type') === suggestionTypes.SEARCH) + const topSiteSuggestions = suggestions.filter((s) => s.get('type') === suggestionTypes.TOP_SITE) let items = [] let index = 0 diff --git a/app/renderer/lib/suggestion.js b/app/renderer/lib/suggestion.js index fed465eefd4..2a0fe8fc81b 100644 --- a/app/renderer/lib/suggestion.js +++ b/app/renderer/lib/suggestion.js @@ -7,6 +7,15 @@ const appConfig = require('../../../js/constants/appConfig') const _ = require('underscore') const Immutable = require('immutable') const {makeImmutable} = require('../../common/state/immutableUtil') +const {aboutUrls, isNavigatableAboutPage, isSourceAboutUrl} = require('../../../js/lib/appUrlUtil') +const suggestionTypes = require('../../../js/constants/suggestionTypes') +const getSetting = require('../../../js/settings').getSetting +const settings = require('../../../js/constants/settings') +const appStoreRenderer = require('../../../js/stores/appStoreRenderer') +const {isBookmark, isDefaultEntry, isHistoryEntry} = require('../../../js/state/siteUtil') +const config = require('../../../js/constants/config') +const top500 = require('../../../js/data/top500') +const {activeFrameStatePath} = require('../../../js/state/frameStateUtil') const sigmoid = (t) => { return 1 / (1 + Math.pow(Math.E, -t)) @@ -23,7 +32,7 @@ const ONE_DAY = 1000 * 60 * 60 * 24 * @param {boolean} lastAccessedTime - Epoch milliseconds of last access * */ -module.exports.sortingPriority = (count, currentTime, lastAccessedTime, ageDecayConstant) => { +const sortingPriority = (count, currentTime, lastAccessedTime, ageDecayConstant) => { // number of days since last access (with fractional component) const ageInDays = (currentTime - (lastAccessedTime || currentTime)) / ONE_DAY // decay factor based on age @@ -71,15 +80,15 @@ module.exports.sortingPriority = (count, currentTime, lastAccessedTime, ageDecay * priority 0.9982851225143763 * */ -module.exports.sortByAccessCountWithAgeDecay = (s1, s2) => { +const sortByAccessCountWithAgeDecay = (s1, s2) => { const now = new Date() - const s1Priority = module.exports.sortingPriority( + const s1Priority = sortingPriority( s1.get('count') || 0, now.getTime(), s1.get('lastAccessedTime') || now.getTime(), appConfig.urlSuggestions.ageDecayConstant ) - const s2Priority = module.exports.sortingPriority( + const s2Priority = sortingPriority( s2.get('count') || 0, now.getTime(), s2.get('lastAccessedTime') || now.getTime(), @@ -95,7 +104,7 @@ module.exports.sortByAccessCountWithAgeDecay = (s1, s2) => { * @param {ImmutableObject} site - object represent history entry * */ -module.exports.simpleDomainNameValue = (site) => { +const simpleDomainNameValue = (site) => { const parsed = urlParse(site.get('location')) if (parsed.hash === null && parsed.search === null && parsed.query === null && parsed.pathname === '/') { return 1 @@ -110,7 +119,7 @@ module.exports.simpleDomainNameValue = (site) => { * @param {string} location - history item location * */ -module.exports.normalizeLocation = (location) => { +const normalizeLocation = (location) => { if (typeof location === 'string') { location = location.replace(/www\./, '') location = location.replace(/^http:\/\//, '') @@ -126,7 +135,7 @@ module.exports.normalizeLocation = (location) => { * * @return true if urls being compared should be normalized */ -module.exports.shouldNormalizeLocation = (input) => { +const shouldNormalizeLocation = (input) => { const prefixes = ['http://', 'https://', 'www.'] return prefixes.every((prefix) => { if (input.length > prefix.length) { @@ -177,7 +186,7 @@ var virtualSite = (sites) => { * * @param {ImmutableList[ImmutableMap]} - history */ -module.exports.createVirtualHistoryItems = (historySites) => { +const createVirtualHistoryItems = (historySites) => { historySites = makeImmutable(historySites || {}) // parse each history item @@ -208,3 +217,237 @@ module.exports.createVirtualHistoryItems = (historySites) => { return [site.location, site] }))) } + +const sortBasedOnLocationPos = (urlLocationLower) => (s1, s2) => { + const shouldNormalize = shouldNormalizeLocation(urlLocationLower) + const urlLocationLowerNormalized = normalizeLocation(urlLocationLower) + + const location1 = shouldNormalize ? normalizeLocation(s1.get('location')) : s1.get('location') + const location2 = shouldNormalize ? normalizeLocation(s2.get('location')) : s2.get('location') + const pos1 = location1.indexOf(urlLocationLowerNormalized) + const pos2 = location2.indexOf(urlLocationLowerNormalized) + if (pos1 === -1 && pos2 === -1) { + return 0 + } else if (pos1 === -1) { + return 1 + } else if (pos2 === -1) { + return -1 + } else { + if (pos1 - pos2 !== 0) { + return pos1 - pos2 + } else { + // sort site.com higher than site.com/somepath + const sdnv1 = simpleDomainNameValue(s1) + const sdnv2 = simpleDomainNameValue(s2) + if (sdnv1 !== sdnv2) { + return sdnv2 - sdnv1 + } else { + // If there's a tie on the match location, use the age + // decay modified access count + return sortByAccessCountWithAgeDecay(s1, s2) + } + } + } +} + +const getMapListToElements = (urlLocationLower) => ({data, maxResults, type, + sortHandler = (x) => x, filterValue = (site) => { + return site.toLowerCase().indexOf(urlLocationLower) !== -1 + } +}) => { + const suggestionsList = Immutable.List() + const formatUrl = (x) => typeof x === 'object' && x !== null ? x.get('location') : x + const formatTitle = (x) => typeof x === 'object' && x !== null ? x.get('title') : x + const formatTabId = (x) => typeof x === 'object' && x !== null ? x.get('tabId') : x + // Filter out things which are already in our own list at a smaller index + // Filter out things which are already in the suggestions list + let filteredData = data.filter((site) => + suggestionsList.findIndex((x) => (x.location || '').toLowerCase() === (formatUrl(site) || '').toLowerCase()) === -1 || + // Tab autosuggestions should always be included since they will almost always be in history + type === suggestionTypes.TAB) + // Per suggestion provider filter + if (filterValue) { + filteredData = filteredData.filter(filterValue) + } + + return makeImmutable(filteredData + .sort(sortHandler) + .take(maxResults) + .map((site) => { + return Immutable.fromJS({ + title: formatTitle(site), + location: formatUrl(site), + tabId: formatTabId(site), + type + }) + })) +} + +const getHistorySuggestions = (state, urlLocationLower) => { + return new Promise((resolve, reject) => { + const sortHandler = sortBasedOnLocationPos(urlLocationLower) + const mapListToElements = getMapListToElements(urlLocationLower) + let suggestionsList = Immutable.List() + // NOTE: Iterating sites can take a long time! Please be mindful when + // working with the history and bookmark suggestion code. + const historySuggestionsOn = getSetting(settings.HISTORY_SUGGESTIONS) + const bookmarkSuggestionsOn = getSetting(settings.BOOKMARK_SUGGESTIONS) + const shouldIterateSites = historySuggestionsOn || bookmarkSuggestionsOn + if (shouldIterateSites) { + // Note: Bookmark sites are now included in history. This will allow + // sites to appear in the auto-complete regardless of their bookmark + // status. If history is turned off, bookmarked sites will appear + // in the bookmark section. + const sitesFilter = (site) => { + const location = site.get('location') + if (!location) { + return false + } + const title = site.get('title') + return location.toLowerCase().indexOf(urlLocationLower) !== -1 || + (title && title.toLowerCase().indexOf(urlLocationLower) !== -1) + } + + let historySites = new Immutable.List() + let bookmarkSites = new Immutable.List() + const sites = appStoreRenderer.state.get('sites') + sites.forEach(site => { + if (!sitesFilter(site)) { + return + } + if (historySuggestionsOn && isHistoryEntry(site) && !isDefaultEntry(site)) { + historySites = historySites.push(site) + return + } + if (bookmarkSuggestionsOn && isBookmark(site) && !isDefaultEntry(site)) { + bookmarkSites = bookmarkSites.push(site) + } + }) + + if (historySites.size > 0) { + historySites = historySites.concat(createVirtualHistoryItems(historySites)) + + suggestionsList = suggestionsList.concat(mapListToElements({ + data: historySites, + maxResults: config.urlBarSuggestions.maxHistorySites, + type: suggestionTypes.HISTORY, + sortHandler, + filterValue: null + })) + } + + if (bookmarkSites.size > 0) { + suggestionsList = suggestionsList.concat(mapListToElements({ + data: bookmarkSites, + maxResults: config.urlBarSuggestions.maxBookmarkSites, + type: suggestionTypes.BOOKMARK, + sortHandler, + filterValue: null + })) + } + } + resolve(suggestionsList) + }) +} + +const getAboutSuggestions = (state, urlLocationLower) => { + return new Promise((resolve, reject) => { + const mapListToElements = getMapListToElements(urlLocationLower) + const suggestionsList = mapListToElements({ + data: aboutUrls.keySeq().filter((x) => isNavigatableAboutPage(x)), + maxResults: config.urlBarSuggestions.maxAboutPages, + type: suggestionTypes.ABOUT_PAGES + }) + resolve(suggestionsList) + }) +} + +const getOpenedTabSuggestions = (state, urlLocationLower) => { + return new Promise((resolve, reject) => { + const sortHandler = sortBasedOnLocationPos(urlLocationLower) + const mapListToElements = getMapListToElements(urlLocationLower) + let suggestionsList = Immutable.List() + if (getSetting(settings.OPENED_TAB_SUGGESTIONS)) { + const activeFrameKey = state.get('activeFrameKey') + suggestionsList = mapListToElements({ + data: state.get('frames'), + maxResults: config.urlBarSuggestions.maxOpenedFrames, + type: suggestionTypes.TAB, + sortHandler, + filterValue: (frame) => !isSourceAboutUrl(frame.get('location')) && + frame.get('key') !== activeFrameKey && + ( + (frame.get('title') && frame.get('title').toLowerCase().indexOf(urlLocationLower) !== -1) || + (frame.get('location') && frame.get('location').toLowerCase().indexOf(urlLocationLower) !== -1) + ) + }) + } + resolve(suggestionsList) + }) +} + +const getSearchSuggestions = (state, urlLocationLower) => { + return new Promise((resolve, reject) => { + const mapListToElements = getMapListToElements(urlLocationLower) + let suggestionsList = Immutable.List() + if (getSetting(settings.OFFER_SEARCH_SUGGESTIONS)) { + const searchResults = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'searchResults'])) + if (searchResults) { + suggestionsList = mapListToElements({ + data: searchResults, + maxResults: config.urlBarSuggestions.maxSearch, + type: suggestionTypes.SEARCH + }) + } + } + resolve(suggestionsList) + }) +} + +const getAlexaSuggestions = (state, urlLocationLower) => { + return new Promise((resolve, reject) => { + const mapListToElements = getMapListToElements(urlLocationLower) + const suggestionsList = mapListToElements({ + data: top500, + maxResults: config.urlBarSuggestions.maxTopSites, + type: suggestionTypes.TOP_SITE + }) + resolve(suggestionsList) + }) +} + +const generateNewSuggestionsList = (state, urlLocation) => { + if (!urlLocation) { + return + } + const urlLocationLower = urlLocation.toLowerCase() + Promise.all([ + getHistorySuggestions(state, urlLocationLower), + getAboutSuggestions(state, urlLocationLower), + getAboutSuggestions(state, urlLocationLower), + getOpenedTabSuggestions(state, urlLocationLower), + getSearchSuggestions(state, urlLocationLower), + getAlexaSuggestions(state, urlLocationLower) + ]).then(([...suggestionsLists]) => { + const appActions = require('../../../js/actions/appActions') + // Flatten only 1 level deep for perf only, nested will be objects within arrrays + appActions.urlBarSuggestionsChanged(makeImmutable(suggestionsLists).flatten(1)) + }) +} + +module.exports = { + sortingPriority, + sortByAccessCountWithAgeDecay, + simpleDomainNameValue, + normalizeLocation, + shouldNormalizeLocation, + createVirtualHistoryItems, + sortBasedOnLocationPos, + getMapListToElements, + getHistorySuggestions, + getAboutSuggestions, + getOpenedTabSuggestions, + getSearchSuggestions, + getAlexaSuggestions, + generateNewSuggestionsList +} diff --git a/app/renderer/reducers/urlBarReducer.js b/app/renderer/reducers/urlBarReducer.js index 13423d9db72..2d8fa4cac04 100644 --- a/app/renderer/reducers/urlBarReducer.js +++ b/app/renderer/reducers/urlBarReducer.js @@ -6,21 +6,16 @@ const windowConstants = require('../../../js/constants/windowConstants') const appConstants = require('../../../js/constants/appConstants') -const {aboutUrls, isNavigatableAboutPage, isSourceAboutUrl, isUrl, getSourceAboutUrl, getSourceMagnetUrl} = require('../../../js/lib/appUrlUtil') +const {isUrl, getSourceAboutUrl, getSourceMagnetUrl} = require('../../../js/lib/appUrlUtil') const {isURL, isPotentialPhishingUrl, getUrlFromInput} = require('../../../js/lib/urlutil') const {getFrameByKey, getFrameKeyByTabId, activeFrameStatePath, frameStatePath, getActiveFrame, getFrameByTabId} = require('../../../js/state/frameStateUtil') const getSetting = require('../../../js/settings').getSetting -const {isBookmark, isDefaultEntry, isHistoryEntry} = require('../../../js/state/siteUtil') const fetchSearchSuggestions = require('../fetchSearchSuggestions') const searchProviders = require('../../../js/data/searchProviders') const settings = require('../../../js/constants/settings') const Immutable = require('immutable') -const config = require('../../../js/constants/config') -const top500 = require('../../../js/data/top500') -const suggestion = require('../lib/suggestion') -const suggestionTypes = require('../../../js/constants/suggestionTypes') +const {generateNewSuggestionsList} = require('../lib/suggestion') const {navigateSiteClickHandler} = require('../suggestionClickHandlers') -const appStoreRenderer = require('../../../js/stores/appStoreRenderer') const navigationBarState = require('../../common/state/navigationBarState') const tabState = require('../../common/state/tabState') @@ -116,11 +111,11 @@ const updateUrlSuffix = (state, suggestionList) => { if (autocompleteEnabled) { const location = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'location'])) || '' - const index = suggestion.location.toLowerCase().indexOf(location.toLowerCase()) + const index = suggestion.get('location').toLowerCase().indexOf(location.toLowerCase()) if (index !== -1) { - const beforePrefix = suggestion.location.substring(0, index) + const beforePrefix = suggestion.get('location').substring(0, index) if (beforePrefix.endsWith('://') || beforePrefix.endsWith('://www.') || index === 0) { - suffix = suggestion.location.substring(index + location.length) + suffix = suggestion.get('location').substring(index + location.length) } } } @@ -129,182 +124,6 @@ const updateUrlSuffix = (state, suggestionList) => { return state } -const generateNewSuggestionsList = (state) => { - const activeFrameKey = state.get('activeFrameKey') - const urlLocation = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'location'])) - const searchResults = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'searchResults'])) - - if (!urlLocation) { - return state - } - - const urlLocationLower = urlLocation.toLowerCase() - let suggestionsList = new Immutable.List() - const formatUrl = (x) => typeof x === 'object' && x !== null ? x.get('location') : x - const formatTitle = (x) => typeof x === 'object' && x !== null ? x.get('title') : x - const formatTabId = (x) => typeof x === 'object' && x !== null ? x.get('tabId') : x - const mapListToElements = ({data, maxResults, type, - sortHandler = (x) => x, filterValue = (site) => { - return site.toLowerCase().indexOf(urlLocationLower) !== -1 - } - }) => { - // Filter out things which are already in our own list at a smaller index - // Filter out things which are already in the suggestions list - let filteredData = data.filter((site) => - suggestionsList.findIndex((x) => (x.location || '').toLowerCase() === (formatUrl(site) || '').toLowerCase()) === -1 || - // Tab autosuggestions should always be included since they will almost always be in history - type === suggestionTypes.TAB) - // Per suggestion provider filter - if (filterValue) { - filteredData = filteredData.filter(filterValue) - } - - return filteredData - .sort(sortHandler) - .take(maxResults) - .map((site) => { - return { - title: formatTitle(site), - location: formatUrl(site), - tabId: formatTabId(site), - type - } - }) - } - - const shouldNormalize = suggestion.shouldNormalizeLocation(urlLocationLower) - const urlLocationLowerNormalized = suggestion.normalizeLocation(urlLocationLower) - const sortBasedOnLocationPos = (s1, s2) => { - const location1 = shouldNormalize ? suggestion.normalizeLocation(s1.get('location')) : s1.get('location') - const location2 = shouldNormalize ? suggestion.normalizeLocation(s2.get('location')) : s2.get('location') - const pos1 = location1.indexOf(urlLocationLowerNormalized) - const pos2 = location2.indexOf(urlLocationLowerNormalized) - if (pos1 === -1 && pos2 === -1) { - return 0 - } else if (pos1 === -1) { - return 1 - } else if (pos2 === -1) { - return -1 - } else { - if (pos1 - pos2 !== 0) { - return pos1 - pos2 - } else { - // sort site.com higher than site.com/somepath - const sdnv1 = suggestion.simpleDomainNameValue(s1) - const sdnv2 = suggestion.simpleDomainNameValue(s2) - if (sdnv1 !== sdnv2) { - return sdnv2 - sdnv1 - } else { - // If there's a tie on the match location, use the age - // decay modified access count - return suggestion.sortByAccessCountWithAgeDecay(s1, s2) - } - } - } - } - - // NOTE: Iterating sites can take a long time! Please be mindful when - // working with the history and bookmark suggestion code. - const historySuggestionsOn = getSetting(settings.HISTORY_SUGGESTIONS) - const bookmarkSuggestionsOn = getSetting(settings.BOOKMARK_SUGGESTIONS) - const shouldIterateSites = historySuggestionsOn || bookmarkSuggestionsOn - if (shouldIterateSites) { - // Note: Bookmark sites are now included in history. This will allow - // sites to appear in the auto-complete regardless of their bookmark - // status. If history is turned off, bookmarked sites will appear - // in the bookmark section. - const sitesFilter = (site) => { - const location = site.get('location') - if (!location) { - return false - } - const title = site.get('title') - return location.toLowerCase().indexOf(urlLocationLower) !== -1 || - (title && title.toLowerCase().indexOf(urlLocationLower) !== -1) - } - - let historySites = new Immutable.List() - let bookmarkSites = new Immutable.List() - const sites = appStoreRenderer.state.get('sites') - sites.forEach(site => { - if (!sitesFilter(site)) { - return - } - if (historySuggestionsOn && isHistoryEntry(site) && !isDefaultEntry(site)) { - historySites = historySites.push(site) - return - } - if (bookmarkSuggestionsOn && isBookmark(site) && !isDefaultEntry(site)) { - bookmarkSites = bookmarkSites.push(site) - } - }) - - if (historySites.size > 0) { - historySites = historySites.concat(suggestion.createVirtualHistoryItems(historySites)) - - suggestionsList = suggestionsList.concat(mapListToElements({ - data: historySites, - maxResults: config.urlBarSuggestions.maxHistorySites, - type: suggestionTypes.HISTORY, - sortHandler: sortBasedOnLocationPos, - filterValue: null - })) - } - - if (bookmarkSites.size > 0) { - suggestionsList = suggestionsList.concat(mapListToElements({ - data: bookmarkSites, - maxResults: config.urlBarSuggestions.maxBookmarkSites, - type: suggestionTypes.BOOKMARK, - sortHandler: sortBasedOnLocationPos, - filterValue: null - })) - } - } - - // about pages - suggestionsList = suggestionsList.concat(mapListToElements({ - data: aboutUrls.keySeq().filter((x) => isNavigatableAboutPage(x)), - maxResults: config.urlBarSuggestions.maxAboutPages, - type: suggestionTypes.ABOUT_PAGES - })) - - // opened frames - if (getSetting(settings.OPENED_TAB_SUGGESTIONS)) { - suggestionsList = suggestionsList.concat(mapListToElements({ - data: state.get('frames'), - maxResults: config.urlBarSuggestions.maxOpenedFrames, - type: suggestionTypes.TAB, - sortHandler: sortBasedOnLocationPos, - filterValue: (frame) => !isSourceAboutUrl(frame.get('location')) && - frame.get('key') !== activeFrameKey && - ( - (frame.get('title') && frame.get('title').toLowerCase().indexOf(urlLocationLower) !== -1) || - (frame.get('location') && frame.get('location').toLowerCase().indexOf(urlLocationLower) !== -1) - ) - })) - } - - // Search suggestions - if (getSetting(settings.OFFER_SEARCH_SUGGESTIONS) && searchResults) { - suggestionsList = suggestionsList.concat(mapListToElements({ - data: searchResults, - maxResults: config.urlBarSuggestions.maxSearch, - type: suggestionTypes.SEARCH - })) - } - - // Alexa top 500 - suggestionsList = suggestionsList.concat(mapListToElements({ - data: top500, - maxResults: config.urlBarSuggestions.maxTopSites, - type: suggestionTypes.TOP_SITE - })) - - const appActions = require('../../../js/actions/appActions') - appActions.urlBarSuggestionsChanged(suggestionsList) -} - const getLocation = (location) => { location = location.trim() location = getSourceAboutUrl(location) || @@ -347,8 +166,9 @@ const setNavBarUserInput = (state, location) => { const activeFrameProps = getActiveFrame(state) state = updateSearchEngineInfoFromInput(state, activeFrameProps) state = searchXHR(state, activeFrameProps, true) + const urlLocation = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'location'])) setImmediate(() => { - generateNewSuggestionsList(state) + generateNewSuggestionsList(state, urlLocation) }) if (!location) { state = setRenderUrlBarSuggestions(state, false) @@ -488,8 +308,9 @@ const urlBarReducer = (state, action) => { case windowConstants.WINDOW_SEARCH_SUGGESTION_RESULTS_AVAILABLE: const frameKey = getFrameKeyByTabId(state, action.tabId) state = state.setIn(frameStatePath(state, frameKey).concat(['navbar', 'urlbar', 'suggestions', 'searchResults']), action.searchResults) + const urlLocation = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'location'])) setImmediate(() => { - generateNewSuggestionsList(state) + generateNewSuggestionsList(state, urlLocation) }) break case windowConstants.WINDOW_URL_BAR_AUTOCOMPLETE_ENABLED: From c215ab396f4fd0f2b20b6482f47542bd2ebe0b1f Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Fri, 12 May 2017 14:11:56 -0400 Subject: [PATCH 06/22] Make suggestions independent of windowState --- app/renderer/fetchSearchSuggestions.js | 6 +- app/renderer/lib/suggestion.js | 88 +++++-- app/renderer/reducers/urlBarReducer.js | 54 +--- docs/appActions.md | 13 + docs/windowActions.md | 13 - js/actions/appActions.js | 245 ++++++++++-------- js/actions/windowActions.js | 22 +- js/constants/appConstants.js | 1 + js/constants/windowConstants.js | 1 - js/dispatcher/appDispatcher.js | 1 + .../renderer/reducers/urlBarReducerTest.js | 4 +- 11 files changed, 229 insertions(+), 219 deletions(-) diff --git a/app/renderer/fetchSearchSuggestions.js b/app/renderer/fetchSearchSuggestions.js index 13402756dc0..caa8f281b11 100644 --- a/app/renderer/fetchSearchSuggestions.js +++ b/app/renderer/fetchSearchSuggestions.js @@ -3,10 +3,10 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const Immutable = require('immutable') -const windowActions = require('../../js/actions/windowActions') +const appActions = require('../../js/actions/appActions') const debounce = require('../../js/lib/debounce') -const fetchSearchSuggestions = debounce((tabId, autocompleteURL, searchTerms) => { +const fetchSearchSuggestions = debounce((windowId, tabId, autocompleteURL, searchTerms) => { const xhr = new window.XMLHttpRequest() xhr.open('GET', autocompleteURL .replace('{searchTerms}', encodeURIComponent(searchTerms)), true) @@ -14,7 +14,7 @@ const fetchSearchSuggestions = debounce((tabId, autocompleteURL, searchTerms) => xhr.send() xhr.onload = () => { // Once we have the online suggestions, append them to the others - windowActions.searchSuggestionResultsAvailable(tabId, Immutable.fromJS(xhr.response[1])) + appActions.searchSuggestionResultsAvailable(windowId, tabId, Immutable.fromJS(xhr.response[1])) } }, 100) diff --git a/app/renderer/lib/suggestion.js b/app/renderer/lib/suggestion.js index 2a0fe8fc81b..502985fbfb7 100644 --- a/app/renderer/lib/suggestion.js +++ b/app/renderer/lib/suggestion.js @@ -7,15 +7,15 @@ const appConfig = require('../../../js/constants/appConfig') const _ = require('underscore') const Immutable = require('immutable') const {makeImmutable} = require('../../common/state/immutableUtil') -const {aboutUrls, isNavigatableAboutPage, isSourceAboutUrl} = require('../../../js/lib/appUrlUtil') +const {isUrl, aboutUrls, isNavigatableAboutPage, isSourceAboutUrl} = require('../../../js/lib/appUrlUtil') const suggestionTypes = require('../../../js/constants/suggestionTypes') const getSetting = require('../../../js/settings').getSetting const settings = require('../../../js/constants/settings') -const appStoreRenderer = require('../../../js/stores/appStoreRenderer') const {isBookmark, isDefaultEntry, isHistoryEntry} = require('../../../js/state/siteUtil') const config = require('../../../js/constants/config') const top500 = require('../../../js/data/top500') -const {activeFrameStatePath} = require('../../../js/state/frameStateUtil') +const fetchSearchSuggestions = require('../fetchSearchSuggestions') +const {getFrameByTabId, getTabsByWindowId} = require('../../common/state/tabState') const sigmoid = (t) => { return 1 / (1 + Math.pow(Math.E, -t)) @@ -105,7 +105,7 @@ const sortByAccessCountWithAgeDecay = (s1, s2) => { * */ const simpleDomainNameValue = (site) => { - const parsed = urlParse(site.get('location')) + const parsed = urlParse(getURL(site)) if (parsed.hash === null && parsed.search === null && parsed.query === null && parsed.pathname === '/') { return 1 } else { @@ -222,8 +222,8 @@ const sortBasedOnLocationPos = (urlLocationLower) => (s1, s2) => { const shouldNormalize = shouldNormalizeLocation(urlLocationLower) const urlLocationLowerNormalized = normalizeLocation(urlLocationLower) - const location1 = shouldNormalize ? normalizeLocation(s1.get('location')) : s1.get('location') - const location2 = shouldNormalize ? normalizeLocation(s2.get('location')) : s2.get('location') + const location1 = shouldNormalize ? normalizeLocation(getURL(s1)) : getURL(s1) + const location2 = shouldNormalize ? normalizeLocation(getURL(s2)) : getURL(s2) const pos1 = location1.indexOf(urlLocationLowerNormalized) const pos2 = location2.indexOf(urlLocationLowerNormalized) if (pos1 === -1 && pos2 === -1) { @@ -250,19 +250,20 @@ const sortBasedOnLocationPos = (urlLocationLower) => (s1, s2) => { } } +const getURL = (x) => typeof x === 'object' && x !== null ? x.get('location') || x.get('url') : x + const getMapListToElements = (urlLocationLower) => ({data, maxResults, type, sortHandler = (x) => x, filterValue = (site) => { return site.toLowerCase().indexOf(urlLocationLower) !== -1 } }) => { const suggestionsList = Immutable.List() - const formatUrl = (x) => typeof x === 'object' && x !== null ? x.get('location') : x const formatTitle = (x) => typeof x === 'object' && x !== null ? x.get('title') : x const formatTabId = (x) => typeof x === 'object' && x !== null ? x.get('tabId') : x // Filter out things which are already in our own list at a smaller index // Filter out things which are already in the suggestions list let filteredData = data.filter((site) => - suggestionsList.findIndex((x) => (x.location || '').toLowerCase() === (formatUrl(site) || '').toLowerCase()) === -1 || + suggestionsList.findIndex((x) => (x.location || '').toLowerCase() === (getURL(site) || '').toLowerCase()) === -1 || // Tab autosuggestions should always be included since they will almost always be in history type === suggestionTypes.TAB) // Per suggestion provider filter @@ -276,7 +277,7 @@ const getMapListToElements = (urlLocationLower) => ({data, maxResults, type, .map((site) => { return Immutable.fromJS({ title: formatTitle(site), - location: formatUrl(site), + location: getURL(site), tabId: formatTabId(site), type }) @@ -310,7 +311,7 @@ const getHistorySuggestions = (state, urlLocationLower) => { let historySites = new Immutable.List() let bookmarkSites = new Immutable.List() - const sites = appStoreRenderer.state.get('sites') + const sites = state.get('sites') sites.forEach(site => { if (!sitesFilter(site)) { return @@ -362,23 +363,23 @@ const getAboutSuggestions = (state, urlLocationLower) => { }) } -const getOpenedTabSuggestions = (state, urlLocationLower) => { +const getOpenedTabSuggestions = (state, windowId, urlLocationLower) => { return new Promise((resolve, reject) => { const sortHandler = sortBasedOnLocationPos(urlLocationLower) const mapListToElements = getMapListToElements(urlLocationLower) + const tabs = getTabsByWindowId(state, windowId) let suggestionsList = Immutable.List() if (getSetting(settings.OPENED_TAB_SUGGESTIONS)) { - const activeFrameKey = state.get('activeFrameKey') suggestionsList = mapListToElements({ - data: state.get('frames'), + data: tabs, maxResults: config.urlBarSuggestions.maxOpenedFrames, type: suggestionTypes.TAB, sortHandler, - filterValue: (frame) => !isSourceAboutUrl(frame.get('location')) && - frame.get('key') !== activeFrameKey && + filterValue: (tab) => !isSourceAboutUrl(tab.get('url')) && + !tab.get('active') && ( - (frame.get('title') && frame.get('title').toLowerCase().indexOf(urlLocationLower) !== -1) || - (frame.get('location') && frame.get('location').toLowerCase().indexOf(urlLocationLower) !== -1) + (tab.get('title') && tab.get('title').toLowerCase().indexOf(urlLocationLower) !== -1) || + (tab.get('url') && tab.get('url').toLowerCase().indexOf(urlLocationLower) !== -1) ) }) } @@ -386,12 +387,17 @@ const getOpenedTabSuggestions = (state, urlLocationLower) => { }) } -const getSearchSuggestions = (state, urlLocationLower) => { +const getSearchSuggestions = (state, tabId, urlLocationLower) => { return new Promise((resolve, reject) => { const mapListToElements = getMapListToElements(urlLocationLower) let suggestionsList = Immutable.List() if (getSetting(settings.OFFER_SEARCH_SUGGESTIONS)) { - const searchResults = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'searchResults'])) + const frame = getFrameByTabId(state, tabId) + if (!frame) { + resolve(suggestionsList) + return + } + const searchResults = frame.getIn(['navbar', 'urlbar', 'suggestions', 'searchResults']) if (searchResults) { suggestionsList = mapListToElements({ data: searchResults, @@ -416,7 +422,7 @@ const getAlexaSuggestions = (state, urlLocationLower) => { }) } -const generateNewSuggestionsList = (state, urlLocation) => { +const generateNewSuggestionsList = (state, windowId, tabId, urlLocation) => { if (!urlLocation) { return } @@ -424,9 +430,8 @@ const generateNewSuggestionsList = (state, urlLocation) => { Promise.all([ getHistorySuggestions(state, urlLocationLower), getAboutSuggestions(state, urlLocationLower), - getAboutSuggestions(state, urlLocationLower), - getOpenedTabSuggestions(state, urlLocationLower), - getSearchSuggestions(state, urlLocationLower), + getOpenedTabSuggestions(state, windowId, urlLocationLower), + getSearchSuggestions(state, tabId, urlLocationLower), getAlexaSuggestions(state, urlLocationLower) ]).then(([...suggestionsLists]) => { const appActions = require('../../../js/actions/appActions') @@ -435,6 +440,40 @@ const generateNewSuggestionsList = (state, urlLocation) => { }) } +const generateNewSearchXHRResults = (state, windowId, tabId, input) => { + const frame = getFrameByTabId(state, tabId) + if (!frame) { + // Frame info may not be available yet in app store + return + } + const frameSearchDetail = frame.getIn(['navbar', 'urlbar', 'searchDetail']) + // TODO: Migrate searchDetail to app state + const searchDetail = state.get('searchDetail') + if (!searchDetail && !frameSearchDetail) { + return + } + const autocompleteURL = frameSearchDetail + ? frameSearchDetail.get('autocomplete') + : searchDetail.get('autocompleteURL') + + const shouldDoSearchSuggestions = getSetting(settings.OFFER_SEARCH_SUGGESTIONS) && + autocompleteURL && + !isUrl(input) && + input.length !== 0 + + if (shouldDoSearchSuggestions) { + if (searchDetail) { + const replaceRE = new RegExp('^' + searchDetail.get('shortcut') + ' ', 'g') + input = input.replace(replaceRE, '') + } + + fetchSearchSuggestions(windowId, tabId, autocompleteURL, input) + } else { + const appActions = require('../../../js/actions/appActions') + appActions.searchSuggestionResultsAvailable(windowId, tabId, Immutable.List()) + } +} + module.exports = { sortingPriority, sortByAccessCountWithAgeDecay, @@ -449,5 +488,6 @@ module.exports = { getOpenedTabSuggestions, getSearchSuggestions, getAlexaSuggestions, - generateNewSuggestionsList + generateNewSuggestionsList, + generateNewSearchXHRResults } diff --git a/app/renderer/reducers/urlBarReducer.js b/app/renderer/reducers/urlBarReducer.js index 2d8fa4cac04..7ab777c410e 100644 --- a/app/renderer/reducers/urlBarReducer.js +++ b/app/renderer/reducers/urlBarReducer.js @@ -9,13 +9,12 @@ const appConstants = require('../../../js/constants/appConstants') const {isUrl, getSourceAboutUrl, getSourceMagnetUrl} = require('../../../js/lib/appUrlUtil') const {isURL, isPotentialPhishingUrl, getUrlFromInput} = require('../../../js/lib/urlutil') const {getFrameByKey, getFrameKeyByTabId, activeFrameStatePath, frameStatePath, getActiveFrame, getFrameByTabId} = require('../../../js/state/frameStateUtil') -const getSetting = require('../../../js/settings').getSetting -const fetchSearchSuggestions = require('../fetchSearchSuggestions') const searchProviders = require('../../../js/data/searchProviders') -const settings = require('../../../js/constants/settings') const Immutable = require('immutable') -const {generateNewSuggestionsList} = require('../lib/suggestion') +const {generateNewSuggestionsList, generateNewSearchXHRResults} = require('../lib/suggestion') const {navigateSiteClickHandler} = require('../suggestionClickHandlers') +const {getCurrentWindowId} = require('../currentWindow') +const appStoreRenderer = require('../../../js/stores/appStoreRenderer') const navigationBarState = require('../../common/state/navigationBarState') const tabState = require('../../common/state/tabState') @@ -42,36 +41,6 @@ const updateSearchEngineInfoFromInput = (state, frameProps) => { return state } -const searchXHR = (state, frameProps, searchOnline) => { - const searchDetail = state.get('searchDetail') - const frameSearchDetail = frameProps.getIn(['navbar', 'urlbar', 'searchDetail']) - if (!searchDetail && !frameSearchDetail) { - return state - } - let autocompleteURL = frameSearchDetail - ? frameSearchDetail.get('autocomplete') - : searchDetail.get('autocompleteURL') - if (!getSetting(settings.OFFER_SEARCH_SUGGESTIONS) || !autocompleteURL) { - state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'searchResults']), Immutable.fromJS([])) - return state - } - - let input = frameProps.getIn(['navbar', 'urlbar', 'location']) - if (!isUrl(input) && input.length > 0) { - if (searchDetail) { - const replaceRE = new RegExp('^' + searchDetail.get('shortcut') + ' ', 'g') - input = input.replace(replaceRE, '') - } - - if (searchOnline) { - fetchSearchSuggestions(frameProps.get('tabId'), autocompleteURL, input) - } - } else { - state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'searchResults']), Immutable.fromJS([])) - } - return state -} - const setUrlSuggestions = (state, suggestionList) => { if (suggestionList !== undefined) { state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'suggestionList']), suggestionList) @@ -165,11 +134,15 @@ const setNavBarUserInput = (state, location) => { state = updateNavBarInput(state, location) const activeFrameProps = getActiveFrame(state) state = updateSearchEngineInfoFromInput(state, activeFrameProps) - state = searchXHR(state, activeFrameProps, true) const urlLocation = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'location'])) - setImmediate(() => { - generateNewSuggestionsList(state, urlLocation) - }) + const windowId = getCurrentWindowId() + const tabId = activeFrameProps.get('tabId') + if (tabId) { + setImmediate(() => { + generateNewSearchXHRResults(appStoreRenderer.state, windowId, tabId, urlLocation) + generateNewSuggestionsList(appStoreRenderer.state, windowId, tabId, urlLocation) + }) + } if (!location) { state = setRenderUrlBarSuggestions(state, false) } @@ -190,7 +163,6 @@ const setActive = (state, isActive) => { const urlBarReducer = (state, action) => { const tabId = state.getIn(activeFrameStatePath(state).concat(['tabId']), tabState.TAB_ID_NONE) - switch (action.actionType) { case appConstants.APP_URL_BAR_TEXT_CHANGED: state = setNavBarUserInput(state, action.input) @@ -305,12 +277,12 @@ const urlBarReducer = (state, action) => { state = updateUrlSuffix(state, state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'suggestionList']), suggestionList)) break } - case windowConstants.WINDOW_SEARCH_SUGGESTION_RESULTS_AVAILABLE: + case appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE: const frameKey = getFrameKeyByTabId(state, action.tabId) state = state.setIn(frameStatePath(state, frameKey).concat(['navbar', 'urlbar', 'suggestions', 'searchResults']), action.searchResults) const urlLocation = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'location'])) setImmediate(() => { - generateNewSuggestionsList(state, urlLocation) + generateNewSuggestionsList(appStoreRenderer.state, getCurrentWindowId(), action.tabId, urlLocation) }) break case windowConstants.WINDOW_URL_BAR_AUTOCOMPLETE_ENABLED: diff --git a/docs/appActions.md b/docs/appActions.md index f13421222ce..0f18c2a960b 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -1067,6 +1067,19 @@ Indicates that the urlbar text has changed, usually from user input +### searchSuggestionResultsAvailable(tabId, searchResults) + +New URL bar suggestion search results are available. +This is typically from a service like Duck Duck Go auto complete for the portion of text that the user typed in. + +**Parameters** + +**tabId**: `number`, the tab id for the action + +**searchResults**: , The search results for the currently entered URL bar text. + + + ### urlBarSuggestionsChanged(suggestionList, selectedIndex) Indicates URL bar suggestions and selected index. diff --git a/docs/windowActions.md b/docs/windowActions.md index 46f95910a44..bbe8e68e19d 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -319,19 +319,6 @@ This is sometimes only temporarily disabled, e.g. a user is pressing backspace. -### searchSuggestionResultsAvailable(tabId, searchResults) - -New URL bar suggestion search results are available. -This is typically from a service like Duck Duck Go auto complete for the portion of text that the user typed in. - -**Parameters** - -**tabId**: `number`, the tab id for the action - -**searchResults**: , The search results for the currently entered URL bar text. - - - ### setUrlBarSelected(isSelected) Marks the URL bar text as selected or not diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 0747b85f7ce..37533995420 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -3,7 +3,7 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ 'use strict' -const AppDispatcher = require('../dispatcher/appDispatcher') +const {dispatch} = require('../dispatcher/appDispatcher') const appConstants = require('../constants/appConstants') const appActions = { @@ -14,7 +14,7 @@ const appActions = { * @param {object} appState - Initial app state object (not yet converted to ImmutableJS) */ setState: function (appState) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_STATE, appState }) @@ -28,7 +28,7 @@ const appActions = { * @param {function} cb - Callback to call after the window is loaded, will only work if called from the main process. */ newWindow: function (frameOpts, browserOpts, restoredState, cb) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_NEW_WINDOW, frameOpts, browserOpts, @@ -38,35 +38,35 @@ const appActions = { }, windowReady: function (windowId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_WINDOW_READY, windowId }) }, closeWindow: function (windowId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_CLOSE_WINDOW, windowId }) }, windowClosed: function (windowValue) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_WINDOW_CLOSED, windowValue }) }, windowCreated: function (windowValue) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_WINDOW_CREATED, windowValue }) }, windowUpdated: function (windowValue) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_WINDOW_UPDATED, windowValue }) @@ -77,7 +77,7 @@ const appActions = { * @param {Object} frame */ frameChanged: function (frame) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_FRAME_CHANGED, frame }) @@ -88,7 +88,7 @@ const appActions = { * @param {Object} tabValue */ tabCreated: function (tabValue) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_CREATED, tabValue }) @@ -102,7 +102,7 @@ const appActions = { * @param {Number} windowId */ tabMoved: function (tabId, frameOpts, browserOpts, windowId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_MOVED, tabId, frameOpts, @@ -116,7 +116,7 @@ const appActions = { * @param {Object} createProperties */ createTabRequested: function (createProperties, activateIfOpen = false, isRestore = false) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_CREATE_TAB_REQUESTED, createProperties, activateIfOpen, @@ -130,7 +130,7 @@ const appActions = { * @param {string} url - The url to load */ loadURLRequested: function (tabId, url) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_LOAD_URL_REQUESTED, tabId, url @@ -143,7 +143,7 @@ const appActions = { * @param {string} url - The url to load */ loadURLInActiveTabRequested: function (windowId, url) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_LOAD_URL_IN_ACTIVE_TAB_REQUESTED, windowId, url @@ -156,7 +156,7 @@ const appActions = { * @param {string} shareType - The type of share to do, must be one of: "email", "facebook", "pinterest", "twitter", "googlePlus", "linkedIn", "buffer", "reddit", or "digg" */ simpleShareActiveTabRequested: function (windowId, shareType) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SIMPLE_SHARE_ACTIVE_TAB_REQUESTED, windowId, shareType @@ -169,7 +169,7 @@ const appActions = { * @param {Object} changeInfo from chrome-tabs-updated */ tabUpdated: function (tabValue, changeInfo) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_UPDATED, tabValue, changeInfo, @@ -185,7 +185,7 @@ const appActions = { * @param {Number} tabId - the tabId to activate */ tabActivateRequested: function (tabId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_ACTIVATE_REQUESTED, tabId }) @@ -212,7 +212,7 @@ const appActions = { * @param {Boolean} forceClosePinned - force close if pinned */ tabCloseRequested: function (tabId, forceClosePinned = false) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_CLOSE_REQUESTED, tabId, forceClosePinned @@ -224,7 +224,7 @@ const appActions = { * @param {number} tabId */ tabClosed: function (tabId, windowId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_CLOSED, tabId, queryInfo: { @@ -243,7 +243,7 @@ const appActions = { * @param {boolean} skipSync - Set true if a site isn't eligible for Sync (e.g. if addSite was triggered by Sync) */ addSite: function (siteDetail, tag, originalSiteDetail, destinationDetail, skipSync) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ADD_SITE, siteDetail, tag, @@ -260,7 +260,7 @@ const appActions = { * @param {boolean} skipSync - Set true if a site isn't eligible for Sync (e.g. if this removal was triggered by Sync) */ removeSite: function (siteDetail, tag, skipSync) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_REMOVE_SITE, siteDetail, tag, @@ -278,7 +278,7 @@ const appActions = { * @param {boolean} destinationIsParent - Whether or not the destinationDetail should be considered the new parent. */ moveSite: function (sourceKey, destinationKey, prepend, destinationIsParent) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_MOVE_SITE, sourceKey, destinationKey, @@ -294,7 +294,7 @@ const appActions = { * @param {Object} downloadDetail - Properties for the download */ mergeDownloadDetail: function (downloadId, downloadDetail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_MERGE_DOWNLOAD_DETAIL, downloadId, downloadDetail @@ -305,7 +305,7 @@ const appActions = { * Dispatches a message to clear all completed downloads */ clearCompletedDownloads: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_CLEAR_COMPLETED_DOWNLOADS }) }, @@ -314,7 +314,7 @@ const appActions = { * Dispatches a message indicating ledger recovery succeeded */ ledgerRecoverySucceeded: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_LEDGER_RECOVERY_STATUS_CHANGED, recoverySucceeded: true }) @@ -324,7 +324,7 @@ const appActions = { * Dispatches a message indicating ledger recovery failed */ ledgerRecoveryFailed: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_LEDGER_RECOVERY_STATUS_CHANGED, recoverySucceeded: false }) @@ -336,7 +336,7 @@ const appActions = { * @param {Array} position - [x, y] */ defaultWindowParamsChanged: function (size, position) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DEFAULT_WINDOW_PARAMS_CHANGED, size, position @@ -351,7 +351,7 @@ const appActions = { * @param {string} etag - The etag of the reosurce from the http response */ setResourceETag: function (resourceName, etag) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_DATA_FILE_ETAG, resourceName, etag @@ -364,7 +364,7 @@ const appActions = { * @param {number} lastCheck - The last check date of the reosurce from the http response */ setResourceLastCheck: function (resourceName, lastCheckVersion, lastCheckDate) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_DATA_FILE_LAST_CHECK, resourceName, lastCheckVersion, @@ -378,7 +378,7 @@ const appActions = { * @param {boolean} enabled - true if the resource is enabled. */ setResourceEnabled: function (resourceName, enabled) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_RESOURCE_ENABLED, resourceName, enabled @@ -390,7 +390,7 @@ const appActions = { * @param {string} resourceName - 'widevine' */ resourceReady: function (resourceName) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_RESOURCE_READY, resourceName }) @@ -402,7 +402,7 @@ const appActions = { * @param {number} count - number of blocked resources to add to the global count */ addResourceCount: function (resourceName, count) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ADD_RESOURCE_COUNT, resourceName, count @@ -414,7 +414,7 @@ const appActions = { * epoch timestamp (milliseconds) */ setUpdateLastCheck: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_UPDATE_LAST_CHECK }) }, @@ -426,7 +426,7 @@ const appActions = { * @param {object} metadata - Metadata from the pdate server, with info like release notes. */ setUpdateStatus: function (status, verbose, metadata) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_UPDATE_STATUS, status, verbose, @@ -440,7 +440,7 @@ const appActions = { * @param {string} value - The value of the setting */ changeSetting: function (key, value) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_CHANGE_SETTING, key, value @@ -457,7 +457,7 @@ const appActions = { * @param {boolean} skipSync - Set true if a site isn't eligible for Sync (e.g. if addSite was triggered by Sync) */ changeSiteSetting: function (hostPattern, key, value, temp, skipSync) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_CHANGE_SITE_SETTING, hostPattern, key, @@ -476,7 +476,7 @@ const appActions = { * @param {boolean} skipSync - Set true if a site isn't eligible for Sync (e.g. if addSite was triggered by Sync) */ removeSiteSetting: function (hostPattern, key, temp, skipSync) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_REMOVE_SITE_SETTING, hostPattern, key, @@ -490,7 +490,7 @@ const appActions = { * @param {object} ledgerInfo - the current ledger state */ updateLedgerInfo: function (ledgerInfo) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_UPDATE_LEDGER_INFO, ledgerInfo }) @@ -501,7 +501,7 @@ const appActions = { * @param {object} locationInfo - the current location synopsis */ updateLocationInfo: function (locationInfo) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_UPDATE_LOCATION_INFO, locationInfo }) @@ -512,7 +512,7 @@ const appActions = { * @param {object} publisherInfo - the current publisher synopsis */ updatePublisherInfo: function (publisherInfo) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_UPDATE_PUBLISHER_INFO, publisherInfo }) @@ -523,7 +523,7 @@ const appActions = { * @param {{message: string, buttons: Array., frameOrigin: string, options: Object}} detail */ showNotification: function (detail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SHOW_NOTIFICATION, detail }) @@ -534,7 +534,7 @@ const appActions = { * @param {string} message */ hideNotification: function (message) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_HIDE_NOTIFICATION, message }) @@ -546,7 +546,7 @@ const appActions = { * @param {boolean} learn - true if the word should be learned, false if ignored */ addWord: function (word, learn) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ADD_WORD, word, learn @@ -558,7 +558,7 @@ const appActions = { * @param {string} locale - The locale to set for the dictionary */ setDictionary: function (locale) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_DICTIONARY, locale }) @@ -570,7 +570,7 @@ const appActions = { * @param {string} detail - login request info */ setLoginRequiredDetail: function (tabId, detail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_LOGIN_REQUIRED_DETAIL, tabId, detail @@ -578,7 +578,7 @@ const appActions = { }, setLoginResponseDetail: function (tabId, detail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_LOGIN_RESPONSE_DETAIL, tabId, detail @@ -590,7 +590,7 @@ const appActions = { * @param {object} clearDataDetail - the app data to clear as per doc/state.md's clearBrowsingDataDefaults */ onClearBrowsingData: function (clearDataDetail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ON_CLEAR_BROWSING_DATA, clearDataDetail }) @@ -601,7 +601,7 @@ const appActions = { * @param {object} selected - the browser data to import as per doc/state.md's importBrowserDataSelected */ importBrowserData: function (selected) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_IMPORT_BROWSER_DATA, selected }) @@ -613,7 +613,7 @@ const appActions = { * @param {object} originalDetail - the original address before editing */ addAutofillAddress: function (detail, originalDetail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ADD_AUTOFILL_ADDRESS, detail, originalDetail @@ -625,7 +625,7 @@ const appActions = { * @param {object} detail - the address to remove as per doc/state.md's autofillAddressDetail */ removeAutofillAddress: function (detail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_REMOVE_AUTOFILL_ADDRESS, detail }) @@ -637,7 +637,7 @@ const appActions = { * @param {object} originalDetail - the original credit card before editing */ addAutofillCreditCard: function (detail, originalDetail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ADD_AUTOFILL_CREDIT_CARD, detail, originalDetail @@ -649,7 +649,7 @@ const appActions = { * @param {object} detail - the credit card to remove as per doc/state.md's autofillCreditCardDetail */ removeAutofillCreditCard: function (detail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_REMOVE_AUTOFILL_CREDIT_CARD, detail }) @@ -661,7 +661,7 @@ const appActions = { * @param {Array} creditCardGuids - the guid array to access credit card entries in autofill DB */ autofillDataChanged: function (addressGuids, creditCardGuids) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_AUTOFILL_DATA_CHANGED, addressGuids, creditCardGuids @@ -674,7 +674,7 @@ const appActions = { * @param {Number} windowId - the unique id of the window */ windowBlurred: function (windowId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_WINDOW_BLURRED, windowId: windowId }) @@ -686,7 +686,7 @@ const appActions = { * @param {Number} windowId - the unique id of the window */ windowFocused: function (windowId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_WINDOW_FOCUSED, windowId: windowId }) @@ -697,7 +697,7 @@ const appActions = { * @param {Object} menubarTemplate - JSON used to build the menu */ setMenubarTemplate: function (menubarTemplate) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_MENUBAR_TEMPLATE, menubarTemplate }) @@ -708,7 +708,7 @@ const appActions = { * after being disconnected */ networkConnected: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_NETWORK_CONNECTED }) }, @@ -717,7 +717,7 @@ const appActions = { * Dispatches a message when the network is disconnected */ networkDisconnected: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_NETWORK_DISCONNECTED }) }, @@ -728,7 +728,7 @@ const appActions = { * @param {boolean} useBrave - whether set Brave as default browser */ defaultBrowserUpdated: function (useBrave) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DEFAULT_BROWSER_UPDATED, useBrave }) @@ -738,7 +738,7 @@ const appActions = { * Dispatch a message to indicate default browser check is complete */ defaultBrowserCheckComplete: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DEFAULT_BROWSER_CHECK_COMPLETE }) }, @@ -747,13 +747,13 @@ const appActions = { * Notify the AppStore to provide default history values. */ populateHistory: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_POPULATE_HISTORY }) }, allowFlashOnce: function (tabId, url, isPrivate) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ALLOW_FLASH_ONCE, tabId, url, @@ -762,7 +762,7 @@ const appActions = { }, allowFlashAlways: function (tabId, url) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ALLOW_FLASH_ALWAYS, tabId, url @@ -773,7 +773,7 @@ const appActions = { * Dispatch a message to copy data URL to clipboard **/ dataURLCopied: function (dataURL, html, text) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DATA_URL_COPIED, dataURL, html, @@ -785,7 +785,7 @@ const appActions = { * Dispatches a message when the app is shutting down. */ shuttingDown: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SHUTTING_DOWN }) }, @@ -796,7 +796,7 @@ const appActions = { * @param {string} downloadId - ID of the download being revealed */ downloadRevealed: function (downloadId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DOWNLOAD_REVEALED, downloadId }) @@ -807,7 +807,7 @@ const appActions = { * @param {string} downloadId - ID of the download being opened */ downloadOpened: function (downloadId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DOWNLOAD_OPENED, downloadId }) @@ -819,7 +819,7 @@ const appActions = { * @param {string} downloadAction - the action to perform from constants/electronDownloadItemActions.js */ downloadActionPerformed: function (downloadId, downloadAction) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DOWNLOAD_ACTION_PERFORMED, downloadId, downloadAction @@ -831,7 +831,7 @@ const appActions = { * @param {string} downloadId - ID of the download item being copied to the clipboard */ downloadCopiedToClipboard: function (downloadId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DOWNLOAD_COPIED_TO_CLIPBOARD, downloadId }) @@ -842,7 +842,7 @@ const appActions = { * @param {string} downloadId - ID of the download item being deleted */ downloadDeleted: function (downloadId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DOWNLOAD_DELETED, downloadId }) @@ -853,7 +853,7 @@ const appActions = { * @param {string} downloadId - ID of the download item being cleared */ downloadCleared: function (downloadId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DOWNLOAD_CLEARED, downloadId }) @@ -864,7 +864,7 @@ const appActions = { * @param {string} downloadId - ID of the download item being redownloaded */ downloadRedownloaded: function (downloadId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DOWNLOAD_REDOWNLOADED, downloadId }) @@ -874,7 +874,7 @@ const appActions = { * Shows delete confirmation bar in download item panel */ showDownloadDeleteConfirmation: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SHOW_DOWNLOAD_DELETE_CONFIRMATION }) }, @@ -883,7 +883,7 @@ const appActions = { * Hides delete confirmation bar in download item panel */ hideDownloadDeleteConfirmation: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_HIDE_DOWNLOAD_DELETE_CONFIRMATION }) }, @@ -893,7 +893,7 @@ const appActions = { * @param {string} text - clipboard text which is copied */ clipboardTextCopied: function (text) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_CLIPBOARD_TEXT_UPDATED, text }) @@ -904,7 +904,7 @@ const appActions = { * @param {number} tabId - The tabId */ toggleDevTools: function (tabId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_TOGGLE_DEV_TOOLS, tabId }) @@ -916,7 +916,7 @@ const appActions = { * @param {object} options - object containing options such as acive, back, and forward booleans */ tabCloned: function (tabId, options) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_CLONED, tabId, options @@ -930,7 +930,7 @@ const appActions = { * @param {boolean} temporary */ noScriptExceptionsAdded: function (hostPattern, origins, temporary) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ADD_NOSCRIPT_EXCEPTIONS, hostPattern, origins, @@ -944,7 +944,7 @@ const appActions = { * @param {Array.} objectPath */ setObjectId: function (objectId, objectPath) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_OBJECT_ID, objectId, objectPath @@ -958,7 +958,7 @@ const appActions = { * @param {Object} devices {[deviceId]: {lastRecordTimestamp=, name=}} */ saveSyncDevices: function (devices) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SAVE_SYNC_DEVICES, devices }) @@ -972,7 +972,7 @@ const appActions = { * @param {string=} seedQr */ saveSyncInitData: function (seed, deviceId, lastFetchTimestamp, seedQr) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SAVE_SYNC_INIT_DATA, seed, deviceId, @@ -986,7 +986,7 @@ const appActions = { * @param {string|null} error */ setSyncSetupError: function (error) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SET_SYNC_SETUP_ERROR, error }) @@ -998,7 +998,7 @@ const appActions = { * @param {Array.} records */ applySiteRecords: function (records) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_APPLY_SITE_RECORDS, records }) @@ -1008,7 +1008,7 @@ const appActions = { * Dispatch to populate the sync object id -> appState key path mapping cache */ createSyncCache: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_CREATE_SYNC_CACHE }) }, @@ -1017,7 +1017,7 @@ const appActions = { * Dispatches a message to delete sync data. */ resetSyncData: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_RESET_SYNC_DATA }) }, @@ -1028,7 +1028,7 @@ const appActions = { * @param {Object} detail - Object containing: title, message, buttons to show */ tabMessageBoxShown: function (tabId, detail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_MESSAGE_BOX_SHOWN, tabId, detail @@ -1041,7 +1041,7 @@ const appActions = { * @param {Object} detail - Object containing: suppressCheckbox (boolean) */ tabMessageBoxDismissed: function (tabId, detail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_MESSAGE_BOX_DISMISSED, tabId, detail @@ -1054,7 +1054,7 @@ const appActions = { * @param {Object} detail - Replacement object */ tabMessageBoxUpdated: function (tabId, detail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_MESSAGE_BOX_UPDATED, tabId, detail @@ -1068,7 +1068,7 @@ const appActions = { * @param location {string} location where handler was triggered */ navigatorHandlerRegistered: function (partition, protocol, location) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_NAVIGATOR_HANDLER_REGISTERED, partition, protocol, @@ -1083,7 +1083,7 @@ const appActions = { * @param location {string} location where handler was triggered */ navigatorHandlerUnregistered: function (partition, protocol, location) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_NAVIGATOR_HANDLER_UNREGISTERED, partition, protocol, @@ -1095,7 +1095,7 @@ const appActions = { * Open dialog for default download path setting */ defaultDownloadPath: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DOWNLOAD_DEFAULT_PATH }) }, @@ -1107,7 +1107,7 @@ const appActions = { * @param publishers {Object} publishers from the synopsis */ enableUndefinedPublishers: function (publishers) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ENABLE_UNDEFINED_PUBLISHERS, publishers }) @@ -1118,7 +1118,7 @@ const appActions = { * @param publishers {Object} updated publishers */ changeLedgerPinnedPercentages: function (publishers) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_CHANGE_LEDGER_PINNED_PERCENTAGES, publishers }) @@ -1131,7 +1131,7 @@ const appActions = { * @param {number} tabId - The tabId of the tab to pin */ tabPinned: function (tabId, pinned) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_PINNED, tabId, pinned @@ -1145,7 +1145,7 @@ const appActions = { * @param {object} tabValue - the created tab state */ newWebContentsAdded: function (windowId, frameOpts, tabValue) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_NEW_WEB_CONTENTS_ADDED, queryInfo: { windowId @@ -1162,7 +1162,7 @@ const appActions = { * @param {object} dragData - Data being transfered */ dragStarted: function (windowId, dragType, dragData) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DRAG_STARTED, windowId, dragType, @@ -1176,7 +1176,7 @@ const appActions = { * @param {object} dragData - Data being transfered */ dragEnded: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DRAG_ENDED }) }, @@ -1185,7 +1185,7 @@ const appActions = { * Notifies the app that a drop operation occurred */ dataDropped: function (dropWindowId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DATA_DROPPED, dropWindowId }) @@ -1195,7 +1195,7 @@ const appActions = { * Notifies the app that a drop operation occurred */ draggedOver: function (draggedOverData) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DRAGGED_OVER, draggedOverData }) @@ -1206,7 +1206,7 @@ const appActions = { * @param {number} tabId - Tab id used for an action */ onGoBack: function (tabId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ON_GO_BACK, tabId }) @@ -1217,7 +1217,7 @@ const appActions = { * @param {number} tabId - Tab id used for an action */ onGoForward: function (tabId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ON_GO_FORWARD, tabId }) @@ -1229,7 +1229,7 @@ const appActions = { * @param {number} index - Index in the history */ onGoToIndex: function (tabId, index) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ON_GO_TO_INDEX, tabId, @@ -1243,7 +1243,7 @@ const appActions = { * @param {ClientRect} rect - Parent element position for this action */ onGoBackLong: function (tabId, rect) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ON_GO_BACK_LONG, tabId, rect @@ -1256,7 +1256,7 @@ const appActions = { * @param {ClientRect} rect - Parent element position for this action */ onGoForwardLong: function (tabId, rect) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_ON_GO_FORWARD_LONG, tabId, rect @@ -1268,7 +1268,7 @@ const appActions = { * because ESC was pressed. */ dragCancelled: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_DRAG_CANCELLED }) }, @@ -1278,7 +1278,7 @@ const appActions = { * @param {number} tabId - Tab id of current frame */ autoplayBlocked: function (tabId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_AUTOPLAY_BLOCKED, tabId }) @@ -1288,7 +1288,7 @@ const appActions = { * Handle 'save-password' event from muon */ savePassword: function (username, origin, tabId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_SAVE_PASSWORD, username, origin, @@ -1300,7 +1300,7 @@ const appActions = { * Handle 'update-password' event from muon */ updatePassword: function (username, origin, tabId) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_UPDATE_PASSWORD, username, origin, @@ -1313,7 +1313,7 @@ const appActions = { * @param {Object} passwordDetail - login details */ deletePassword: function (passwordDetail) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_REMOVE_PASSWORD, passwordDetail }) @@ -1323,7 +1323,7 @@ const appActions = { * Deletes all saved login credentials */ clearPasswords: function () { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_CLEAR_PASSWORDS }) }, @@ -1332,7 +1332,7 @@ const appActions = { * Delete legacy "never saved password" list */ deletePasswordSite: function (origin) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_CHANGE_SITE_SETTING, hostPattern: origin, key: 'savePasswords' @@ -1345,7 +1345,7 @@ const appActions = { * @param {string} location - The text to set as the new navbar URL input */ urlBarTextChanged: function (windowId, input) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_URL_BAR_TEXT_CHANGED, input, queryInfo: { @@ -1354,6 +1354,24 @@ const appActions = { }) }, + /** + * New URL bar suggestion search results are available. + * This is typically from a service like Duck Duck Go auto complete for the portion of text that the user typed in. + * + * @param {number} tabId - the tab id for the action + * @param searchResults The search results for the currently entered URL bar text. + */ + searchSuggestionResultsAvailable: function (windowId, tabId, searchResults) { + dispatch({ + actionType: appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE, + tabId, + searchResults, + queryInfo: { + windowId + } + }) + }, + /** * Indicates URL bar suggestions and selected index. * @@ -1361,13 +1379,12 @@ const appActions = { * @param {number} selectedIndex - The index for the selected item (users can select items with down arrow on their keyboard) */ urlBarSuggestionsChanged: function (suggestionList, selectedIndex) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_URL_BAR_SUGGESTIONS_CHANGED, suggestionList, selectedIndex }) } - } module.exports = appActions diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index ceca582cb8e..1eceef9c589 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -4,15 +4,10 @@ 'use strict' -const AppDispatcher = require('../dispatcher/appDispatcher') +const {dispatch} = require('../dispatcher/appDispatcher') const windowConstants = require('../constants/windowConstants') -function dispatch (action) { - AppDispatcher.dispatch(action) -} - const windowActions = { - /** * Dispatches an event to the main process to replace the window state * @@ -399,21 +394,6 @@ const windowActions = { }) }, - /** - * New URL bar suggestion search results are available. - * This is typically from a service like Duck Duck Go auto complete for the portion of text that the user typed in. - * - * @param {number} tabId - the tab id for the action - * @param searchResults The search results for the currently entered URL bar text. - */ - searchSuggestionResultsAvailable: function (tabId, searchResults) { - dispatch({ - actionType: windowConstants.WINDOW_SEARCH_SUGGESTION_RESULTS_AVAILABLE, - tabId, - searchResults - }) - }, - /** * Marks the URL bar text as selected or not * diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index 9152153b13a..dc0757db7ed 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -109,6 +109,7 @@ const appConstants = { APP_NAVIGATOR_HANDLER_UNREGISTERED: _, APP_URL_BAR_TEXT_CHANGED: _, APP_URL_BAR_SUGGESTIONS_CHANGED: _, + APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE: _, APP_CHANGE_LEDGER_PINNED_PERCENTAGES: _, APP_ENABLE_UNDEFINED_PUBLISHERS: _, APP_TAB_PINNED: _, diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index 4c2db17cf64..3b9e9ec3a45 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -22,7 +22,6 @@ const windowConstants = { WINDOW_SET_LINK_HOVER_PREVIEW: _, WINDOW_SET_RENDER_URL_BAR_SUGGESTIONS: _, WINDOW_URL_BAR_AUTOCOMPLETE_ENABLED: _, - WINDOW_SEARCH_SUGGESTION_RESULTS_AVAILABLE: _, WINDOW_URL_BAR_SUGGESTIONS_CLEARED: _, WINDOW_PREVIOUS_URL_BAR_SUGGESTION_SELECTED: _, WINDOW_NEXT_URL_BAR_SUGGESTION_SELECTED: _, diff --git a/js/dispatcher/appDispatcher.js b/js/dispatcher/appDispatcher.js index 23c786da011..4c42c4072d8 100644 --- a/js/dispatcher/appDispatcher.js +++ b/js/dispatcher/appDispatcher.js @@ -22,6 +22,7 @@ class AppDispatcher { this.callbacks = [] this.promises = [] this.dispatching = false + this.dispatch = this.dispatch.bind(this) } /** diff --git a/test/unit/app/renderer/reducers/urlBarReducerTest.js b/test/unit/app/renderer/reducers/urlBarReducerTest.js index be22d690e30..622743ea33a 100644 --- a/test/unit/app/renderer/reducers/urlBarReducerTest.js +++ b/test/unit/app/renderer/reducers/urlBarReducerTest.js @@ -281,10 +281,10 @@ describe('urlBarReducer', function () { }) }) - describe('WINDOW_SEARCH_SUGGESTION_RESULTS_AVAILABLE', function () { + describe('APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE', function () { it('turns off suggestions', function () { const searchResults = Immutable.fromJS(['0110001001110010011010010110000101101110']) - const newState = urlBarReducer(windowState, {actionType: windowConstants.WINDOW_SEARCH_SUGGESTION_RESULTS_AVAILABLE, searchResults, tabId: 2}) + const newState = urlBarReducer(windowState, {actionType: appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE, searchResults, tabId: 2}) assert.deepEqual(newState.getIn(['frames', 1, 'navbar', 'urlbar', 'suggestions', 'searchResults']).toJS(), searchResults.toJS()) }) }) From 9cf76c08c2dc9cdfad3a0038970eceeb9c262e63 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Fri, 12 May 2017 15:00:13 -0400 Subject: [PATCH 07/22] Default search engine windowState -> appState --- app/renderer/components/navigation/urlBar.js | 5 ++++- app/renderer/suggestionClickHandlers.js | 3 ++- app/sessionStore.js | 3 ++- docs/appActions.md | 10 ++++++++++ docs/windowActions.md | 10 ---------- js/actions/appActions.js | 12 ++++++++++++ js/actions/windowActions.js | 11 ----------- js/components/main.js | 2 +- js/constants/appConstants.js | 1 + js/constants/windowConstants.js | 1 - js/contextMenus.js | 6 +++--- js/stores/appStore.js | 3 +++ js/stores/windowStore.js | 10 ++-------- 13 files changed, 40 insertions(+), 37 deletions(-) diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 1d2ff0c58d4..1b606ed77eb 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -41,6 +41,9 @@ const {getCurrentWindowId} = require('../../currentWindow') // Icons const iconNoScript = require('../../../../img/url-bar-no-script.svg') +// Stores +const appStoreRenderer = require('../../../../js/stores/appStoreRenderer') + class UrlBar extends React.Component { constructor (props) { super(props) @@ -463,7 +466,7 @@ class UrlBar extends React.Component { const activateSearchEngine = urlbar.getIn(['searchDetail', 'activateSearchEngine']) const urlbarSearchDetail = urlbar.get('searchDetail') - let searchURL = currentWindow.getIn(['searchDetail', 'searchURL']) + let searchURL = appStoreRenderer.state.getIn(['searchDetail', 'searchURL']) let searchShortcut = '' // remove shortcut from the search terms if (activateSearchEngine && urlbarSearchDetail !== null) { diff --git a/app/renderer/suggestionClickHandlers.js b/app/renderer/suggestionClickHandlers.js index 129e08f7683..eff83d0af23 100644 --- a/app/renderer/suggestionClickHandlers.js +++ b/app/renderer/suggestionClickHandlers.js @@ -7,6 +7,7 @@ const appActions = require('../../js/actions/appActions') const windowActions = require('../../js/actions/windowActions') const windowStore = require('../../js/stores/windowStore') +const appStoreRenderer = require('../../js/stores/appStoreRenderer') const suggestionTypes = require('../../js/constants/suggestionTypes') const {makeImmutable} = require('../common/state/immutableUtil') const {getActiveFrame} = require('../../js/state/frameStateUtil') @@ -21,7 +22,7 @@ const navigateSiteClickHandler = (suggestionData, isForSecondaryAction, shiftKey let url if (type === suggestionTypes.SEARCH) { const frameSearchDetail = suggestionData.getIn(['navbar', 'urlbar', 'searchDetail']) - const searchDetail = windowStore.state.get('searchDetail') + const searchDetail = appStoreRenderer.state.get('searchDetail') let searchURL = frameSearchDetail ? frameSearchDetail.get('search') : searchDetail.get('searchURL') url = searchURL.replace('{searchTerms}', encodeURIComponent(suggestionData.get('location'))) diff --git a/app/sessionStore.js b/app/sessionStore.js index 71f6f8360a5..5deb3aee881 100644 --- a/app/sessionStore.js +++ b/app/sessionStore.js @@ -658,7 +658,8 @@ module.exports.defaultAppState = () => { httpsEverywhere: { count: 0 }, - defaultWindowParams: {} + defaultWindowParams: {}, + searchDetail: null } } diff --git a/docs/appActions.md b/docs/appActions.md index 0f18c2a960b..a2834fcf3ab 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -1092,6 +1092,16 @@ Indicates URL bar suggestions and selected index. +### defaultSearchEngineLoaded(searchDetail) + +Dispatches a message to set the search engine details. + +**Parameters** + +**searchDetail**: `Object`, the search details + + + * * * diff --git a/docs/windowActions.md b/docs/windowActions.md index bbe8e68e19d..b55e4f5ea9b 100644 --- a/docs/windowActions.md +++ b/docs/windowActions.md @@ -357,16 +357,6 @@ set from an IPC call. -### setSearchDetail(searchDetail) - -Dispatches a message to set the search engine details. - -**Parameters** - -**searchDetail**: `Object`, the search details - - - ### setFindDetail(frameKey, findDetail) Dispatches a message to set the find-in-page details. diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 37533995420..993d8164e0e 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -1384,7 +1384,19 @@ const appActions = { suggestionList, selectedIndex }) + }, + + /** + * Dispatches a message to set the search engine details. + * @param {Object} searchDetail - the search details + */ + defaultSearchEngineLoaded: function (searchDetail) { + dispatch({ + actionType: appConstants.APP_DEFAULT_SEARCH_ENGINE_LOADED, + searchDetail + }) } + } module.exports = appActions diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js index 1eceef9c589..6dad5af36a2 100644 --- a/js/actions/windowActions.js +++ b/js/actions/windowActions.js @@ -462,17 +462,6 @@ const windowActions = { }) }, - /** - * Dispatches a message to set the search engine details. - * @param {Object} searchDetail - the search details - */ - setSearchDetail: function (searchDetail) { - dispatch({ - actionType: windowConstants.WINDOW_SET_SEARCH_DETAIL, - searchDetail - }) - }, - /** * Dispatches a message to set the find-in-page details. * @param {Object} frameKey - Frame key of the frame in question diff --git a/js/components/main.js b/js/components/main.js index 936d00b6ced..865cbf1ce43 100644 --- a/js/components/main.js +++ b/js/components/main.js @@ -288,7 +288,7 @@ class Main extends ImmutableComponent { if (this.lastLoadedSearchProviders === undefined || engine !== this.lastLoadedSearchProviders) { entries.forEach((entry) => { if (entry.name === engine) { - windowActions.setSearchDetail(Immutable.fromJS({ + appActions.defaultSearchEngineLoaded(Immutable.fromJS({ searchURL: entry.search, autocompleteURL: entry.autocomplete, platformClientId: entry.platformClientId diff --git a/js/constants/appConstants.js b/js/constants/appConstants.js index dc0757db7ed..f1c518e3932 100644 --- a/js/constants/appConstants.js +++ b/js/constants/appConstants.js @@ -110,6 +110,7 @@ const appConstants = { APP_URL_BAR_TEXT_CHANGED: _, APP_URL_BAR_SUGGESTIONS_CHANGED: _, APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE: _, + APP_DEFAULT_SEARCH_ENGINE_LOADED: _, APP_CHANGE_LEDGER_PINNED_PERCENTAGES: _, APP_ENABLE_UNDEFINED_PUBLISHERS: _, APP_TAB_PINNED: _, diff --git a/js/constants/windowConstants.js b/js/constants/windowConstants.js index 3b9e9ec3a45..3fed146c5e9 100644 --- a/js/constants/windowConstants.js +++ b/js/constants/windowConstants.js @@ -41,7 +41,6 @@ const windowConstants = { WINDOW_URL_BAR_ON_FOCUS: _, WINDOW_TAB_ON_FOCUS: _, WINDOW_SET_URL_BAR_SELECTED: _, - WINDOW_SET_SEARCH_DETAIL: _, 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 diff --git a/js/contextMenus.js b/js/contextMenus.js index efde4f195ab..78c163e8627 100644 --- a/js/contextMenus.js +++ b/js/contextMenus.js @@ -957,7 +957,7 @@ const searchSelectionMenuItem = (location) => { if (location) { let activeFrame = windowStore.getState().get('activeFrameKey') let frame = windowStore.getFrame(activeFrame) - let searchUrl = windowStore.getState().getIn(['searchDetail', 'searchURL']).replace('{searchTerms}', encodeURIComponent(location)) + let searchUrl = appStoreRenderer.state.getIn(['searchDetail', 'searchURL']).replace('{searchTerms}', encodeURIComponent(location)) appActions.createTabRequested({ url: searchUrl, isPrivate: frame.get('isPrivate'), @@ -1064,7 +1064,7 @@ function mainTemplateInit (nodeProps, frame, tab) { click: () => { let activeFrame = windowStore.getState().get('activeFrameKey') let frame = windowStore.getFrame(activeFrame) - let searchUrl = windowStore.getState().getIn(['searchDetail', 'searchURL']) + let searchUrl = appStoreRenderer.state.getIn(['searchDetail', 'searchURL']) .replace('{searchTerms}', encodeURIComponent(nodeProps.srcURL)) .replace('?q', 'byimage?image_url') appActions.createTabRequested({ @@ -1399,8 +1399,8 @@ function onTabPageContextMenu (framePropsList, e) { function onUrlBarContextMenu (e) { e.stopPropagation() + const searchDetail = appStoreRenderer.state.get('searchDetail') const windowState = windowStore.getState() - const searchDetail = windowStore.getState().get('searchDetail') const activeFrame = getActiveFrame(windowState) const inputMenu = Menu.buildFromTemplate(urlBarTemplateInit(searchDetail, activeFrame, e)) inputMenu.popup(getCurrentWindow()) diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 8fccd482889..17997179afa 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -891,6 +891,9 @@ const handleAppAction = (action) => { appState = appState.set('siteSettings', newSiteSettings) }) break + case appConstants.APP_DEFAULT_SEARCH_ENGINE_LOADED: + appState = appState.set('searchDetail', action.searchDetail) + break default: } diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 8e4760d240c..6ff253b6efb 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -35,8 +35,7 @@ let windowState = Immutable.fromJS({ mouseInTitlebar: false, menubar: { } - }, - searchDetail: null + } }) let lastEmittedState @@ -130,7 +129,7 @@ const newFrame = (state, frameOpts, openInForeground, insertionIndex, nextKey) = frameOpts.location = UrlUtil.getUrlFromInput(frameOpts.location) } else { // location is a search - const defaultURL = windowStore.getState().getIn(['searchDetail', 'searchURL']) + const defaultURL = appStoreRenderer.state.getIn(['searchDetail', 'searchURL']) if (defaultURL) { frameOpts.location = defaultURL .replace('{searchTerms}', encodeURIComponent(frameOpts.location)) @@ -424,11 +423,6 @@ const doAction = (action) => { activeShortcutDetails: action.activeShortcutDetails }) break - case windowConstants.WINDOW_SET_SEARCH_DETAIL: - windowState = windowState.merge({ - searchDetail: action.searchDetail - }) - break case windowConstants.WINDOW_SET_FIND_DETAIL: { const frameIndex = frameStateUtil.getFrameIndex(windowState, action.frameKey) From 9d49e4bcdc883961a33227b448500dcf68e37650 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Fri, 12 May 2017 16:58:15 -0400 Subject: [PATCH 08/22] Move suggestion files to common app dir --- app/{renderer => common/lib}/fetchSearchSuggestions.js | 4 ++-- app/{renderer => common}/lib/suggestion.js | 3 +-- app/renderer/reducers/urlBarReducer.js | 2 +- test/unit/app/{renderer => common}/lib/suggestionTest.js | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) rename app/{renderer => common/lib}/fetchSearchSuggestions.js (87%) rename app/{renderer => common}/lib/suggestion.js (99%) rename test/unit/app/{renderer => common}/lib/suggestionTest.js (98%) diff --git a/app/renderer/fetchSearchSuggestions.js b/app/common/lib/fetchSearchSuggestions.js similarity index 87% rename from app/renderer/fetchSearchSuggestions.js rename to app/common/lib/fetchSearchSuggestions.js index caa8f281b11..35c8cfe58ec 100644 --- a/app/renderer/fetchSearchSuggestions.js +++ b/app/common/lib/fetchSearchSuggestions.js @@ -3,8 +3,8 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const Immutable = require('immutable') -const appActions = require('../../js/actions/appActions') -const debounce = require('../../js/lib/debounce') +const appActions = require('../../../js/actions/appActions') +const debounce = require('../../../js/lib/debounce') const fetchSearchSuggestions = debounce((windowId, tabId, autocompleteURL, searchTerms) => { const xhr = new window.XMLHttpRequest() diff --git a/app/renderer/lib/suggestion.js b/app/common/lib/suggestion.js similarity index 99% rename from app/renderer/lib/suggestion.js rename to app/common/lib/suggestion.js index 502985fbfb7..7476bbfdca4 100644 --- a/app/renderer/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -14,7 +14,7 @@ const settings = require('../../../js/constants/settings') const {isBookmark, isDefaultEntry, isHistoryEntry} = require('../../../js/state/siteUtil') const config = require('../../../js/constants/config') const top500 = require('../../../js/data/top500') -const fetchSearchSuggestions = require('../fetchSearchSuggestions') +const fetchSearchSuggestions = require('./fetchSearchSuggestions') const {getFrameByTabId, getTabsByWindowId} = require('../../common/state/tabState') const sigmoid = (t) => { @@ -466,7 +466,6 @@ const generateNewSearchXHRResults = (state, windowId, tabId, input) => { const replaceRE = new RegExp('^' + searchDetail.get('shortcut') + ' ', 'g') input = input.replace(replaceRE, '') } - fetchSearchSuggestions(windowId, tabId, autocompleteURL, input) } else { const appActions = require('../../../js/actions/appActions') diff --git a/app/renderer/reducers/urlBarReducer.js b/app/renderer/reducers/urlBarReducer.js index 7ab777c410e..2aff5ebad3a 100644 --- a/app/renderer/reducers/urlBarReducer.js +++ b/app/renderer/reducers/urlBarReducer.js @@ -11,7 +11,7 @@ const {isURL, isPotentialPhishingUrl, getUrlFromInput} = require('../../../js/li const {getFrameByKey, getFrameKeyByTabId, activeFrameStatePath, frameStatePath, getActiveFrame, getFrameByTabId} = require('../../../js/state/frameStateUtil') const searchProviders = require('../../../js/data/searchProviders') const Immutable = require('immutable') -const {generateNewSuggestionsList, generateNewSearchXHRResults} = require('../lib/suggestion') +const {generateNewSuggestionsList, generateNewSearchXHRResults} = require('../../common/lib/suggestion') const {navigateSiteClickHandler} = require('../suggestionClickHandlers') const {getCurrentWindowId} = require('../currentWindow') const appStoreRenderer = require('../../../js/stores/appStoreRenderer') diff --git a/test/unit/app/renderer/lib/suggestionTest.js b/test/unit/app/common/lib/suggestionTest.js similarity index 98% rename from test/unit/app/renderer/lib/suggestionTest.js rename to test/unit/app/common/lib/suggestionTest.js index cdb5bcac244..3bb419774e5 100644 --- a/test/unit/app/renderer/lib/suggestionTest.js +++ b/test/unit/app/common/lib/suggestionTest.js @@ -32,7 +32,7 @@ describe('suggestion unit tests', function () { makeImmutableSpy = sinon.spy(fakeImmutableUtil, 'makeImmutable') mockery.registerMock('../../common/state/immutableUtil', fakeImmutableUtil) - suggestion = require('../../../../../app/renderer/lib/suggestion') + suggestion = require('../../../../../app/common/lib/suggestion') }) after(function () { From 2d719a74ef4d797a1e780147ebf034fadccffddb Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Sat, 13 May 2017 10:16:28 -0400 Subject: [PATCH 09/22] Move suggestion creation into the browser process --- .../reducers/urlBarSuggestionsReducer.js | 24 +++++++++++++++++++ app/common/lib/suggestion.js | 2 +- app/renderer/components/navigation/urlBar.js | 14 +++++------ .../navigation/urlBarSuggestions.js | 5 ++-- app/renderer/reducers/urlBarReducer.js | 17 ------------- docs/appActions.md | 12 +++++++--- js/actions/appActions.js | 18 ++++++++++---- js/dispatcher/appDispatcher.js | 12 ++++++---- js/stores/appStore.js | 1 + 9 files changed, 67 insertions(+), 38 deletions(-) create mode 100644 app/browser/reducers/urlBarSuggestionsReducer.js diff --git a/app/browser/reducers/urlBarSuggestionsReducer.js b/app/browser/reducers/urlBarSuggestionsReducer.js new file mode 100644 index 00000000000..e73ddd24592 --- /dev/null +++ b/app/browser/reducers/urlBarSuggestionsReducer.js @@ -0,0 +1,24 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +'use strict' + +const appConstants = require('../../../js/constants/appConstants') +const {generateNewSuggestionsList, generateNewSearchXHRResults} = require('../../common/lib/suggestion') + +const urlBarSuggestionsReducer = (state, action) => { + switch (action.actionType) { + case appConstants.APP_URL_BAR_TEXT_CHANGED: + generateNewSuggestionsList(state, action.windowId, action.tabId, action.input) + generateNewSearchXHRResults(state, action.windowId, action.tabId, action.input) + break + case appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE: + // TODO: Find a way to get urlLocation and also convert search suggestions to fetch + // generateNewSuggestionsList(state, action.windowId, action.tabId, urlLocation, action.searchResults) + break + } + return state +} + +module.exports = urlBarSuggestionsReducer diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js index 7476bbfdca4..5e23f80d13f 100644 --- a/app/common/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -436,7 +436,7 @@ const generateNewSuggestionsList = (state, windowId, tabId, urlLocation) => { ]).then(([...suggestionsLists]) => { const appActions = require('../../../js/actions/appActions') // Flatten only 1 level deep for perf only, nested will be objects within arrrays - appActions.urlBarSuggestionsChanged(makeImmutable(suggestionsLists).flatten(1)) + appActions.urlBarSuggestionsChanged(windowId, makeImmutable(suggestionsLists).flatten(1)) }) } diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 1b606ed77eb..74e3e0ea51b 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -72,7 +72,7 @@ class UrlBar extends React.Component { if (this.urlInput) { this.setValue(location) } - appActions.urlBarTextChanged(getCurrentWindowId(), location) + appActions.urlBarTextChanged(getCurrentWindowId(), this.props.activeTabId, location) } /** @@ -97,7 +97,7 @@ class UrlBar extends React.Component { if (this.props.autocompleteEnabled) { windowActions.urlBarAutocompleteEnabled(false) } - appActions.urlBarSuggestionsChanged(undefined, null) + appActions.urlBarSuggestionsChanged(getCurrentWindowId(), undefined, null) windowActions.setRenderUrlBarSuggestions(false) } @@ -273,7 +273,7 @@ class UrlBar extends React.Component { if (e.target.value !== this.lastVal + this.lastSuffix) { e.preventDefault() // clear any current arrow or mouse hover selection - appActions.urlBarSuggestionsChanged(undefined, null) + appActions.urlBarSuggestionsChanged(getCurrentWindowId(), undefined, null) this.setValue(e.target.value) } } @@ -295,7 +295,7 @@ class UrlBar extends React.Component { if (!this.keyPress) { // if this is a key press don't sent the update until keyUp so // showAutocompleteResult can handle the result - appActions.urlBarTextChanged(getCurrentWindowId(), val) + appActions.urlBarTextChanged(getCurrentWindowId(), this.props.activeTabId, val) } } } @@ -311,9 +311,9 @@ class UrlBar extends React.Component { windowActions.setUrlBarSelected(false) } // clear any current arrow or mouse hover selection - appActions.urlBarSuggestionsChanged(undefined, null) + appActions.urlBarSuggestionsChanged(getCurrentWindowId(), undefined, null) this.keyPressed = false - appActions.urlBarTextChanged(getCurrentWindowId(), this.lastVal) + appActions.urlBarTextChanged(getCurrentWindowId(), this.props.activeTabId, this.lastVal) } select () { @@ -368,7 +368,7 @@ class UrlBar extends React.Component { if (this.props.isFocused) { this.focus() } - appActions.urlBarSuggestionsChanged(undefined, null) + appActions.urlBarSuggestionsChanged(getCurrentWindowId(), undefined, null) windowActions.setRenderUrlBarSuggestions(false) } else if (this.props.location !== prevProps.location) { // This is a url nav change diff --git a/app/renderer/components/navigation/urlBarSuggestions.js b/app/renderer/components/navigation/urlBarSuggestions.js index f1fc02b4e75..af907ebf85c 100644 --- a/app/renderer/components/navigation/urlBarSuggestions.js +++ b/app/renderer/components/navigation/urlBarSuggestions.js @@ -12,6 +12,7 @@ const suggestionTypes = require('../../../../js/constants/suggestionTypes') const cx = require('../../../../js/lib/classSet') const locale = require('../../../../js/l10n') const {isForSecondaryAction} = require('../../../../js/lib/eventUtil') +const {getCurrentWindowId} = require('../../currentWindow') class UrlBarSuggestions extends ImmutableComponent { constructor () { @@ -28,7 +29,7 @@ class UrlBarSuggestions extends ImmutableComponent { } blur () { - appActions.urlBarSuggestionsChanged(null, null) + appActions.urlBarSuggestionsChanged(getCurrentWindowId(), null, null) } onSuggestionClicked (e) { @@ -106,7 +107,7 @@ class UrlBarSuggestions extends ImmutableComponent { if (newIndex === 0 || newIndex > suggestions.size) { newIndex = null } - appActions.urlBarSuggestionsChanged(suggestions, newIndex) + appActions.urlBarSuggestionsChanged(getCurrentWindowId(), suggestions, newIndex) } } diff --git a/app/renderer/reducers/urlBarReducer.js b/app/renderer/reducers/urlBarReducer.js index 2aff5ebad3a..d82ed5e93bf 100644 --- a/app/renderer/reducers/urlBarReducer.js +++ b/app/renderer/reducers/urlBarReducer.js @@ -11,11 +11,7 @@ const {isURL, isPotentialPhishingUrl, getUrlFromInput} = require('../../../js/li const {getFrameByKey, getFrameKeyByTabId, activeFrameStatePath, frameStatePath, getActiveFrame, getFrameByTabId} = require('../../../js/state/frameStateUtil') const searchProviders = require('../../../js/data/searchProviders') const Immutable = require('immutable') -const {generateNewSuggestionsList, generateNewSearchXHRResults} = require('../../common/lib/suggestion') const {navigateSiteClickHandler} = require('../suggestionClickHandlers') -const {getCurrentWindowId} = require('../currentWindow') -const appStoreRenderer = require('../../../js/stores/appStoreRenderer') - const navigationBarState = require('../../common/state/navigationBarState') const tabState = require('../../common/state/tabState') @@ -134,15 +130,6 @@ const setNavBarUserInput = (state, location) => { state = updateNavBarInput(state, location) const activeFrameProps = getActiveFrame(state) state = updateSearchEngineInfoFromInput(state, activeFrameProps) - const urlLocation = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'location'])) - const windowId = getCurrentWindowId() - const tabId = activeFrameProps.get('tabId') - if (tabId) { - setImmediate(() => { - generateNewSearchXHRResults(appStoreRenderer.state, windowId, tabId, urlLocation) - generateNewSuggestionsList(appStoreRenderer.state, windowId, tabId, urlLocation) - }) - } if (!location) { state = setRenderUrlBarSuggestions(state, false) } @@ -280,10 +267,6 @@ const urlBarReducer = (state, action) => { case appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE: const frameKey = getFrameKeyByTabId(state, action.tabId) state = state.setIn(frameStatePath(state, frameKey).concat(['navbar', 'urlbar', 'suggestions', 'searchResults']), action.searchResults) - const urlLocation = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'location'])) - setImmediate(() => { - generateNewSuggestionsList(appStoreRenderer.state, getCurrentWindowId(), action.tabId, urlLocation) - }) break case windowConstants.WINDOW_URL_BAR_AUTOCOMPLETE_ENABLED: state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'autocompleteEnabled']), action.enabled) diff --git a/docs/appActions.md b/docs/appActions.md index a2834fcf3ab..f5f8fa5bd23 100644 --- a/docs/appActions.md +++ b/docs/appActions.md @@ -1057,13 +1057,17 @@ Delete legacy "never saved password" list -### urlBarTextChanged(location) +### urlBarTextChanged(windowId, tabId, input) Indicates that the urlbar text has changed, usually from user input **Parameters** -**location**: `string`, The text to set as the new navbar URL input +**windowId**: `number`, The window ID the text is being changed inside of + +**tabId**: `number`, The tab ID the text is being changed inside of + +**input**: `string`, The text that was entered into the URL bar @@ -1080,12 +1084,14 @@ This is typically from a service like Duck Duck Go auto complete for the portion -### urlBarSuggestionsChanged(suggestionList, selectedIndex) +### urlBarSuggestionsChanged(windowId, suggestionList, selectedIndex) Indicates URL bar suggestions and selected index. **Parameters** +**windowId**: `number`, the window ID + **suggestionList**: `Array.<Object>`, The list of suggestions for the entered URL bar text. This can be generated from history, bookmarks, etc. **selectedIndex**: `number`, The index for the selected item (users can select items with down arrow on their keyboard) diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 993d8164e0e..46ad4bcff7a 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -1342,12 +1342,16 @@ const appActions = { /** * Indicates that the urlbar text has changed, usually from user input * - * @param {string} location - The text to set as the new navbar URL input + * @param {number} windowId - The window ID the text is being changed inside of + * @param {number} tabId - The tab ID the text is being changed inside of + * @param {string} input - The text that was entered into the URL bar */ - urlBarTextChanged: function (windowId, input) { + urlBarTextChanged: function (windowId, tabId, input) { dispatch({ actionType: appConstants.APP_URL_BAR_TEXT_CHANGED, input, + tabId, + windowId, queryInfo: { windowId } @@ -1365,6 +1369,7 @@ const appActions = { dispatch({ actionType: appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE, tabId, + windowId, searchResults, queryInfo: { windowId @@ -1375,14 +1380,19 @@ const appActions = { /** * Indicates URL bar suggestions and selected index. * + * @param {number} windowId - the window ID * @param {Object[]} suggestionList - The list of suggestions for the entered URL bar text. This can be generated from history, bookmarks, etc. * @param {number} selectedIndex - The index for the selected item (users can select items with down arrow on their keyboard) */ - urlBarSuggestionsChanged: function (suggestionList, selectedIndex) { + urlBarSuggestionsChanged: function (windowId, suggestionList, selectedIndex) { dispatch({ actionType: appConstants.APP_URL_BAR_SUGGESTIONS_CHANGED, suggestionList, - selectedIndex + selectedIndex, + windowId, + queryInfo: { + windowId + } }) }, diff --git a/js/dispatcher/appDispatcher.js b/js/dispatcher/appDispatcher.js index 4c42c4072d8..dcc05affb92 100644 --- a/js/dispatcher/appDispatcher.js +++ b/js/dispatcher/appDispatcher.js @@ -98,11 +98,15 @@ class AppDispatcher { const {getCurrentWindowId} = require('../../app/renderer/currentWindow') if (!payload.queryInfo || !payload.queryInfo.windowId || payload.queryInfo.windowId === getCurrentWindowId()) { this.dispatchToOwnRegisteredCallbacks(payload) + // We still want to tell the browser prcoess about app actions for payloads with a windowId + // specified for the current window, but we don't want the browser process to forward it back + // to us. + if (payload.queryInfo) { + payload.queryInfo.alreadyHandledByRenderer = payload.queryInfo.windowId === getCurrentWindowId() + } } cb() - if (!payload.queryInfo || !payload.queryInfo.windowId || payload.queryInfo.windowId !== getCurrentWindowId()) { - ipcCargo.push(payload) - } + ipcCargo.push(payload) return } } @@ -176,7 +180,7 @@ if (process.type === 'browser') { let queryInfo = payload.queryInfo || payload.frameProps || (payload.queryInfo = {}) queryInfo = queryInfo.toJS ? queryInfo.toJS() : queryInfo let sender = event.sender - if (!sender.isDestroyed()) { + if (!queryInfo.alreadyHandledByRenderer && !sender.isDestroyed()) { const hostWebContents = sender.hostWebContents sender = hostWebContents || sender const win = require('electron').BrowserWindow.fromWebContents(sender) diff --git a/js/stores/appStore.js b/js/stores/appStore.js index 17997179afa..bd8cf924104 100644 --- a/js/stores/appStore.js +++ b/js/stores/appStore.js @@ -395,6 +395,7 @@ const handleAppAction = (action) => { require('../../app/browser/reducers/windowsReducer'), require('../../app/browser/reducers/spellCheckReducer'), require('../../app/browser/reducers/clipboardReducer'), + require('../../app/browser/reducers/urlBarSuggestionsReducer'), require('../../app/browser/reducers/passwordManagerReducer'), require('../../app/browser/reducers/tabMessageBoxReducer'), require('../../app/browser/reducers/dragDropReducer'), From 4d89a15f11b6e251c42fc2db11c6f7e5b2ac7b3a Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Sat, 13 May 2017 22:37:41 -0400 Subject: [PATCH 10/22] Use bloodhound lib for suggestions Fix #7453 --- .../reducers/urlBarSuggestionsReducer.js | 4 + app/common/lib/siteSuggestions.js | 30 +++- app/common/lib/suggestion.js | 152 +++++++----------- js/constants/config.js | 3 +- .../app/common/lib/siteSuggestionsTest.js | 47 ++++++ 5 files changed, 133 insertions(+), 103 deletions(-) diff --git a/app/browser/reducers/urlBarSuggestionsReducer.js b/app/browser/reducers/urlBarSuggestionsReducer.js index e73ddd24592..59df16bb383 100644 --- a/app/browser/reducers/urlBarSuggestionsReducer.js +++ b/app/browser/reducers/urlBarSuggestionsReducer.js @@ -6,9 +6,13 @@ const appConstants = require('../../../js/constants/appConstants') const {generateNewSuggestionsList, generateNewSearchXHRResults} = require('../../common/lib/suggestion') +const {init} = require('../../common/lib/siteSuggestions') const urlBarSuggestionsReducer = (state, action) => { switch (action.actionType) { + case appConstants.APP_SET_STATE: + init(Object.values(action.appState.get('sites').toJS())) + break case appConstants.APP_URL_BAR_TEXT_CHANGED: generateNewSuggestionsList(state, action.windowId, action.tabId, action.input) generateNewSearchXHRResults(state, action.windowId, action.tabId, action.input) diff --git a/app/common/lib/siteSuggestions.js b/app/common/lib/siteSuggestions.js index ea0b80d4614..3d30a6f70aa 100644 --- a/app/common/lib/siteSuggestions.js +++ b/app/common/lib/siteSuggestions.js @@ -3,17 +3,32 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const Bloodhound = require('bloodhound-js') + let initialized = false let engine +let internalSort +let lastQueryInput -const init = (sites) => { - if (initialized) { - return Promise.resolve() +// Same as sortByAccessCountWithAgeDecay but if one is a prefix of the +// other then it is considered always sorted first. +const sortForSuggestions = (s1, s2) => { + return internalSort(s1, s2) +} + +const getSiteIdentity = (data) => { + if (typeof data === 'string') { + return data } + return (data.location || '') + (data.partitionNumber ? '|' + data.partitionNumber : '') +} + +const init = (sites) => { engine = new Bloodhound({ - local: sites, + local: sites.toJS ? sites.toJS() : sites, + sorter: sortForSuggestions, queryTokenizer: tokenizeInput, - datumTokenizer: tokenizeInput + datumTokenizer: tokenizeInput, + identify: getSiteIdentity }) const promise = engine.initialize() promise.then(() => { @@ -46,7 +61,10 @@ const query = (input) => { return Promise.resolve([]) } return new Promise((resolve, reject) => { - engine.search(input, function (results) { + lastQueryInput = input || '' + const {getSortForSuggestions} = require('./suggestion') + internalSort = getSortForSuggestions(lastQueryInput.toLowerCase()) + engine.search(lastQueryInput, function (results) { resolve(results) }, function (err) { reject(err) diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js index 5e23f80d13f..e14cb034909 100644 --- a/app/common/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -11,11 +11,11 @@ const {isUrl, aboutUrls, isNavigatableAboutPage, isSourceAboutUrl} = require('.. const suggestionTypes = require('../../../js/constants/suggestionTypes') const getSetting = require('../../../js/settings').getSetting const settings = require('../../../js/constants/settings') -const {isBookmark, isDefaultEntry, isHistoryEntry} = require('../../../js/state/siteUtil') const config = require('../../../js/constants/config') const top500 = require('../../../js/data/top500') const fetchSearchSuggestions = require('./fetchSearchSuggestions') const {getFrameByTabId, getTabsByWindowId} = require('../../common/state/tabState') +const {query} = require('./siteSuggestions') const sigmoid = (t) => { return 1 / (1 + Math.pow(Math.E, -t)) @@ -83,15 +83,15 @@ const sortingPriority = (count, currentTime, lastAccessedTime, ageDecayConstant) const sortByAccessCountWithAgeDecay = (s1, s2) => { const now = new Date() const s1Priority = sortingPriority( - s1.get('count') || 0, + s1.count || 0, now.getTime(), - s1.get('lastAccessedTime') || now.getTime(), + s1.lastAccessedTime || now.getTime(), appConfig.urlSuggestions.ageDecayConstant ) const s2Priority = sortingPriority( - s2.get('count') || 0, + s2.count || 0, now.getTime(), - s2.get('lastAccessedTime') || now.getTime(), + s2.lastAccessedTime || now.getTime(), appConfig.urlSuggestions.ageDecayConstant ) return s2Priority - s1Priority @@ -214,43 +214,45 @@ const createVirtualHistoryItems = (historySites) => { return !!site }) return Immutable.fromJS(_.object(virtualHistorySites.map((site) => { - return [site.location, site] + return [site.get('location'), site] }))) } -const sortBasedOnLocationPos = (urlLocationLower) => (s1, s2) => { - const shouldNormalize = shouldNormalizeLocation(urlLocationLower) - const urlLocationLowerNormalized = normalizeLocation(urlLocationLower) - - const location1 = shouldNormalize ? normalizeLocation(getURL(s1)) : getURL(s1) - const location2 = shouldNormalize ? normalizeLocation(getURL(s2)) : getURL(s2) - const pos1 = location1.indexOf(urlLocationLowerNormalized) - const pos2 = location2.indexOf(urlLocationLowerNormalized) - if (pos1 === -1 && pos2 === -1) { - return 0 - } else if (pos1 === -1) { - return 1 - } else if (pos2 === -1) { - return -1 - } else { - if (pos1 - pos2 !== 0) { +// Same as sortByAccessCountWithAgeDecay but if one is a prefix of the +// other then it is considered always sorted first. +const getSortForSuggestions = (userInputLower) => (s1, s2) => { + const {sortByAccessCountWithAgeDecay, normalizeLocation} = require('./suggestion') + const url1 = normalizeLocation(getURL(s1)) + const url2 = normalizeLocation(getURL(s2)) + const pos1 = url1.indexOf(userInputLower) + const pos2 = url2.indexOf(userInputLower) + if (pos1 !== -1 && pos2 !== -1) { + if (pos1 !== pos2) { return pos1 - pos2 - } else { - // sort site.com higher than site.com/somepath - const sdnv1 = simpleDomainNameValue(s1) - const sdnv2 = simpleDomainNameValue(s2) - if (sdnv1 !== sdnv2) { - return sdnv2 - sdnv1 - } else { - // If there's a tie on the match location, use the age - // decay modified access count - return sortByAccessCountWithAgeDecay(s1, s2) - } } + return url1.length - url2.length + } + if (pos1 !== -1 && pos2 === -1) { + return -1 } + if (pos1 === -1 && pos2 !== -1) { + return 1 + } + return sortByAccessCountWithAgeDecay(s1, s2) } -const getURL = (x) => typeof x === 'object' && x !== null ? x.get('location') || x.get('url') : x +// Currently we sort only sites that are not immutableJS and +const getURL = (x) => { + if (typeof x === 'string') { + return x + } + + if (x.get) { + return x.get('location') || x.get('url') + } + + return x.location || x.url +} const getMapListToElements = (urlLocationLower) => ({data, maxResults, type, sortHandler = (x) => x, filterValue = (site) => { @@ -285,69 +287,29 @@ const getMapListToElements = (urlLocationLower) => ({data, maxResults, type, } const getHistorySuggestions = (state, urlLocationLower) => { - return new Promise((resolve, reject) => { - const sortHandler = sortBasedOnLocationPos(urlLocationLower) - const mapListToElements = getMapListToElements(urlLocationLower) - let suggestionsList = Immutable.List() - // NOTE: Iterating sites can take a long time! Please be mindful when - // working with the history and bookmark suggestion code. + /* + * todo: const historySuggestionsOn = getSetting(settings.HISTORY_SUGGESTIONS) const bookmarkSuggestionsOn = getSetting(settings.BOOKMARK_SUGGESTIONS) - const shouldIterateSites = historySuggestionsOn || bookmarkSuggestionsOn - if (shouldIterateSites) { - // Note: Bookmark sites are now included in history. This will allow - // sites to appear in the auto-complete regardless of their bookmark - // status. If history is turned off, bookmarked sites will appear - // in the bookmark section. - const sitesFilter = (site) => { - const location = site.get('location') - if (!location) { - return false - } - const title = site.get('title') - return location.toLowerCase().indexOf(urlLocationLower) !== -1 || - (title && title.toLowerCase().indexOf(urlLocationLower) !== -1) - } - - let historySites = new Immutable.List() - let bookmarkSites = new Immutable.List() - const sites = state.get('sites') - sites.forEach(site => { - if (!sitesFilter(site)) { - return - } - if (historySuggestionsOn && isHistoryEntry(site) && !isDefaultEntry(site)) { - historySites = historySites.push(site) - return - } - if (bookmarkSuggestionsOn && isBookmark(site) && !isDefaultEntry(site)) { - bookmarkSites = bookmarkSites.push(site) - } - }) - - if (historySites.size > 0) { - historySites = historySites.concat(createVirtualHistoryItems(historySites)) + */ - suggestionsList = suggestionsList.concat(mapListToElements({ - data: historySites, - maxResults: config.urlBarSuggestions.maxHistorySites, - type: suggestionTypes.HISTORY, - sortHandler, - filterValue: null - })) - } + return new Promise((resolve, reject) => { + const sortHandler = getSortForSuggestions(urlLocationLower) + const mapListToElements = getMapListToElements(urlLocationLower) + query(urlLocationLower).then((results) => { + results = makeImmutable(results) + results = results.take(config.urlBarSuggestions.maxHistorySites) + results = results.concat(createVirtualHistoryItems(results)) - if (bookmarkSites.size > 0) { - suggestionsList = suggestionsList.concat(mapListToElements({ - data: bookmarkSites, - maxResults: config.urlBarSuggestions.maxBookmarkSites, - type: suggestionTypes.BOOKMARK, - sortHandler, - filterValue: null - })) - } - } - resolve(suggestionsList) + const suggestionsList = mapListToElements({ + data: results, + maxResults: config.urlBarSuggestions.maxHistorySites, + type: suggestionTypes.HISTORY, + sortHandler, + filterValue: null + }) + resolve(suggestionsList) + }) }) } @@ -365,7 +327,7 @@ const getAboutSuggestions = (state, urlLocationLower) => { const getOpenedTabSuggestions = (state, windowId, urlLocationLower) => { return new Promise((resolve, reject) => { - const sortHandler = sortBasedOnLocationPos(urlLocationLower) + const sortHandler = getSortForSuggestions(urlLocationLower) const mapListToElements = getMapListToElements(urlLocationLower) const tabs = getTabsByWindowId(state, windowId) let suggestionsList = Immutable.List() @@ -476,11 +438,11 @@ const generateNewSearchXHRResults = (state, windowId, tabId, input) => { module.exports = { sortingPriority, sortByAccessCountWithAgeDecay, + getSortForSuggestions, simpleDomainNameValue, normalizeLocation, shouldNormalizeLocation, createVirtualHistoryItems, - sortBasedOnLocationPos, getMapListToElements, getHistorySuggestions, getAboutSuggestions, diff --git a/js/constants/config.js b/js/constants/config.js index 3894edf56fb..159ed361399 100644 --- a/js/constants/config.js +++ b/js/constants/config.js @@ -28,8 +28,7 @@ module.exports = { defaultUrl: 'about:newtab', urlBarSuggestions: { maxOpenedFrames: 2, - maxBookmarkSites: 2, - maxHistorySites: 3, + maxHistorySites: 5, maxAboutPages: 2, maxSearch: 3, maxTopSites: 3 diff --git a/test/unit/app/common/lib/siteSuggestionsTest.js b/test/unit/app/common/lib/siteSuggestionsTest.js index 1ba15b5fc42..4fcb3b4c501 100644 --- a/test/unit/app/common/lib/siteSuggestionsTest.js +++ b/test/unit/app/common/lib/siteSuggestionsTest.js @@ -1,6 +1,7 @@ /* global describe, before, it */ const {tokenizeInput, init, query} = require('../../../../../app/common/lib/siteSuggestions') const assert = require('assert') +const Immutable = require('immutable') const site1 = { location: 'https://www.bradrichter.co/bad_numbers/3', @@ -113,4 +114,50 @@ describe('siteSuggestions lib', function () { checkResult('brave brad', [], cb) }) }) + describe('query sorts results', function () { + before(function (cb) { + const sites = Immutable.fromJS([{ + location: 'https://brave.com/twi' + }, { + location: 'https://twitter.com/brave' + }, { + location: 'https://twitter.com/brianbondy' + }, { + location: 'https://twitter.com/_brianclif' + }, { + location: 'https://twitter.com/cezaraugusto' + }, { + location: 'https://bbondy.com/twitter' + }, { + location: 'https://twitter.com' + }, { + location: 'https://twitter.com/i/moments' + }]) + init(sites).then(cb.bind(null, null)) + }) + it('orders shortest match first', function (cb) { + query('twitter.com').then((results) => { + assert.deepEqual(results[0], { location: 'https://twitter.com' }) + cb() + }) + }) + it('matches prefixes first', function (cb) { + query('twi').then((results) => { + assert.deepEqual(results[0], { location: 'https://twitter.com' }) + cb() + }) + }) + it('closest to the left match wins', function (cb) { + query('twitter.com brian').then((results) => { + assert.deepEqual(results[0], { location: 'https://twitter.com/brianbondy' }) + cb() + }) + }) + it('matches based on tokens and not exactly', function (cb) { + query('twitter.com/moments').then((results) => { + assert.deepEqual(results[0], { location: 'https://twitter.com/i/moments' }) + cb() + }) + }) + }) }) From 6fe984d31d55c1d81465dbf97904ccd35741bfb4 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Sat, 13 May 2017 23:42:46 -0400 Subject: [PATCH 11/22] Respect HISTORY and BOOKMARK suggestion settings --- app/common/lib/siteSuggestions.js | 36 ++++++++++++++++++++++--------- app/common/lib/suggestion.js | 15 ++++++------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/app/common/lib/siteSuggestions.js b/app/common/lib/siteSuggestions.js index 3d30a6f70aa..4f1373bda11 100644 --- a/app/common/lib/siteSuggestions.js +++ b/app/common/lib/siteSuggestions.js @@ -3,16 +3,16 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const Bloodhound = require('bloodhound-js') +const siteTags = require('../../../js/constants/siteTags') let initialized = false let engine -let internalSort -let lastQueryInput +let lastQueryOptions // Same as sortByAccessCountWithAgeDecay but if one is a prefix of the // other then it is considered always sorted first. const sortForSuggestions = (s1, s2) => { - return internalSort(s1, s2) + return lastQueryOptions.internalSort(s1, s2) } const getSiteIdentity = (data) => { @@ -37,6 +37,7 @@ const init = (sites) => { return promise } +const getTagToken = (tag) => '|' + tag + '|' const tokenizeInput = (data) => { let url = data || '' let parts = [] @@ -46,6 +47,13 @@ const tokenizeInput = (data) => { if (data.title) { parts = data.title.toLowerCase().split(/\s/) } + if (data.tags) { + parts = parts.concat(data.tags.map(getTagToken)) + } + } else { + if (lastQueryOptions && !lastQueryOptions.historySuggestionsOn && lastQueryOptions.bookmarkSuggestionsOn) { + parts.push(getTagToken(siteTags.BOOKMARK)) + } } if (url) { @@ -56,19 +64,27 @@ const tokenizeInput = (data) => { return parts } -const query = (input) => { +const query = (input, options = {}) => { if (!initialized) { return Promise.resolve([]) } + return new Promise((resolve, reject) => { - lastQueryInput = input || '' + input = (input || '').toLowerCase() const {getSortForSuggestions} = require('./suggestion') - internalSort = getSortForSuggestions(lastQueryInput.toLowerCase()) - engine.search(lastQueryInput, function (results) { - resolve(results) - }, function (err) { - reject(err) + lastQueryOptions = Object.assign({}, options, { + input, + internalSort: getSortForSuggestions(input) }) + if (lastQueryOptions.historySuggestionsOn !== false || lastQueryOptions.bookmarkSuggestionsOn !== false) { + engine.search(input, function (results) { + resolve(results) + }, function (err) { + reject(err) + }) + } else { + resolve([]) + } }) } diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js index e14cb034909..44cc0d7f571 100644 --- a/app/common/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -287,16 +287,15 @@ const getMapListToElements = (urlLocationLower) => ({data, maxResults, type, } const getHistorySuggestions = (state, urlLocationLower) => { - /* - * todo: - const historySuggestionsOn = getSetting(settings.HISTORY_SUGGESTIONS) - const bookmarkSuggestionsOn = getSetting(settings.BOOKMARK_SUGGESTIONS) - */ - return new Promise((resolve, reject) => { const sortHandler = getSortForSuggestions(urlLocationLower) const mapListToElements = getMapListToElements(urlLocationLower) - query(urlLocationLower).then((results) => { + const options = { + historySuggestionsOn: getSetting(settings.HISTORY_SUGGESTIONS), + bookmarkSuggestionsOn: getSetting(settings.BOOKMARK_SUGGESTIONS) + } + + query(urlLocationLower, options).then((results) => { results = makeImmutable(results) results = results.take(config.urlBarSuggestions.maxHistorySites) results = results.concat(createVirtualHistoryItems(results)) @@ -304,7 +303,7 @@ const getHistorySuggestions = (state, urlLocationLower) => { const suggestionsList = mapListToElements({ data: results, maxResults: config.urlBarSuggestions.maxHistorySites, - type: suggestionTypes.HISTORY, + type: options.historySuggestionsOn ? suggestionTypes.HISTORY : suggestionTypes.BOOKMARK, sortHandler, filterValue: null }) From b02eeb3c9a786fd13a5e7ec02331b85026ecaaf6 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Sun, 14 May 2017 00:16:23 -0400 Subject: [PATCH 12/22] Support history after bloodhound init --- .../reducers/urlBarSuggestionsReducer.js | 12 ++++++- app/common/lib/siteSuggestions.js | 16 +++++++-- .../app/common/lib/siteSuggestionsTest.js | 33 ++++++++++++++++++- 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/app/browser/reducers/urlBarSuggestionsReducer.js b/app/browser/reducers/urlBarSuggestionsReducer.js index 59df16bb383..b98c34a7e56 100644 --- a/app/browser/reducers/urlBarSuggestionsReducer.js +++ b/app/browser/reducers/urlBarSuggestionsReducer.js @@ -6,10 +6,20 @@ const appConstants = require('../../../js/constants/appConstants') const {generateNewSuggestionsList, generateNewSearchXHRResults} = require('../../common/lib/suggestion') -const {init} = require('../../common/lib/siteSuggestions') +const {init, add} = require('../../common/lib/siteSuggestions') +const Immutable = require('immutable') const urlBarSuggestionsReducer = (state, action) => { switch (action.actionType) { + case appConstants.APP_ADD_SITE: + if (Immutable.List.isList(action.siteDetail)) { + action.siteDetail.forEach((s) => { + add(s) + }) + } else { + add(action.siteDetail) + } + break case appConstants.APP_SET_STATE: init(Object.values(action.appState.get('sites').toJS())) break diff --git a/app/common/lib/siteSuggestions.js b/app/common/lib/siteSuggestions.js index 4f1373bda11..5546f466d83 100644 --- a/app/common/lib/siteSuggestions.js +++ b/app/common/lib/siteSuggestions.js @@ -64,14 +64,25 @@ const tokenizeInput = (data) => { return parts } +const add = (data) => { + if (!initialized) { + return + } + if (typeof data === 'string') { + engine.add(data) + } else { + engine.add(data.toJS ? data.toJS() : data) + } +} + const query = (input, options = {}) => { if (!initialized) { return Promise.resolve([]) } return new Promise((resolve, reject) => { - input = (input || '').toLowerCase() - const {getSortForSuggestions} = require('./suggestion') + const {getSortForSuggestions, normalizeLocation} = require('./suggestion') + input = normalizeLocation((input || '').toLowerCase()) lastQueryOptions = Object.assign({}, options, { input, internalSort: getSortForSuggestions(input) @@ -90,6 +101,7 @@ const query = (input, options = {}) => { module.exports = { init, + add, tokenizeInput, query } diff --git a/test/unit/app/common/lib/siteSuggestionsTest.js b/test/unit/app/common/lib/siteSuggestionsTest.js index 4fcb3b4c501..d7c5e6e640f 100644 --- a/test/unit/app/common/lib/siteSuggestionsTest.js +++ b/test/unit/app/common/lib/siteSuggestionsTest.js @@ -1,5 +1,5 @@ /* global describe, before, it */ -const {tokenizeInput, init, query} = require('../../../../../app/common/lib/siteSuggestions') +const {tokenizeInput, init, query, add} = require('../../../../../app/common/lib/siteSuggestions') const assert = require('assert') const Immutable = require('immutable') @@ -160,4 +160,35 @@ describe('siteSuggestions lib', function () { }) }) }) + describe('add sites after init', function () { + before(function (cb) { + const sites = [site1, site2, site3, site4] + init(sites).then(() => { + add({ + location: 'https://slack.com' + }) + }).then(cb.bind(null, null)) + }) + it('can be found', function (cb) { + checkResult('slack', [{ location: 'https://slack.com' }], cb) + }) + it('adding twice results in 1 result only', function (cb) { + add({ + location: 'https://slack.com' + }) + checkResult('slack', [{ location: 'https://slack.com' }], cb) + }) + it('can add simple strings', function (cb) { + add({ + location: 'https://slashdot.org' + }) + checkResult('slash', [{ location: 'https://slashdot.org' }], cb) + }) + it('can add Immutable objects', function (cb) { + add(Immutable.fromJS({ + location: 'https://microsoft.com' + })) + checkResult('micro', [{ location: 'https://microsoft.com' }], cb) + }) + }) }) From 7ff4a150902cb8d270564d7ceb46b5ee0184b666 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Sun, 14 May 2017 07:52:49 -0400 Subject: [PATCH 13/22] Add tests for urlBarSuggestionsReducer Also fixes various other failing unit tests --- .../reducers/urlBarSuggestionsReducerTest.js | 99 +++++++++++++++++++ test/unit/app/common/lib/suggestionTest.js | 3 + .../navigation/urlBarSuggestionItemTest.js | 9 +- .../renderer/reducers/urlBarReducerTest.js | 21 +++- 4 files changed, 124 insertions(+), 8 deletions(-) create mode 100644 test/unit/app/browser/reducers/urlBarSuggestionsReducerTest.js diff --git a/test/unit/app/browser/reducers/urlBarSuggestionsReducerTest.js b/test/unit/app/browser/reducers/urlBarSuggestionsReducerTest.js new file mode 100644 index 00000000000..fb5eeeebd86 --- /dev/null +++ b/test/unit/app/browser/reducers/urlBarSuggestionsReducerTest.js @@ -0,0 +1,99 @@ +/* global describe, it, before, after, afterEach */ +const mockery = require('mockery') +const sinon = require('sinon') +const Immutable = require('immutable') +const assert = require('assert') + +const appConstants = require('../../../../../js/constants/appConstants') +require('../../../braveUnit') + +describe('urlBarSuggestionsReducer', function () { + let urlBarSuggestionsReducer + before(function () { + mockery.enable({ + warnOnReplace: false, + warnOnUnregistered: false, + useCleanCache: true + }) + + this.siteSuggestionsStub = { + init: sinon.stub(), + add: sinon.stub() + } + mockery.registerMock('../../common/lib/siteSuggestions', this.siteSuggestionsStub) + + this.suggestionStub = { + generateNewSuggestionsList: sinon.stub(), + generateNewSearchXHRResults: sinon.stub() + } + mockery.registerMock('../../common/lib/suggestion', this.suggestionStub) + + urlBarSuggestionsReducer = require('../../../../../app/browser/reducers/urlBarSuggestionsReducer') + }) + + after(function () { + mockery.disable() + }) + + afterEach(function () { + this.siteSuggestionsStub.init.reset() + this.siteSuggestionsStub.add.reset() + this.suggestionStub.generateNewSuggestionsList.reset() + this.suggestionStub.generateNewSearchXHRResults.reset() + }) + + const site1 = { + location: 'https://brave.com' + } + + const initState = Immutable.fromJS({ + sites: { + 'key': site1 + } + }) + + describe('APP_SET_STATE', function () { + it('inits the suggestions lib with sites', function () { + urlBarSuggestionsReducer(Immutable.Map(), {actionType: appConstants.APP_SET_STATE, appState: initState}) + const callCount = this.siteSuggestionsStub.init.calledOnce + assert.equal(callCount, 1) + assert.deepEqual(this.siteSuggestionsStub.init.args[0][0], [site1]) + }) + }) + describe('APP_ADD_SITE', function () { + it('adds a site in the suggestions lib', function () { + const newState = urlBarSuggestionsReducer(initState, {actionType: appConstants.APP_ADD_SITE, siteDetail: site1}) + const callCount = this.siteSuggestionsStub.add.calledOnce + assert.equal(callCount, 1) + assert.deepEqual(this.siteSuggestionsStub.add.args[0][0], site1) + assert.deepEqual(newState, initState) + }) + }) + describe('APP_URL_BAR_TEXT_CHANGED', function () { + it('regenerates suggestions', function () { + const windowId = 1 + const tabId = 42 + const input = 'hello world' + const newState = urlBarSuggestionsReducer(initState, {actionType: appConstants.APP_URL_BAR_TEXT_CHANGED, tabId, windowId, input}) + const callCount = this.suggestionStub.generateNewSuggestionsList.calledOnce + assert.equal(callCount, 1) + assert.deepEqual(this.suggestionStub.generateNewSuggestionsList.args[0][0], initState) + assert.deepEqual(this.suggestionStub.generateNewSuggestionsList.args[0][1], windowId) + assert.deepEqual(this.suggestionStub.generateNewSuggestionsList.args[0][2], tabId) + assert.deepEqual(this.suggestionStub.generateNewSuggestionsList.args[0][3], input) + assert.deepEqual(newState, initState) + + const xhrCallCount = this.suggestionStub.generateNewSearchXHRResults.calledOnce + assert.equal(xhrCallCount, 1) + assert.deepEqual(this.suggestionStub.generateNewSearchXHRResults.args[0][0], initState) + assert.deepEqual(this.suggestionStub.generateNewSearchXHRResults.args[0][1], windowId) + assert.deepEqual(this.suggestionStub.generateNewSearchXHRResults.args[0][2], tabId) + assert.deepEqual(this.suggestionStub.generateNewSearchXHRResults.args[0][3], input) + }) + }) + describe('APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE', function () { + it.skip('starts generating new suggestions', function () { + // TODO + }) + }) +}) diff --git a/test/unit/app/common/lib/suggestionTest.js b/test/unit/app/common/lib/suggestionTest.js index 3bb419774e5..c91918992ad 100644 --- a/test/unit/app/common/lib/suggestionTest.js +++ b/test/unit/app/common/lib/suggestionTest.js @@ -17,6 +17,9 @@ const AGE_DECAY = 50 const fakeImmutableUtil = { makeImmutable: (obj) => { return makeImmutable(obj) + }, + isList: (obj) => { + return Immutable.List.isList(obj) } } diff --git a/test/unit/app/renderer/components/navigation/urlBarSuggestionItemTest.js b/test/unit/app/renderer/components/navigation/urlBarSuggestionItemTest.js index a99da2394a3..b1cbb3c8c20 100644 --- a/test/unit/app/renderer/components/navigation/urlBarSuggestionItemTest.js +++ b/test/unit/app/renderer/components/navigation/urlBarSuggestionItemTest.js @@ -7,6 +7,7 @@ const mockery = require('mockery') const {mount} = require('enzyme') const assert = require('assert') const sinon = require('sinon') +const Immutable = require('immutable') const fakeElectron = require('../../../../lib/fakeElectron') const suggestionTypes = require('../../../../../../js/constants/suggestionTypes') let UrlBarSuggestionItem @@ -31,11 +32,11 @@ describe('UrlBarSuggestionItem component', function () { before(function () { this.onMouseOver = sinon.spy() this.onSuggestionClicked = sinon.spy() - this.suggestion = { + this.suggestion = Immutable.fromJS({ title: 'hello', type: suggestionType, location: 'http://www.brave.com' - } + }) this.result = mount( Date: Mon, 15 May 2017 21:59:54 -0400 Subject: [PATCH 14/22] Fix search suggestions in browser process --- .../reducers/urlBarSuggestionsReducer.js | 5 ++-- app/common/lib/fetchSearchSuggestions.js | 26 ++++++++++++------- app/common/lib/suggestion.js | 9 ++----- app/renderer/reducers/urlBarReducer.js | 6 +---- docs/state.md | 2 +- js/actions/appActions.js | 8 ++---- .../reducers/urlBarSuggestionsReducerTest.js | 5 ---- .../app/common/lib/siteSuggestionsTest.js | 15 ++++++++++- .../renderer/reducers/urlBarReducerTest.js | 8 ------ 9 files changed, 40 insertions(+), 44 deletions(-) diff --git a/app/browser/reducers/urlBarSuggestionsReducer.js b/app/browser/reducers/urlBarSuggestionsReducer.js index b98c34a7e56..3ea89981344 100644 --- a/app/browser/reducers/urlBarSuggestionsReducer.js +++ b/app/browser/reducers/urlBarSuggestionsReducer.js @@ -8,6 +8,7 @@ const appConstants = require('../../../js/constants/appConstants') const {generateNewSuggestionsList, generateNewSearchXHRResults} = require('../../common/lib/suggestion') const {init, add} = require('../../common/lib/siteSuggestions') const Immutable = require('immutable') +const {makeImmutable} = require('../../common/state/immutableUtil') const urlBarSuggestionsReducer = (state, action) => { switch (action.actionType) { @@ -28,8 +29,8 @@ const urlBarSuggestionsReducer = (state, action) => { generateNewSearchXHRResults(state, action.windowId, action.tabId, action.input) break case appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE: - // TODO: Find a way to get urlLocation and also convert search suggestions to fetch - // generateNewSuggestionsList(state, action.windowId, action.tabId, urlLocation, action.searchResults) + state = state.set('searchResults', makeImmutable(action.searchResults)) + generateNewSuggestionsList(state, action.windowId, action.tabId) break } return state diff --git a/app/common/lib/fetchSearchSuggestions.js b/app/common/lib/fetchSearchSuggestions.js index 35c8cfe58ec..479da80a006 100644 --- a/app/common/lib/fetchSearchSuggestions.js +++ b/app/common/lib/fetchSearchSuggestions.js @@ -2,20 +2,28 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ -const Immutable = require('immutable') const appActions = require('../../../js/actions/appActions') const debounce = require('../../../js/lib/debounce') +const {request} = require('../../../js/lib/request') const fetchSearchSuggestions = debounce((windowId, tabId, autocompleteURL, searchTerms) => { - const xhr = new window.XMLHttpRequest() - xhr.open('GET', autocompleteURL - .replace('{searchTerms}', encodeURIComponent(searchTerms)), true) - xhr.responseType = 'json' - xhr.send() - xhr.onload = () => { + autocompleteURL.replace('{searchTerms}', encodeURIComponent(searchTerms)) + request(autocompleteURL.replace('{searchTerms}', encodeURIComponent(searchTerms)), (err, response, body) => { + if (err) { + return + } + + let searchResults + try { + searchResults = JSON.parse(body)[1] + } catch (e) { + console.warn(e) + return + } + // Once we have the online suggestions, append them to the others - appActions.searchSuggestionResultsAvailable(windowId, tabId, Immutable.fromJS(xhr.response[1])) - } + appActions.searchSuggestionResultsAvailable(tabId, searchResults) + }) }, 100) module.exports = fetchSearchSuggestions diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js index 44cc0d7f571..15844d1e4d8 100644 --- a/app/common/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -353,12 +353,7 @@ const getSearchSuggestions = (state, tabId, urlLocationLower) => { const mapListToElements = getMapListToElements(urlLocationLower) let suggestionsList = Immutable.List() if (getSetting(settings.OFFER_SEARCH_SUGGESTIONS)) { - const frame = getFrameByTabId(state, tabId) - if (!frame) { - resolve(suggestionsList) - return - } - const searchResults = frame.getIn(['navbar', 'urlbar', 'suggestions', 'searchResults']) + const searchResults = state.get('searchResults') if (searchResults) { suggestionsList = mapListToElements({ data: searchResults, @@ -430,7 +425,7 @@ const generateNewSearchXHRResults = (state, windowId, tabId, input) => { fetchSearchSuggestions(windowId, tabId, autocompleteURL, input) } else { const appActions = require('../../../js/actions/appActions') - appActions.searchSuggestionResultsAvailable(windowId, tabId, Immutable.List()) + appActions.searchSuggestionResultsAvailable(tabId, Immutable.List()) } } diff --git a/app/renderer/reducers/urlBarReducer.js b/app/renderer/reducers/urlBarReducer.js index d82ed5e93bf..170793e5578 100644 --- a/app/renderer/reducers/urlBarReducer.js +++ b/app/renderer/reducers/urlBarReducer.js @@ -8,7 +8,7 @@ const windowConstants = require('../../../js/constants/windowConstants') const appConstants = require('../../../js/constants/appConstants') const {isUrl, getSourceAboutUrl, getSourceMagnetUrl} = require('../../../js/lib/appUrlUtil') const {isURL, isPotentialPhishingUrl, getUrlFromInput} = require('../../../js/lib/urlutil') -const {getFrameByKey, getFrameKeyByTabId, activeFrameStatePath, frameStatePath, getActiveFrame, getFrameByTabId} = require('../../../js/state/frameStateUtil') +const {getFrameByKey, activeFrameStatePath, frameStatePath, getActiveFrame, getFrameByTabId} = require('../../../js/state/frameStateUtil') const searchProviders = require('../../../js/data/searchProviders') const Immutable = require('immutable') const {navigateSiteClickHandler} = require('../suggestionClickHandlers') @@ -264,10 +264,6 @@ const urlBarReducer = (state, action) => { state = updateUrlSuffix(state, state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'suggestionList']), suggestionList)) break } - case appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE: - const frameKey = getFrameKeyByTabId(state, action.tabId) - state = state.setIn(frameStatePath(state, frameKey).concat(['navbar', 'urlbar', 'suggestions', 'searchResults']), action.searchResults) - break case windowConstants.WINDOW_URL_BAR_AUTOCOMPLETE_ENABLED: state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'autocompleteEnabled']), action.enabled) break diff --git a/docs/state.md b/docs/state.md index 5077232623e..63f901a000a 100644 --- a/docs/state.md +++ b/docs/state.md @@ -465,7 +465,6 @@ WindowStore selected: boolean, // is the urlbar text selected suggestions: { autocompleteEnabled: boolean, // used to enable or disable autocomplete - searchResults: array, // autocomplete server results if enabled selectedIndex: number, // index of the item in focus shouldRender: boolean, // if the suggestions should render suggestionList: { @@ -628,6 +627,7 @@ WindowStore autocompleteURL: string, // ditto re: {searchTerms} searchURL: string // with replacement var in string: {searchTerms} }, + searchResults: array, // autocomplete server results if enabled ui: { bookmarksToolbar: { selectedFolderId: number // folderId from the siteDetail of the currently expanded folder diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 46ad4bcff7a..54c9fbdd530 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -1365,15 +1365,11 @@ const appActions = { * @param {number} tabId - the tab id for the action * @param searchResults The search results for the currently entered URL bar text. */ - searchSuggestionResultsAvailable: function (windowId, tabId, searchResults) { + searchSuggestionResultsAvailable: function (tabId, searchResults) { dispatch({ actionType: appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE, tabId, - windowId, - searchResults, - queryInfo: { - windowId - } + searchResults }) }, diff --git a/test/unit/app/browser/reducers/urlBarSuggestionsReducerTest.js b/test/unit/app/browser/reducers/urlBarSuggestionsReducerTest.js index fb5eeeebd86..b3cec29ea84 100644 --- a/test/unit/app/browser/reducers/urlBarSuggestionsReducerTest.js +++ b/test/unit/app/browser/reducers/urlBarSuggestionsReducerTest.js @@ -91,9 +91,4 @@ describe('urlBarSuggestionsReducer', function () { assert.deepEqual(this.suggestionStub.generateNewSearchXHRResults.args[0][3], input) }) }) - describe('APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE', function () { - it.skip('starts generating new suggestions', function () { - // TODO - }) - }) }) diff --git a/test/unit/app/common/lib/siteSuggestionsTest.js b/test/unit/app/common/lib/siteSuggestionsTest.js index d7c5e6e640f..c03d4223a54 100644 --- a/test/unit/app/common/lib/siteSuggestionsTest.js +++ b/test/unit/app/common/lib/siteSuggestionsTest.js @@ -1,7 +1,9 @@ -/* global describe, before, it */ +/* global describe, before, after, it */ const {tokenizeInput, init, query, add} = require('../../../../../app/common/lib/siteSuggestions') const assert = require('assert') const Immutable = require('immutable') +const fakeElectron = require('../../../lib/fakeElectron') +const mockery = require('mockery') const site1 = { location: 'https://www.bradrichter.co/bad_numbers/3', @@ -24,6 +26,17 @@ const site4 = { require('../../../braveUnit') describe('siteSuggestions lib', function () { + before(function () { + mockery.enable({ + warnOnReplace: false, + warnOnUnregistered: false, + useCleanCache: true + }) + mockery.registerMock('electron', fakeElectron) + }) + after(function () { + mockery.disable() + }) describe('tokenizeInput', function () { it('empty string has no tokens', function () { assert.deepEqual(tokenizeInput(''), []) diff --git a/test/unit/app/renderer/reducers/urlBarReducerTest.js b/test/unit/app/renderer/reducers/urlBarReducerTest.js index d82efb06f72..a1c10b82e6d 100644 --- a/test/unit/app/renderer/reducers/urlBarReducerTest.js +++ b/test/unit/app/renderer/reducers/urlBarReducerTest.js @@ -294,14 +294,6 @@ describe('urlBarReducer', function () { }) }) - describe('APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE', function () { - it('turns off suggestions', function () { - const searchResults = Immutable.fromJS(['0110001001110010011010010110000101101110']) - const newState = urlBarReducer(windowState, {actionType: appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE, searchResults, tabId: 2}) - assert.deepEqual(newState.getIn(['frames', 1, 'navbar', 'urlbar', 'suggestions', 'searchResults']).toJS(), searchResults.toJS()) - }) - }) - describe('APP_URL_BAR_TEXT_CHANGED', function () { // TODO }) From eeda733b3a16f98e38765d9addf2afb22b4ab2df Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Mon, 15 May 2017 22:25:22 -0400 Subject: [PATCH 15/22] Debounce on suggestion generation --- app/common/lib/fetchSearchSuggestions.js | 5 ++--- app/common/lib/suggestion.js | 9 +++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/common/lib/fetchSearchSuggestions.js b/app/common/lib/fetchSearchSuggestions.js index 479da80a006..77b7d6e38c6 100644 --- a/app/common/lib/fetchSearchSuggestions.js +++ b/app/common/lib/fetchSearchSuggestions.js @@ -3,10 +3,9 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const appActions = require('../../../js/actions/appActions') -const debounce = require('../../../js/lib/debounce') const {request} = require('../../../js/lib/request') -const fetchSearchSuggestions = debounce((windowId, tabId, autocompleteURL, searchTerms) => { +const fetchSearchSuggestions = (windowId, tabId, autocompleteURL, searchTerms) => { autocompleteURL.replace('{searchTerms}', encodeURIComponent(searchTerms)) request(autocompleteURL.replace('{searchTerms}', encodeURIComponent(searchTerms)), (err, response, body) => { if (err) { @@ -24,6 +23,6 @@ const fetchSearchSuggestions = debounce((windowId, tabId, autocompleteURL, searc // Once we have the online suggestions, append them to the others appActions.searchSuggestionResultsAvailable(tabId, searchResults) }) -}, 100) +} module.exports = fetchSearchSuggestions diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js index 15844d1e4d8..0638d42610b 100644 --- a/app/common/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -16,6 +16,7 @@ const top500 = require('../../../js/data/top500') const fetchSearchSuggestions = require('./fetchSearchSuggestions') const {getFrameByTabId, getTabsByWindowId} = require('../../common/state/tabState') const {query} = require('./siteSuggestions') +const debounce = require('../../../js/lib/debounce') const sigmoid = (t) => { return 1 / (1 + Math.pow(Math.E, -t)) @@ -378,7 +379,7 @@ const getAlexaSuggestions = (state, urlLocationLower) => { }) } -const generateNewSuggestionsList = (state, windowId, tabId, urlLocation) => { +const generateNewSuggestionsList = debounce((state, windowId, tabId, urlLocation) => { if (!urlLocation) { return } @@ -394,9 +395,9 @@ const generateNewSuggestionsList = (state, windowId, tabId, urlLocation) => { // Flatten only 1 level deep for perf only, nested will be objects within arrrays appActions.urlBarSuggestionsChanged(windowId, makeImmutable(suggestionsLists).flatten(1)) }) -} +}, 5) -const generateNewSearchXHRResults = (state, windowId, tabId, input) => { +const generateNewSearchXHRResults = debounce((state, windowId, tabId, input) => { const frame = getFrameByTabId(state, tabId) if (!frame) { // Frame info may not be available yet in app store @@ -427,7 +428,7 @@ const generateNewSearchXHRResults = (state, windowId, tabId, input) => { const appActions = require('../../../js/actions/appActions') appActions.searchSuggestionResultsAvailable(tabId, Immutable.List()) } -} +}, 100) module.exports = { sortingPriority, From 6d3cade0678cb68a750f4b9711c986401e5c7468 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Tue, 16 May 2017 00:51:29 -0400 Subject: [PATCH 16/22] Remove some imperative actions for clearing suggestion selectedIndex --- app/renderer/components/navigation/urlBar.js | 15 +++++++-------- js/actions/appActions.js | 2 +- js/stores/windowStore.js | 1 + test/unit/app/common/lib/suggestionTest.js | 6 ++++-- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 74e3e0ea51b..83082090ffe 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -37,6 +37,7 @@ const UrlUtil = require('../../../../js/lib/urlutil') const {eventElHasAncestorWithClasses, isForSecondaryAction} = require('../../../../js/lib/eventUtil') const {getBaseUrl, isUrl, isIntermediateAboutPage} = require('../../../../js/lib/appUrlUtil') const {getCurrentWindowId} = require('../../currentWindow') +const {normalizeLocation} = require('../../../common/lib/suggestion') // Icons const iconNoScript = require('../../../../img/url-bar-no-script.svg') @@ -62,7 +63,7 @@ class UrlBar extends React.Component { if (this.keyPressed || !this.urlInput || this.props.locationValueSuffix.length === 0) { return } - this.updateAutocomplete(this.lastVal, this.props.locationValue + this.props.locationValueSuffix) + this.updateAutocomplete(this.lastVal) }, 10) } @@ -97,7 +98,6 @@ class UrlBar extends React.Component { if (this.props.autocompleteEnabled) { windowActions.urlBarAutocompleteEnabled(false) } - appActions.urlBarSuggestionsChanged(getCurrentWindowId(), undefined, null) windowActions.setRenderUrlBarSuggestions(false) } @@ -229,7 +229,11 @@ class UrlBar extends React.Component { } } - updateAutocomplete (newValue, suggestion = this.lastVal + this.lastSuffix) { + updateAutocomplete (newValue) { + let suggestion = '' + if (this.props.suggestionList && this.props.suggestionList.size > 0) { + suggestion = normalizeLocation(this.props.suggestionList.getIn([this.activeIndex || 0, 'location'])) || '' + } if (suggestion.startsWith(newValue)) { const newSuffix = suggestion.substring(newValue.length) this.setValue(newValue, newSuffix) @@ -272,8 +276,6 @@ class UrlBar extends React.Component { onChange (e) { if (e.target.value !== this.lastVal + this.lastSuffix) { e.preventDefault() - // clear any current arrow or mouse hover selection - appActions.urlBarSuggestionsChanged(getCurrentWindowId(), undefined, null) this.setValue(e.target.value) } } @@ -310,8 +312,6 @@ class UrlBar extends React.Component { if (this.props.isSelected) { windowActions.setUrlBarSelected(false) } - // clear any current arrow or mouse hover selection - appActions.urlBarSuggestionsChanged(getCurrentWindowId(), undefined, null) this.keyPressed = false appActions.urlBarTextChanged(getCurrentWindowId(), this.props.activeTabId, this.lastVal) } @@ -368,7 +368,6 @@ class UrlBar extends React.Component { if (this.props.isFocused) { this.focus() } - appActions.urlBarSuggestionsChanged(getCurrentWindowId(), undefined, null) windowActions.setRenderUrlBarSuggestions(false) } else if (this.props.location !== prevProps.location) { // This is a url nav change diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 54c9fbdd530..5dfcbec4ba8 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -198,7 +198,7 @@ const appActions = { * @param {Number} index - the new index */ tabIndexChanged: function (tabId, index) { - AppDispatcher.dispatch({ + dispatch({ actionType: appConstants.APP_TAB_INDEX_CHANGED, tabId, index diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index 6ff253b6efb..feb8dde0578 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -24,6 +24,7 @@ const Serializer = require('../dispatcher/serializer') const {updateTabPageIndex} = require('../../app/renderer/lib/tabUtil') const assert = require('assert') const contextMenuState = require('../../app/common/state/contextMenuState') +const appStoreRenderer = require('./appStoreRenderer') let windowState = Immutable.fromJS({ activeFrameKey: null, diff --git a/test/unit/app/common/lib/suggestionTest.js b/test/unit/app/common/lib/suggestionTest.js index c91918992ad..4e8c4f11c73 100644 --- a/test/unit/app/common/lib/suggestionTest.js +++ b/test/unit/app/common/lib/suggestionTest.js @@ -8,20 +8,21 @@ const assert = require('assert') const sinon = require('sinon') const Immutable = require('immutable') const {makeImmutable} = require('../../../../../app/common/state/immutableUtil') +const fakeElectron = require('../../../lib/fakeElectron') const _ = require('underscore') let suggestion require('../../../braveUnit') const AGE_DECAY = 50 -const fakeImmutableUtil = { +const fakeImmutableUtil = Object.assign({ makeImmutable: (obj) => { return makeImmutable(obj) }, isList: (obj) => { return Immutable.List.isList(obj) } -} +}, require('../../../../../app/common/state/immutableUtil')) describe('suggestion unit tests', function () { let makeImmutableSpy @@ -35,6 +36,7 @@ describe('suggestion unit tests', function () { makeImmutableSpy = sinon.spy(fakeImmutableUtil, 'makeImmutable') mockery.registerMock('../../common/state/immutableUtil', fakeImmutableUtil) + mockery.registerMock('electron', fakeElectron) suggestion = require('../../../../../app/common/lib/suggestion') }) From 46c7bf0265cb05338fbe504a4cc64353a4ca6962 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Tue, 16 May 2017 16:18:22 -0400 Subject: [PATCH 17/22] Tokenize with urlParse and for non urls --- app/common/lib/siteSuggestions.js | 42 ++++++++-- app/common/lib/suggestion.js | 26 +++--- app/renderer/components/navigation/urlBar.js | 9 ++- .../urlBarSuggestionsTest.js | 15 ++-- test/navbar-components/urlBarTest.js | 8 +- .../app/common/lib/siteSuggestionsTest.js | 81 ++++++++++++++++--- test/unit/app/common/lib/suggestionTest.js | 6 +- 7 files changed, 140 insertions(+), 47 deletions(-) diff --git a/app/common/lib/siteSuggestions.js b/app/common/lib/siteSuggestions.js index 5546f466d83..4c56b7bb52b 100644 --- a/app/common/lib/siteSuggestions.js +++ b/app/common/lib/siteSuggestions.js @@ -3,7 +3,9 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const Bloodhound = require('bloodhound-js') +const {isUrl} = require('../../../js/lib/appUrlUtil') const siteTags = require('../../../js/constants/siteTags') +const urlParse = require('../urlParse') let initialized = false let engine @@ -37,15 +39,24 @@ const init = (sites) => { return promise } +const getPartsFromNonUrlInput = (input) => + input.toLowerCase().split(/[,-.\s\\/?&]/) + const getTagToken = (tag) => '|' + tag + '|' + const tokenizeInput = (data) => { let url = data || '' let parts = [] if (typeof data === 'object' && data !== null) { + // When lastAccessTime is 1 it is a default built-in entry which we don't want + // to appear in suggestions. + if (data.lastAccessedTime === 1) { + return [] + } url = data.location if (data.title) { - parts = data.title.toLowerCase().split(/\s/) + parts = getPartsFromNonUrlInput(data.title) } if (data.tags) { parts = parts.concat(data.tags.map(getTagToken)) @@ -56,12 +67,27 @@ const tokenizeInput = (data) => { } } - if (url) { - url = url.toLowerCase().replace(/^https?:\/\//i, '') - parts = parts.concat(url.split(/[.\s\\/?&]/)) + if (url && isUrl(url)) { + const parsedUrl = urlParse(url.toLowerCase()) + if (parsedUrl.hash) { + parts.push(parsedUrl.hash.slice(1)) + } + if (parsedUrl.host) { + parts = parts.concat(parsedUrl.host.split('.')) + } + if (parsedUrl.pathname) { + parts = parts.concat(parsedUrl.pathname.split(/[.\s\\/]/)) + } + if (parsedUrl.query) { + parts = parts.concat(parsedUrl.query.split(/[&=]/)) + } + if (parsedUrl.protocol) { + parts = parts.concat(parsedUrl.protocol) + } + } else if (url) { + parts = parts.concat(getPartsFromNonUrlInput(url)) } - - return parts + return parts.filter(x => !!x) } const add = (data) => { @@ -81,8 +107,8 @@ const query = (input, options = {}) => { } return new Promise((resolve, reject) => { - const {getSortForSuggestions, normalizeLocation} = require('./suggestion') - input = normalizeLocation((input || '').toLowerCase()) + const {getSortForSuggestions} = require('./suggestion') + input = (input || '').toLowerCase() lastQueryOptions = Object.assign({}, options, { input, internalSort: getSortForSuggestions(input) diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js index 0638d42610b..92a6e68a39c 100644 --- a/app/common/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -99,18 +99,18 @@ const sortByAccessCountWithAgeDecay = (s1, s2) => { } /* - * Return a 1 if the url is 'simple' as in without query, search or - * hash components. Return 0 otherwise. + * Return true1 the url is 'simple' as in without query, search or + * hash components. Return false otherwise. * - * @param {ImmutableObject} site - object represent history entry + * @param {object} An already normalized simple URL * */ -const simpleDomainNameValue = (site) => { +const isSimpleDomainNameValue = (site) => { const parsed = urlParse(getURL(site)) if (parsed.hash === null && parsed.search === null && parsed.query === null && parsed.pathname === '/') { - return 1 + return true } else { - return 0 + return false } } @@ -231,7 +231,16 @@ const getSortForSuggestions = (userInputLower) => (s1, s2) => { if (pos1 !== pos2) { return pos1 - pos2 } - return url1.length - url2.length + const url1IsSimple = isSimpleDomainNameValue(s1) + const url2IsSimple = isSimpleDomainNameValue(s2) + // If either both false or both true then rely on count w/ decay sort + if (url1IsSimple === url2IsSimple) { + return sortByAccessCountWithAgeDecay(s1, s2) + } + if (url1IsSimple) { + return -1 + } + return 1 } if (pos1 !== -1 && pos2 === -1) { return -1 @@ -404,7 +413,6 @@ const generateNewSearchXHRResults = debounce((state, windowId, tabId, input) => return } const frameSearchDetail = frame.getIn(['navbar', 'urlbar', 'searchDetail']) - // TODO: Migrate searchDetail to app state const searchDetail = state.get('searchDetail') if (!searchDetail && !frameSearchDetail) { return @@ -434,7 +442,7 @@ module.exports = { sortingPriority, sortByAccessCountWithAgeDecay, getSortForSuggestions, - simpleDomainNameValue, + isSimpleDomainNameValue, normalizeLocation, shouldNormalizeLocation, createVirtualHistoryItems, diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 83082090ffe..59ae51a097e 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -231,11 +231,14 @@ class UrlBar extends React.Component { updateAutocomplete (newValue) { let suggestion = '' + let suggestionNormalized = '' if (this.props.suggestionList && this.props.suggestionList.size > 0) { - suggestion = normalizeLocation(this.props.suggestionList.getIn([this.activeIndex || 0, 'location'])) || '' + suggestion = this.props.suggestionList.getIn([this.activeIndex || 0, 'location']) || '' + suggestionNormalized = normalizeLocation(suggestion) } - if (suggestion.startsWith(newValue)) { - const newSuffix = suggestion.substring(newValue.length) + const newValueNormalized = normalizeLocation(newValue) + if (suggestionNormalized.startsWith(newValueNormalized) && suggestionNormalized.length > 0) { + const newSuffix = suggestionNormalized.substring(newValueNormalized.length) this.setValue(newValue, newSuffix) this.urlInput.setSelectionRange(newValue.length, newValue.length + newSuffix.length + 1) return true diff --git a/test/navbar-components/urlBarSuggestionsTest.js b/test/navbar-components/urlBarSuggestionsTest.js index 9f673a607e8..c1ce2cfb8c6 100644 --- a/test/navbar-components/urlBarSuggestionsTest.js +++ b/test/navbar-components/urlBarSuggestionsTest.js @@ -46,7 +46,7 @@ describe('urlBarSuggestions', function () { it('show suggestion when single letter is typed in', function * () { yield this.app.client.ipcSend('shortcut-focus-url') .waitForElementFocus(urlInput) - .setInputText(urlInput, 'a') + .setInputText(urlInput, 'about:about') .waitForExist(urlBarSuggestions) }) @@ -103,13 +103,13 @@ describe('urlBarSuggestions', function () { .keys(Brave.keys.DOWN) .waitForExist(urlBarSuggestions + ' li.suggestionItem[data-index="1"].selected') .keys(Brave.keys.ENTER) - .tabByIndex(1).getUrl().should.become(this.page1Url) + .tabByIndex(1).getUrl().should.become(this.page2Url) }) it('selects a location auto complete result but not for titles', function * () { yield this.app.client .setValue(urlInput, 'http://') - .waitForInputText(urlInput, Brave.server.urlOrigin()) + .waitForInputText(urlInput, this.page1Url) .waitForExist(urlBarSuggestions + ' li.selected') .setValue(urlInput, 'Page') .waitForElementCount(urlBarSuggestions + ' li.selected', 0) @@ -124,9 +124,8 @@ describe('urlBarSuggestions', function () { // 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 .keys(pagePartialUrl) - .waitForInputText(urlInput, page2Url) // after entering partial URL matching two options, 1st is tentatively filled in (_without_ moving cursor to end) + .waitForInputText(urlInput, 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)') .waitForInputText(urlInput, page1Url) // mousing over 2nd option tentatively completes URL with 2nd option (_without_ moving cursor to end) .keys('2.html') .waitForInputText(urlInput, page2Url) // without moving mouse, typing rest of 1st option URL overwrites the autocomplete from mouseover @@ -134,7 +133,7 @@ describe('urlBarSuggestions', function () { it('selection is not reset when pressing non-input key', function * () { const pagePartialUrl = Brave.server.url('page') - const page1Url = Brave.server.url('page1.html') + const page2Url = Brave.server.url('page2.html') aboutHistoryState.setHistory(Immutable.fromJS({ about: { history: { @@ -148,10 +147,10 @@ describe('urlBarSuggestions', function () { .setValue(urlInput, pagePartialUrl) .waitForVisible(urlBarSuggestions) .keys(Brave.keys.DOWN) - .waitForInputText(urlInput, page1Url) + .waitForInputText(urlInput, page2Url) .keys(Brave.keys.CONTROL) .keys(Brave.keys.CONTROL) - .waitForSelectedText('1.html') + .waitForSelectedText('2.html') }) it('non-prefixed active suggestion loads the suggestion when enter is pressed', function * () { diff --git a/test/navbar-components/urlBarTest.js b/test/navbar-components/urlBarTest.js index 2ed7ccfdfa7..eb0cdde65de 100644 --- a/test/navbar-components/urlBarTest.js +++ b/test/navbar-components/urlBarTest.js @@ -293,7 +293,7 @@ describe('urlBar tests', function () { it('does not show suggestions', function * () { yield this.app.client .keys('brave') - .waitForVisible(urlBarSuggestions, 1) + .waitForVisible(urlBarSuggestions) .ipcSend('shortcut-focus-url') .waitForElementFocus(urlInput) .waitForElementCount(urlBarSuggestions, 0) @@ -317,12 +317,12 @@ describe('urlBar tests', function () { .waitForVisible(urlBarSuggestions) // highlight for autocomplete brianbondy.com .moveToObject(urlBarSuggestions, 0, 100) - yield selectsText(this.app.client, 'rianbondy.com') - .keys('ra') + yield selectsText(this.app.client, 'rave.com/test3') + .keys('rian') .execute(function (urlBarSuggestions) { document.querySelector(urlBarSuggestions).scrollTop = 200 }, urlBarSuggestions) - yield selectsText(this.app.client, 've.com') + yield selectsText(this.app.client, 'bondy.com/test4') }) }) diff --git a/test/unit/app/common/lib/siteSuggestionsTest.js b/test/unit/app/common/lib/siteSuggestionsTest.js index c03d4223a54..6f7bd71ebd6 100644 --- a/test/unit/app/common/lib/siteSuggestionsTest.js +++ b/test/unit/app/common/lib/siteSuggestionsTest.js @@ -50,11 +50,14 @@ describe('siteSuggestions lib', function () { it('lowercases tokens', function () { assert.deepEqual(tokenizeInput('BRaD HaTES PRIMES'), ['brad', 'hates', 'primes']) }) - it('does not include http', function () { - assert.deepEqual(tokenizeInput('http://bradrichter.co/I/hate/primes.html'), ['bradrichter', 'co', 'i', 'hate', 'primes', 'html']) + it('includes protocol', function () { + assert.deepEqual(tokenizeInput('https://bradrichter.co/I/hate/primes.html'), ['bradrichter', 'co', 'i', 'hate', 'primes', 'html', 'https:']) }) - it('does not include https as a token', function () { - assert.deepEqual(tokenizeInput('https://bradrichter.co/I/hate/primes.html'), ['bradrichter', 'co', 'i', 'hate', 'primes', 'html']) + it('includes query', function () { + assert.deepEqual(tokenizeInput('https://bradrichter.co/I/hate/primes.html?test=abc&test2=abcd'), ['bradrichter', 'co', 'i', 'hate', 'primes', 'html', 'test', 'abc', 'test2', 'abcd', 'https:']) + }) + it('does not include hash', function () { + assert.deepEqual(tokenizeInput('https://bradrichter.co/I/hate/primes.html?test=abc#testing'), ['testing', 'bradrichter', 'co', 'i', 'hate', 'primes', 'html', 'test', 'abc', 'https:']) }) it('spaces get tokenized', function () { assert.deepEqual(tokenizeInput('brad\thates primes'), ['brad', 'hates', 'primes']) @@ -68,14 +71,11 @@ describe('siteSuggestions lib', function () { it('\\ gets tokenized', function () { assert.deepEqual(tokenizeInput('brad\\hates\\primes'), ['brad', 'hates', 'primes']) }) - it('? gets tokenized', function () { - assert.deepEqual(tokenizeInput('brad?hates?primes'), ['brad', 'hates', 'primes']) - }) - it('& gets tokenized', function () { - assert.deepEqual(tokenizeInput('brad&hates&primes'), ['brad', 'hates', 'primes']) - }) it('can tokenize site objects', function () { - assert.deepEqual(tokenizeInput(site1), ['do', 'not', 'use', '3', 'for', 'items', 'because', 'it', 'is', 'prime', 'www', 'bradrichter', 'co', 'bad_numbers', '3']) + assert.deepEqual(tokenizeInput(site1), ['do', 'not', 'use', '3', 'for', 'items', 'because', 'it', 'is', 'prime', 'www', 'bradrichter', 'co', 'bad_numbers', '3', 'https:']) + }) + it('non URLs get tokenized', function () { + assert.deepEqual(tokenizeInput('hello world Greatest...Boss...Ever'), ['hello', 'world', 'greatest', 'boss', 'ever']) }) }) @@ -127,7 +127,7 @@ describe('siteSuggestions lib', function () { checkResult('brave brad', [], cb) }) }) - describe('query sorts results', function () { + describe('query sorts results by location', function () { before(function (cb) { const sites = Immutable.fromJS([{ location: 'https://brave.com/twi' @@ -173,6 +173,63 @@ describe('siteSuggestions lib', function () { }) }) }) + describe('query sorts results by count', function () { + before(function (cb) { + this.page2 = { + location: 'https://brave.com/page2', + count: 20 + } + const sites = Immutable.fromJS([{ + location: 'https://brave.com/page1', + count: 5 + }, this.page2, { + location: 'https://brave.com/page3', + count: 2 + }]) + init(sites).then(cb.bind(null, null)) + }) + it('highest count first', function (cb) { + query('https://brave.com/page').then((results) => { + assert.deepEqual(results[0], this.page2) + cb() + }) + }) + }) + describe('query respects lastAccessTime', function () { + before(function (cb) { + this.site = { + location: 'https://bravebrowser.com/page2', + lastAccessedTime: 1494958046427, // most recent + count: 1 + } + const sites = Immutable.fromJS([{ + location: 'https://bravez.com/page1', + lastAccessedTime: 1, + count: 1 + }, { + location: 'https://bravebrowser.com/page1', + lastAccessedTime: 1494957046426, + count: 1 + }, this.site, { + location: 'https://bravebrowser.com/page3', + lastAccessedTime: 1494957046437, + count: 1 + }]) + init(sites).then(cb.bind(null, null)) + }) + it('items with lastAccessTime of 1 get ignored (signifies preloaded default)', function (cb) { + query('https://bravez.com/page').then((results) => { + assert.equal(results.length, 0) + cb() + }) + }) + it('most recently accessed get sorted first', function (cb) { + query('bravebrowser').then((results) => { + assert.deepEqual(results[0], this.site) + cb() + }) + }) + }) describe('add sites after init', function () { before(function (cb) { const sites = [site1, site2, site3, site4] diff --git a/test/unit/app/common/lib/suggestionTest.js b/test/unit/app/common/lib/suggestionTest.js index 4e8c4f11c73..8946b67908b 100644 --- a/test/unit/app/common/lib/suggestionTest.js +++ b/test/unit/app/common/lib/suggestionTest.js @@ -68,12 +68,12 @@ describe('suggestion unit tests', function () { }) }) - describe('simpleDomainNameValue', function () { + describe('isSimpleDomainNameValue', function () { it('sorts simple sites higher than complex sites', function () { const siteSimple = Immutable.Map({ location: 'http://www.site.com' }) const siteComplex = Immutable.Map({ location: 'http://www.site.com/?foo=bar#a' }) - assert.ok(suggestion.simpleDomainNameValue(siteSimple) === 1, 'simple site returns 1') - assert.ok(suggestion.simpleDomainNameValue(siteComplex) === 0, 'complex site returns 0') + assert.ok(suggestion.isSimpleDomainNameValue(siteSimple) === true, 'simple site returns 1') + assert.ok(suggestion.isSimpleDomainNameValue(siteComplex) === false, 'complex site returns 0') }) }) From 1dab48d56cdd1b29f9a42491a0749ceb0a79e35b Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Wed, 17 May 2017 08:59:59 -0400 Subject: [PATCH 18/22] Fix some failing tests post suggestion changes - search suggestions failing test - tab tests tab transfer can detach into new windows --- .../reducers/urlBarSuggestionsReducer.js | 6 +++++- app/common/lib/fetchSearchSuggestions.js | 12 ++++++++---- app/common/lib/suggestion.js | 4 ++-- app/renderer/components/navigation/urlBar.js | 12 +++++++++--- js/actions/appActions.js | 5 +++-- js/dispatcher/appDispatcher.js | 2 +- js/stores/windowStore.js | 3 ++- test/navbar-components/urlBarSuggestionsTest.js | 17 ++++++++++++++++- 8 files changed, 46 insertions(+), 15 deletions(-) diff --git a/app/browser/reducers/urlBarSuggestionsReducer.js b/app/browser/reducers/urlBarSuggestionsReducer.js index 3ea89981344..9c9403b72ce 100644 --- a/app/browser/reducers/urlBarSuggestionsReducer.js +++ b/app/browser/reducers/urlBarSuggestionsReducer.js @@ -9,6 +9,7 @@ const {generateNewSuggestionsList, generateNewSearchXHRResults} = require('../.. const {init, add} = require('../../common/lib/siteSuggestions') const Immutable = require('immutable') const {makeImmutable} = require('../../common/state/immutableUtil') +const tabState = require('../../common/state/tabState') const urlBarSuggestionsReducer = (state, action) => { switch (action.actionType) { @@ -30,7 +31,10 @@ const urlBarSuggestionsReducer = (state, action) => { break case appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE: state = state.set('searchResults', makeImmutable(action.searchResults)) - generateNewSuggestionsList(state, action.windowId, action.tabId) + if (action.query) { + const windowId = tabState.windowId(state, action.tabId) + generateNewSuggestionsList(state, windowId, action.tabId, action.query) + } break } return state diff --git a/app/common/lib/fetchSearchSuggestions.js b/app/common/lib/fetchSearchSuggestions.js index 77b7d6e38c6..f1b4fc9477f 100644 --- a/app/common/lib/fetchSearchSuggestions.js +++ b/app/common/lib/fetchSearchSuggestions.js @@ -4,8 +4,9 @@ const appActions = require('../../../js/actions/appActions') const {request} = require('../../../js/lib/request') +const debounce = require('../../../js/lib/debounce') -const fetchSearchSuggestions = (windowId, tabId, autocompleteURL, searchTerms) => { +const fetchSearchSuggestions = debounce((windowId, tabId, autocompleteURL, searchTerms) => { autocompleteURL.replace('{searchTerms}', encodeURIComponent(searchTerms)) request(autocompleteURL.replace('{searchTerms}', encodeURIComponent(searchTerms)), (err, response, body) => { if (err) { @@ -13,16 +14,19 @@ const fetchSearchSuggestions = (windowId, tabId, autocompleteURL, searchTerms) = } let searchResults + let query try { - searchResults = JSON.parse(body)[1] + const parsed = JSON.parse(body) + query = parsed[0] + searchResults = parsed[1] } catch (e) { console.warn(e) return } // Once we have the online suggestions, append them to the others - appActions.searchSuggestionResultsAvailable(tabId, searchResults) + appActions.searchSuggestionResultsAvailable(tabId, query, searchResults) }) -} +}, 10) module.exports = fetchSearchSuggestions diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js index 92a6e68a39c..925946c1ef5 100644 --- a/app/common/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -434,9 +434,9 @@ const generateNewSearchXHRResults = debounce((state, windowId, tabId, input) => fetchSearchSuggestions(windowId, tabId, autocompleteURL, input) } else { const appActions = require('../../../js/actions/appActions') - appActions.searchSuggestionResultsAvailable(tabId, Immutable.List()) + appActions.searchSuggestionResultsAvailable(tabId, undefined, Immutable.List()) } -}, 100) +}, 10) module.exports = { sortingPriority, diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 59ae51a097e..d86b4c717ef 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -67,13 +67,19 @@ class UrlBar extends React.Component { }, 10) } + maybeUrlBarTextChanged (value) { + if (value !== this.props.locationValue) { + appActions.urlBarTextChanged(getCurrentWindowId(), this.props.activeTabId, value) + } + } + // restores the url bar to the current location restore () { const location = UrlUtil.getDisplayLocation(this.props.location, getSetting(settings.PDFJS_ENABLED)) if (this.urlInput) { this.setValue(location) } - appActions.urlBarTextChanged(getCurrentWindowId(), this.props.activeTabId, location) + this.maybeUrlBarTextChanged(location) } /** @@ -300,7 +306,7 @@ class UrlBar extends React.Component { if (!this.keyPress) { // if this is a key press don't sent the update until keyUp so // showAutocompleteResult can handle the result - appActions.urlBarTextChanged(getCurrentWindowId(), this.props.activeTabId, val) + this.maybeUrlBarTextChanged(val) } } } @@ -316,7 +322,7 @@ class UrlBar extends React.Component { windowActions.setUrlBarSelected(false) } this.keyPressed = false - appActions.urlBarTextChanged(getCurrentWindowId(), this.props.activeTabId, this.lastVal) + this.maybeUrlBarTextChanged(this.lastVal) } select () { diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 5dfcbec4ba8..361a60ab69c 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -1365,11 +1365,12 @@ const appActions = { * @param {number} tabId - the tab id for the action * @param searchResults The search results for the currently entered URL bar text. */ - searchSuggestionResultsAvailable: function (tabId, searchResults) { + searchSuggestionResultsAvailable: function (tabId, query, searchResults) { dispatch({ actionType: appConstants.APP_SEARCH_SUGGESTION_RESULTS_AVAILABLE, tabId, - searchResults + searchResults, + query }) }, diff --git a/js/dispatcher/appDispatcher.js b/js/dispatcher/appDispatcher.js index dcc05affb92..6b6b662866f 100644 --- a/js/dispatcher/appDispatcher.js +++ b/js/dispatcher/appDispatcher.js @@ -180,7 +180,7 @@ if (process.type === 'browser') { let queryInfo = payload.queryInfo || payload.frameProps || (payload.queryInfo = {}) queryInfo = queryInfo.toJS ? queryInfo.toJS() : queryInfo let sender = event.sender - if (!queryInfo.alreadyHandledByRenderer && !sender.isDestroyed()) { + if (!sender.isDestroyed()) { const hostWebContents = sender.hostWebContents sender = hostWebContents || sender const win = require('electron').BrowserWindow.fromWebContents(sender) diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js index feb8dde0578..dd4f59c6062 100644 --- a/js/stores/windowStore.js +++ b/js/stores/windowStore.js @@ -32,6 +32,7 @@ let windowState = Immutable.fromJS({ closedFrames: [], ui: { tabs: { + tabPageIndex: 0 }, mouseInTitlebar: false, menubar: { @@ -796,7 +797,7 @@ const dispatchEventPayload = (e, payload) => { queryInfo.windowId = getCurrentWindowId() } // handle any ipc dispatches that are targeted to this window - if (queryInfo.windowId && queryInfo.windowId === getCurrentWindowId()) { + if (queryInfo.windowId && queryInfo.windowId === getCurrentWindowId() && !queryInfo.alreadyHandledByRenderer) { doAction(payload) } } diff --git a/test/navbar-components/urlBarSuggestionsTest.js b/test/navbar-components/urlBarSuggestionsTest.js index c1ce2cfb8c6..5ef5b6646c2 100644 --- a/test/navbar-components/urlBarSuggestionsTest.js +++ b/test/navbar-components/urlBarSuggestionsTest.js @@ -186,7 +186,22 @@ describe('search suggestions', function () { it('Finds search suggestions and performs a search when selected', function * () { yield this.app.client .changeSetting(settings.OFFER_SEARCH_SUGGESTIONS, true) - .setInputText(urlInput, 'what is') + + // Until a refactor happens with search suggestions, + // they are a bit fragile if you aren't actually typing. + // So this for loop avoids an intermittent failure. + // I also couldn't use .typeText() because the autocomplete makes + // that hang when it checks for the value that was typed. + // The refactor needed is to allow urlbar suggestions to be built + // in parts and then rendered together, so that different suggestion + // types would be combined and rendered together as they are available. + const input = 'what is' + for (let i = 0; i < input.length; i++) { + yield this.app.client + .keys(input[i]) + .pause(50) + } + yield this.app.client .waitForVisible(urlBarSuggestions) .keys(Brave.keys.DOWN) .waitForExist(urlBarSuggestions + ' li.suggestionItem[data-index="0"]:not(.selected)') From b37785e1d70cdd977a6a18ad2d3155e764647efe Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Wed, 17 May 2017 22:39:17 -0400 Subject: [PATCH 19/22] Improve sorting of results This makes results appear much better now and fixes the various use cases you found --- app/common/lib/siteSuggestions.js | 7 +- app/common/lib/suggestion.js | 146 +++++++++--- app/common/state/navigationBarState.js | 5 + app/renderer/components/navigation/urlBar.js | 3 +- .../navigation/urlBarSuggestions.js | 2 +- app/renderer/reducers/urlBarReducer.js | 11 +- docs/state.md | 10 +- .../app/common/lib/siteSuggestionsTest.js | 211 ++++++++++-------- test/unit/app/common/lib/suggestionTest.js | 90 ++++++++ 9 files changed, 355 insertions(+), 130 deletions(-) diff --git a/app/common/lib/siteSuggestions.js b/app/common/lib/siteSuggestions.js index 4c56b7bb52b..3bbdbb44f23 100644 --- a/app/common/lib/siteSuggestions.js +++ b/app/common/lib/siteSuggestions.js @@ -48,7 +48,8 @@ const tokenizeInput = (data) => { let url = data || '' let parts = [] - if (typeof data === 'object' && data !== null) { + const isSiteObject = typeof data === 'object' && data !== null + if (isSiteObject) { // When lastAccessTime is 1 it is a default built-in entry which we don't want // to appear in suggestions. if (data.lastAccessedTime === 1) { @@ -69,6 +70,10 @@ const tokenizeInput = (data) => { if (url && isUrl(url)) { const parsedUrl = urlParse(url.toLowerCase()) + // Cache parsed value for latter use when sorting + if (isSiteObject) { + data.parsedUrl = parsedUrl + } if (parsedUrl.hash) { parts.push(parsedUrl.hash.slice(1)) } diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js index 925946c1ef5..1fa9521fc94 100644 --- a/app/common/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -105,9 +105,10 @@ const sortByAccessCountWithAgeDecay = (s1, s2) => { * @param {object} An already normalized simple URL * */ -const isSimpleDomainNameValue = (site) => { - const parsed = urlParse(getURL(site)) - if (parsed.hash === null && parsed.search === null && parsed.query === null && parsed.pathname === '/') { +const isSimpleDomainNameValue = (site) => isParsedUrlSimpleDomainNameValue(urlParse(getURL(site))) +const isParsedUrlSimpleDomainNameValue = (parsed) => { + if ((parsed.hash === null || parsed.hash === '#') && + parsed.search === null && parsed.query === null && parsed.pathname === '/') { return true } else { return false @@ -219,36 +220,128 @@ const createVirtualHistoryItems = (historySites) => { }))) } -// Same as sortByAccessCountWithAgeDecay but if one is a prefix of the -// other then it is considered always sorted first. -const getSortForSuggestions = (userInputLower) => (s1, s2) => { - const {sortByAccessCountWithAgeDecay, normalizeLocation} = require('./suggestion') - const url1 = normalizeLocation(getURL(s1)) - const url2 = normalizeLocation(getURL(s2)) - const pos1 = url1.indexOf(userInputLower) - const pos2 = url2.indexOf(userInputLower) - if (pos1 !== -1 && pos2 !== -1) { - if (pos1 !== pos2) { - return pos1 - pos2 +/** + * Returns a function that sorts 2 sites by their host. + * The result of that function is a postive, negative, or 0 result. + * 3 or -3 for a strong indicator of a superior result. + * 2 or -2 for a good indicator of a superior result. + * 1 or -1 for a weak indicator of a superior result. + * 0 if no determination can be made. + */ +const getSortByDomain = (userInputLower, userInputHost) => { + return (s1, s2) => { + // Check for matches on hostname which if found overrides + // any count or frequency calculation. + // Note that for parsed URLs that are not complete, the pathname contains + // what the user is entering as the host and the host is null. + const host1 = s1.parsedUrl.host || s1.parsedUrl.pathname + const host2 = s2.parsedUrl.host || s2.parsedUrl.pathname + + let pos1 = host1.indexOf(userInputHost) + let pos2 = host2.indexOf(userInputHost) + if (pos1 !== -1 && pos2 === -1) { + return -3 } - const url1IsSimple = isSimpleDomainNameValue(s1) - const url2IsSimple = isSimpleDomainNameValue(s2) - // If either both false or both true then rely on count w/ decay sort - if (url1IsSimple === url2IsSimple) { - return sortByAccessCountWithAgeDecay(s1, s2) + if (pos1 === -1 && pos2 !== -1) { + return 3 } - if (url1IsSimple) { - return -1 + if (pos1 !== -1 && pos2 !== -1) { + // Try to match on the first position without taking into account decay sort. + // This is because autocomplete is based on matching prefixes. + if (pos1 === 0 && pos2 !== 0) { + return -2 + } + if (pos1 !== 0 && pos2 === 0) { + return 2 + } + + // Try the same to see if taking off www. helps. + if (!userInputLower.startsWith('www.')) { + pos1 = host1.indexOf('www.' + userInputLower) + pos2 = host2.indexOf('www.' + userInputLower) + if (pos1 === 0 && pos2 !== 0) { + return -1 + } + if (pos1 !== 0 && pos2 === 0) { + return 1 + } + } + + const sortBySimpleURLResult = sortBySimpleURL(s1, s2) + if (sortBySimpleURLResult !== 0) { + return sortBySimpleURLResult + } } - return 1 + // Can't determine what is the best match + return 0 } - if (pos1 !== -1 && pos2 === -1) { +} + +/** + * Sorts 2 URLS by if they are a simple URL or not. + * Returns the normal -1, 1, or 0 result for sort functions. + */ +const sortBySimpleURL = (s1, s2) => { + // If one of the URLs is a simpleURL and the other isn't then sort the simple one first + const url1IsSimple = isParsedUrlSimpleDomainNameValue(s1.parsedUrl) + const url2IsSimple = isParsedUrlSimpleDomainNameValue(s2.parsedUrl) + if (url1IsSimple && !url2IsSimple) { return -1 } - if (pos1 === -1 && pos2 !== -1) { + if (!url1IsSimple && url2IsSimple) { return 1 } - return sortByAccessCountWithAgeDecay(s1, s2) + return 0 +} + +/** + * Returns a function that sorts 2 sites by their host. + * The result of that function is a postive, negative, or 0 result. + */ +const getSortByPath = (userInputLower) => { + return (path1, path2) => { + const pos1 = path1.indexOf(userInputLower) + const pos2 = path2.indexOf(userInputLower) + if (pos1 !== -1 && pos2 === -1) { + return -1 + } + if (pos1 === -1 && pos2 !== -1) { + return 1 + } + // Can't determine what is the best match + return 0 + } +} + +// Same as sortByAccessCountWithAgeDecay but if one is a prefix of the +// other then it is considered always sorted first. +const getSortForSuggestions = (userInputLower) => { + userInputLower = userInputLower.replace(/^http:\/\//, '') + userInputLower = userInputLower.replace(/^https:\/\//, '') + const userInputParts = userInputLower.split('/') + const userInputHost = userInputParts[0] + const sortByDomain = getSortByDomain(userInputLower, userInputHost) + const sortByPath = getSortByPath(userInputLower) + const {sortByAccessCountWithAgeDecay} = require('./suggestion') + + return (s1, s2) => { + s1.parsedUrl = s1.parsedUrl || urlParse(getURL(s1) || '') + s2.parsedUrl = s2.parsedUrl || urlParse(getURL(s2) || '') + + const sortByDomainResult = sortByDomain(s1, s2) + if (sortByDomainResult !== 0) { + return sortByDomainResult + } + + const path1 = s1.parsedUrl.path + (s1.parsedUrl.hash || '') + const path2 = s2.parsedUrl.path + (s2.parsedUrl.hash || '') + const sortByPathResult = sortByPath(path1, path2) + if (sortByPathResult !== 0) { + return sortByPathResult + } + + return sortByAccessCountWithAgeDecay(s1, s2) + } } // Currently we sort only sites that are not immutableJS and @@ -442,6 +535,9 @@ module.exports = { sortingPriority, sortByAccessCountWithAgeDecay, getSortForSuggestions, + getSortByPath, + sortBySimpleURL, + getSortByDomain, isSimpleDomainNameValue, normalizeLocation, shouldNormalizeLocation, diff --git a/app/common/state/navigationBarState.js b/app/common/state/navigationBarState.js index 248500c7a7a..bfe5c63d1e4 100644 --- a/app/common/state/navigationBarState.js +++ b/app/common/state/navigationBarState.js @@ -81,6 +81,11 @@ const api = { return api.getUrlBar(state, tabId).getIn(['suggestions', 'urlSuffix']) || '' }, + hasSuggestionMatch: (state, tabId) => { + state = validateState(state) + return api.getUrlBar(state, tabId).getIn(['suggestions', 'hasSuggestionMatch']) || false + }, + hasLocationValueSuffix: (state, tabId) => { state = validateState(state) return api.locationValueSuffix(state, tabId).length > 0 diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index d86b4c717ef..826d9fc1005 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -498,6 +498,7 @@ class UrlBar extends React.Component { props.scriptsBlocked = activeFrame.getIn(['noScript', 'blocked']) props.isSecure = activeFrame.getIn(['security', 'isSecure']) props.hasLocationValueSuffix = urlbar.getIn(['suggestions', 'urlSuffix']) + props.hasSuggestionMatch = urlbar.getIn(['suggestions', 'hasSuggestionMatch']) props.startLoadTime = activeFrame.get('startLoadTime') props.endLoadTime = activeFrame.get('endLoadTime') props.loading = activeFrame.get('loading') @@ -604,7 +605,7 @@ class UrlBar extends React.Component { ? : null } diff --git a/app/renderer/components/navigation/urlBarSuggestions.js b/app/renderer/components/navigation/urlBarSuggestions.js index af907ebf85c..0f7e9948ca0 100644 --- a/app/renderer/components/navigation/urlBarSuggestions.js +++ b/app/renderer/components/navigation/urlBarSuggestions.js @@ -65,7 +65,7 @@ class UrlBarSuggestions extends ImmutableComponent { } items = items.concat(suggestions.map((suggestion, i) => { const currentIndex = index + i - const selected = this.activeIndex === currentIndex || (!this.activeIndex && currentIndex === 0 && this.props.hasLocationValueSuffix) + const selected = this.activeIndex === currentIndex || (!this.activeIndex && currentIndex === 0 && this.props.hasSuggestionMatch) return { } const suggestion = suggestionList && suggestionList.get(selectedIndex) let suffix = '' + let hasSuggestionMatch = false if (suggestion) { const autocompleteEnabled = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'autocompleteEnabled'])) @@ -81,11 +82,13 @@ const updateUrlSuffix = (state, suggestionList) => { const beforePrefix = suggestion.get('location').substring(0, index) if (beforePrefix.endsWith('://') || beforePrefix.endsWith('://www.') || index === 0) { suffix = suggestion.get('location').substring(index + location.length) + hasSuggestionMatch = true } } } } state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'urlSuffix']), suffix) + state = state.setIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'hasSuggestionMatch']), hasSuggestionMatch) return state } @@ -238,8 +241,8 @@ const urlBarReducer = (state, action) => { const selectedIndexPath = activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']) const suggestionList = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'suggestionList'])) const selectedIndex = state.getIn(selectedIndexPath) - const lastSuffix = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'urlSuffix'])) - if (!selectedIndex && selectedIndex !== 0 && !lastSuffix) { + const hasSuggestionMatch = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'hasSuggestionMatch'])) + if (!selectedIndex && selectedIndex !== 0 && !hasSuggestionMatch) { state = state.setIn(selectedIndexPath, 0) } else if (selectedIndex > 0) { state = state.setIn(selectedIndexPath, selectedIndex - 1) @@ -253,8 +256,8 @@ const urlBarReducer = (state, action) => { const selectedIndexPath = activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'selectedIndex']) const suggestionList = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'suggestionList'])) const selectedIndex = state.getIn(selectedIndexPath) - const lastSuffix = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'urlSuffix'])) - if (!selectedIndex && selectedIndex !== 0 && !lastSuffix) { + const hasSuggestionMatch = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'hasSuggestionMatch'])) + if (!selectedIndex && selectedIndex !== 0 && !hasSuggestionMatch) { state = state.setIn(selectedIndexPath, 0) } else if (selectedIndex < suggestionList.size - 1) { state = state.setIn(selectedIndexPath, selectedIndex + 1) diff --git a/docs/state.md b/docs/state.md index 63f901a000a..9c4307bab9a 100644 --- a/docs/state.md +++ b/docs/state.md @@ -357,7 +357,11 @@ AppStore width: number, // session properties windowId: number // the muon id for the window - }] + }], + searchDetail: { + autocompleteURL: string, // ditto re: {searchTerms} + searchURL: string // with replacement var in string: {searchTerms} + }, } ``` @@ -623,10 +627,6 @@ WindowStore minPublisherVisits: number // e.g., 0 } }, - searchDetail: { - autocompleteURL: string, // ditto re: {searchTerms} - searchURL: string // with replacement var in string: {searchTerms} - }, searchResults: array, // autocomplete server results if enabled ui: { bookmarksToolbar: { diff --git a/test/unit/app/common/lib/siteSuggestionsTest.js b/test/unit/app/common/lib/siteSuggestionsTest.js index 6f7bd71ebd6..96e5fc48586 100644 --- a/test/unit/app/common/lib/siteSuggestionsTest.js +++ b/test/unit/app/common/lib/siteSuggestionsTest.js @@ -23,6 +23,27 @@ const site4 = { title: 'Brad Saves The World!' } +// Compares 2 sites via deepEqual while first clearing out cached data +const siteEqual = (actual, expected) => { + assert.equal(actual.constructor, expected.constructor) + if (expected.constructor === Array) { + assert.equal(actual.length, expected.length) + for (let i = 0; i < actual.length; i++) { + const a = Object.assign({}, actual[i]) + delete a.parsedUrl + const e = Object.assign({}, expected[i]) + delete e.parsedUrl + assert.deepEqual(a, e) + } + } else { + const a = Object.assign({}, actual) + delete a.parsedUrl + const e = Object.assign({}, expected) + delete e.parsedUrl + assert.deepEqual(a, e) + } +} + require('../../../braveUnit') describe('siteSuggestions lib', function () { @@ -81,7 +102,7 @@ describe('siteSuggestions lib', function () { const checkResult = (inputQuery, expectedResults, cb) => { query(inputQuery).then((results) => { - assert.deepEqual(results, expectedResults) + siteEqual(results, expectedResults) cb() }) } @@ -127,109 +148,113 @@ describe('siteSuggestions lib', function () { checkResult('brave brad', [], cb) }) }) - describe('query sorts results by location', function () { - before(function (cb) { - const sites = Immutable.fromJS([{ - location: 'https://brave.com/twi' - }, { - location: 'https://twitter.com/brave' - }, { - location: 'https://twitter.com/brianbondy' - }, { - location: 'https://twitter.com/_brianclif' - }, { - location: 'https://twitter.com/cezaraugusto' - }, { - location: 'https://bbondy.com/twitter' - }, { - location: 'https://twitter.com' - }, { - location: 'https://twitter.com/i/moments' - }]) - init(sites).then(cb.bind(null, null)) - }) - it('orders shortest match first', function (cb) { - query('twitter.com').then((results) => { - assert.deepEqual(results[0], { location: 'https://twitter.com' }) - cb() + + describe('query', function () { + describe('sorts results by location', function () { + before(function (cb) { + const sites = Immutable.fromJS([{ + location: 'https://brave.com/twi' + }, { + location: 'https://twitter.com/brave' + }, { + location: 'https://twitter.com/brianbondy' + }, { + location: 'https://twitter.com/_brianclif' + }, { + location: 'https://twitter.com/cezaraugusto' + }, { + location: 'https://bbondy.com/twitter' + }, { + location: 'https://twitter.com' + }, { + location: 'https://twitter.com/i/moments' + }]) + init(sites).then(cb.bind(null, null)) }) - }) - it('matches prefixes first', function (cb) { - query('twi').then((results) => { - assert.deepEqual(results[0], { location: 'https://twitter.com' }) - cb() + it('orders shortest match first', function (cb) { + query('twitter.com').then((results) => { + siteEqual(results[0], { location: 'https://twitter.com' }) + cb() + }) }) - }) - it('closest to the left match wins', function (cb) { - query('twitter.com brian').then((results) => { - assert.deepEqual(results[0], { location: 'https://twitter.com/brianbondy' }) - cb() + it('matches prefixes first', function (cb) { + query('twi').then((results) => { + siteEqual(results[0], { location: 'https://twitter.com' }) + cb() + }) }) - }) - it('matches based on tokens and not exactly', function (cb) { - query('twitter.com/moments').then((results) => { - assert.deepEqual(results[0], { location: 'https://twitter.com/i/moments' }) - cb() + it('closest to the left match wins', function (cb) { + query('twitter.com brian').then((results) => { + siteEqual(results[0], { location: 'https://twitter.com/brianbondy' }) + cb() + }) }) - }) - }) - describe('query sorts results by count', function () { - before(function (cb) { - this.page2 = { - location: 'https://brave.com/page2', - count: 20 - } - const sites = Immutable.fromJS([{ - location: 'https://brave.com/page1', - count: 5 - }, this.page2, { - location: 'https://brave.com/page3', - count: 2 - }]) - init(sites).then(cb.bind(null, null)) - }) - it('highest count first', function (cb) { - query('https://brave.com/page').then((results) => { - assert.deepEqual(results[0], this.page2) - cb() + it('matches based on tokens and not exactly', function (cb) { + query('twitter.com/moments').then((results) => { + siteEqual(results[0], { location: 'https://twitter.com/i/moments' }) + cb() + }) }) }) - }) - describe('query respects lastAccessTime', function () { - before(function (cb) { - this.site = { - location: 'https://bravebrowser.com/page2', - lastAccessedTime: 1494958046427, // most recent - count: 1 - } - const sites = Immutable.fromJS([{ - location: 'https://bravez.com/page1', - lastAccessedTime: 1, - count: 1 - }, { - location: 'https://bravebrowser.com/page1', - lastAccessedTime: 1494957046426, - count: 1 - }, this.site, { - location: 'https://bravebrowser.com/page3', - lastAccessedTime: 1494957046437, - count: 1 - }]) - init(sites).then(cb.bind(null, null)) - }) - it('items with lastAccessTime of 1 get ignored (signifies preloaded default)', function (cb) { - query('https://bravez.com/page').then((results) => { - assert.equal(results.length, 0) - cb() + describe('sorts results by count', function () { + before(function (cb) { + this.page2 = { + location: 'https://brave.com/page2', + count: 20 + } + const sites = Immutable.fromJS([{ + location: 'https://brave.com/page1', + count: 5 + }, this.page2, { + location: 'https://brave.com/page3', + count: 2 + }]) + init(sites).then(cb.bind(null, null)) + }) + it('highest count first', function (cb) { + query('https://brave.com/page').then((results) => { + siteEqual(results[0], this.page2) + cb() + }) }) }) - it('most recently accessed get sorted first', function (cb) { - query('bravebrowser').then((results) => { - assert.deepEqual(results[0], this.site) - cb() + describe('respects lastAccessTime', function () { + before(function (cb) { + this.site = { + location: 'https://bravebrowser.com/page2', + lastAccessedTime: 1494958046427, // most recent + count: 1 + } + const sites = Immutable.fromJS([{ + location: 'https://bravez.com/page1', + lastAccessedTime: 1, + count: 1 + }, { + location: 'https://bravebrowser.com/page1', + lastAccessedTime: 1494957046426, + count: 1 + }, this.site, { + location: 'https://bravebrowser.com/page3', + lastAccessedTime: 1494957046437, + count: 1 + }]) + init(sites).then(cb.bind(null, null)) + }) + it('items with lastAccessTime of 1 get ignored (signifies preloaded default)', function (cb) { + query('https://bravez.com/page').then((results) => { + assert.equal(results.length, 0) + cb() + }) + }) + it('most recently accessed get sorted first', function (cb) { + query('bravebrowser').then((results) => { + siteEqual(results[0], this.site) + cb() + }) }) }) }) + describe('add sites after init', function () { before(function (cb) { const sites = [site1, site2, site3, site4] diff --git a/test/unit/app/common/lib/suggestionTest.js b/test/unit/app/common/lib/suggestionTest.js index 8946b67908b..f0cdde43010 100644 --- a/test/unit/app/common/lib/suggestionTest.js +++ b/test/unit/app/common/lib/suggestionTest.js @@ -7,6 +7,7 @@ const mockery = require('mockery') const assert = require('assert') const sinon = require('sinon') const Immutable = require('immutable') +const urlParse = require('url').parse const {makeImmutable} = require('../../../../../app/common/state/immutableUtil') const fakeElectron = require('../../../lib/fakeElectron') const _ = require('underscore') @@ -159,4 +160,93 @@ describe('suggestion unit tests', function () { assert.ok(virtual[keys[0]].lastAccessedTime > 0) }) }) + + describe('sorting functions', function () { + describe('getSortByPath', function () { + before(function () { + this.sort = suggestion.getSortByPath('twitter') + }) + it('returns 0 when both paths contain the string', function () { + assert.equal(this.sort('https://brave.com/twitter', 'https://brianbondy.com/twitter'), 0) + }) + it('returns 0 when neihter path contain the string', function () { + assert.equal(this.sort('https://brave.com/facebook', 'https://brianbondy.com/facebook'), 0) + }) + it('returns -1 when the first site contains the string only', function () { + assert.equal(this.sort('https://brave.com/twitter', 'https://brianbondy.com/facebook'), -1) + }) + it('returns 1 when the second site contains the string only', function () { + assert.equal(this.sort('https://brave.com/facebook', 'https://brianbondy.com/twitter'), 1) + }) + it('matches even domain name for input string', function () { + assert.equal(this.sort('https://brave.com/facebook', 'https://twitter.com'), 1) + }) + }) + describe('sortBySimpleURL', function () { + before(function () { + this.sort = (url1, url2) => { + return suggestion.sortBySimpleURL( + { location: url1, parsedUrl: urlParse(url1) }, + { location: url2, parsedUrl: urlParse(url2) } + ) + } + }) + it('returns 0 when both paths are simple', function () { + assert.equal(this.sort('https://brave.com', 'https://brianbondy.com'), 0) + }) + it('returns 0 when neihter path is simple', function () { + assert.equal(this.sort('https://brave.com/facebook', 'https://brianbondy.com/facebook'), 0) + }) + it('returns -1 when the first site is simple only', function () { + assert.equal(this.sort('https://brave.com', 'https://brianbondy.com/facebook'), -1) + }) + it('returns 1 when the second site is simple only', function () { + assert.equal(this.sort('https://brave.com/facebook', 'https://brianbondy.com'), 1) + }) + it('trailing slash is considered simple', function () { + assert.equal(this.sort('https://brave.com/', 'https://twitter.com'), 0) + }) + it('trailing hash is considered simple', function () { + assert.equal(this.sort('https://brave.com/#', 'https://twitter.com'), 0) + }) + }) + describe('getSortByDomain', function () { + before(function () { + const userInputLower = 'google.c' + const userInputParts = userInputLower.split('/') + const userInputHost = userInputParts[0] + const internalSort = suggestion.getSortByDomain(userInputLower, userInputHost) + this.sort = (url1, url2) => { + return internalSort( + { location: url1, parsedUrl: urlParse(url1) }, + { location: url2, parsedUrl: urlParse(url2) } + ) + } + }) + it('negative if only first has a matching domain', function () { + assert(this.sort('https://google.com', 'https://www.brianbondy.com') < 0) + }) + it('positive if only second has a matching domain', function () { + assert(this.sort('https://www.brianbondy.com', 'https://google.com') > 0) + }) + it('0 if both have a matching domain from index 0', function () { + assert.equal(this.sort('https://google.com', 'https://google.ca'), 0) + }) + it('0 if neither has a matching domain', function () { + assert.equal(this.sort('https://brianbondy.com', 'https://clifton.io/'), 0) + }) + it('negative if first site has a match from the start of domain', function () { + assert(this.sort('https://google.com', 'https://mygoogle.com') < 0) + }) + it('positive if second site has a match but without www.', function () { + assert(this.sort('https://www.google.com', 'https://google.com') > 0) + }) + it('negative if there is a pos 0 match not including www.', function () { + assert(this.sort('https://www.google.com', 'https://mygoogle.com') < 0) + }) + it('simple domain gets matched higher', function () { + assert(this.sort('https://www.google.com', 'https://www.google.com/extra') < 0) + }) + }) + }) }) From afd891f3e7cfefd35af074db3ad147e6cace6a7e Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Thu, 18 May 2017 01:06:29 -0400 Subject: [PATCH 20/22] Tab should cycle between results Fix #5878 Auditors: @bsclifton --- app/renderer/components/navigation/urlBar.js | 10 +++++++++- test/lib/brave.js | 1 + test/navbar-components/urlBarTest.js | 15 +++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 826d9fc1005..405f8478c58 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -199,7 +199,14 @@ class UrlBar extends React.Component { this.hideAutoComplete() break case KeyCodes.TAB: - this.hideAutoComplete() + if (this.shouldRenderUrlBarSuggestions) { + if (e.shiftKey) { + windowActions.previousUrlBarSuggestionSelected() + } else { + windowActions.nextUrlBarSuggestionSelected() + } + e.preventDefault() + } break default: this.keyPressed = true @@ -315,6 +322,7 @@ class UrlBar extends React.Component { switch (e.keyCode) { case KeyCodes.UP: case KeyCodes.DOWN: + case KeyCodes.TAB: case KeyCodes.ESC: return } diff --git a/test/lib/brave.js b/test/lib/brave.js index 4ae9a9f2436..14f01991d65 100644 --- a/test/lib/brave.js +++ b/test/lib/brave.js @@ -114,6 +114,7 @@ var exports = { UP: '\ue013', PAGEDOWN: '\uE00F', END: '\uE010', + TAB: '\ue004', NULL: '\uE000' }, diff --git a/test/navbar-components/urlBarTest.js b/test/navbar-components/urlBarTest.js index eb0cdde65de..faea6c30676 100644 --- a/test/navbar-components/urlBarTest.js +++ b/test/navbar-components/urlBarTest.js @@ -265,6 +265,21 @@ describe('urlBar tests', function () { }) }) + describe('highlight suggestions with tab', function () { + it('autofills from selected suggestion', function * () { + // now type something + yield this.app.client + .keys('https://br') + .waitForInputText(urlInput, 'https://brave.com') + // hit down + .keys(Brave.keys.TAB) + .waitForInputText(urlInput, 'https://brave.com/test') + // hit up + .keys(Brave.keys.SHIFT + Brave.keys.TAB) + .waitForInputText(urlInput, 'https://brave.com') + }) + }) + describe('highlight suggestions', function () { it('autofills from selected suggestion', function * () { // now type something From ff41f60a21f9f9a3cef35d9341086d5d0afaa68d Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Thu, 18 May 2017 01:22:07 -0400 Subject: [PATCH 21/22] Don't do suggestion actions on non-char key presses Auditors: @bsclifton Fix #5878 Test comming in a follow up commit now, just pushing without so you can get a build going --- app/renderer/components/navigation/urlBar.js | 3 +++ test/navbar-components/urlBarSuggestionsTest.js | 17 +++++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index 405f8478c58..e864e8abd44 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -324,6 +324,9 @@ class UrlBar extends React.Component { case KeyCodes.DOWN: case KeyCodes.TAB: case KeyCodes.ESC: + case KeyCodes.LEFT: + case KeyCodes.SHIFT: + case KeyCodes.RIGHT: return } if (this.props.isSelected) { diff --git a/test/navbar-components/urlBarSuggestionsTest.js b/test/navbar-components/urlBarSuggestionsTest.js index 5ef5b6646c2..875a934bca8 100644 --- a/test/navbar-components/urlBarSuggestionsTest.js +++ b/test/navbar-components/urlBarSuggestionsTest.js @@ -66,6 +66,23 @@ describe('urlBarSuggestions', function () { .waitForElementCount(urlBarSuggestions, 0) }) + it('deactivated suggestions do not pop back up when left or shift is pressed', function * () { + yield this.app.client + .setInputText(urlInput, 'Page 1') + .waitForExist(urlBarSuggestions + ' li.suggestionItem[data-index="0"]') + .keys(Brave.keys.BACKSPACE) + .waitForElementCount(urlBarSuggestions, 0) + .keys(Brave.keys.LEFT) + .pause(50) + .keys(Brave.keys.SHIFT + Brave.keys.LEFT) + .pause(50) + .keys(Brave.keys.LEFT) + .pause(50) + .keys(Brave.keys.SHIFT) + .pause(50) + .waitForElementCount(urlBarSuggestions, 0) + }) + it('deactivates suggestions on delete', function * () { yield this.app.client .setInputText(urlInput, 'Page 1') From f94239acb22f893635a91b2679a5351666744fe9 Mon Sep 17 00:00:00 2001 From: "Brian R. Bondy" Date: Thu, 18 May 2017 10:49:03 -0400 Subject: [PATCH 22/22] Match entered paths better Matches things better if the user puts a / in the input --- app/common/lib/suggestion.js | 23 +++++++-- app/renderer/components/navigation/urlBar.js | 5 +- test/unit/app/common/lib/suggestionTest.js | 49 ++++++++++++++++++++ 3 files changed, 69 insertions(+), 8 deletions(-) diff --git a/app/common/lib/suggestion.js b/app/common/lib/suggestion.js index 1fa9521fc94..093273750a8 100644 --- a/app/common/lib/suggestion.js +++ b/app/common/lib/suggestion.js @@ -291,6 +291,16 @@ const sortBySimpleURL = (s1, s2) => { if (!url1IsSimple && url2IsSimple) { return 1 } + const url1IsSecure = s1.parsedUrl.protocol === 'https:' + const url2IsSecure = s2.parsedUrl.protocol === 'https:' + if (url1IsSimple && url2IsSimple) { + if (url1IsSecure && !url2IsSecure) { + return -1 + } + if (!url1IsSecure && url2IsSecure) { + return 1 + } + } return 0 } @@ -320,6 +330,7 @@ const getSortForSuggestions = (userInputLower) => { userInputLower = userInputLower.replace(/^https:\/\//, '') const userInputParts = userInputLower.split('/') const userInputHost = userInputParts[0] + const userInputValue = userInputParts[1] || '' const sortByDomain = getSortByDomain(userInputLower, userInputHost) const sortByPath = getSortByPath(userInputLower) const {sortByAccessCountWithAgeDecay} = require('./suggestion') @@ -328,13 +339,15 @@ const getSortForSuggestions = (userInputLower) => { s1.parsedUrl = s1.parsedUrl || urlParse(getURL(s1) || '') s2.parsedUrl = s2.parsedUrl || urlParse(getURL(s2) || '') - const sortByDomainResult = sortByDomain(s1, s2) - if (sortByDomainResult !== 0) { - return sortByDomainResult + if (!userInputValue) { + const sortByDomainResult = sortByDomain(s1, s2) + if (sortByDomainResult !== 0) { + return sortByDomainResult + } } - const path1 = s1.parsedUrl.path + (s1.parsedUrl.hash || '') - const path2 = s2.parsedUrl.path + (s2.parsedUrl.hash || '') + const path1 = s1.parsedUrl.host + s1.parsedUrl.path + (s1.parsedUrl.hash || '') + const path2 = s2.parsedUrl.host + s2.parsedUrl.path + (s2.parsedUrl.hash || '') const sortByPathResult = sortByPath(path1, path2) if (sortByPathResult !== 0) { return sortByPathResult diff --git a/app/renderer/components/navigation/urlBar.js b/app/renderer/components/navigation/urlBar.js index e864e8abd44..d972d8d753d 100644 --- a/app/renderer/components/navigation/urlBar.js +++ b/app/renderer/components/navigation/urlBar.js @@ -60,7 +60,7 @@ class UrlBar extends React.Component { this.onContextMenu = this.onContextMenu.bind(this) this.keyPressed = false this.showAutocompleteResult = debounce(() => { - if (this.keyPressed || !this.urlInput || this.props.locationValueSuffix.length === 0) { + if (this.keyPressed || !this.urlInput) { return } this.updateAutocomplete(this.lastVal) @@ -392,7 +392,7 @@ class UrlBar extends React.Component { } else if (this.props.location !== prevProps.location) { // This is a url nav change this.setValue(UrlUtil.getDisplayLocation(this.props.location, pdfjsEnabled)) - } else if (this.props.hasLocationValueSuffix && + } else if (this.props.hasSuggestionMatch && this.props.isActive && this.props.locationValueSuffix !== this.lastSuffix) { this.showAutocompleteResult() @@ -508,7 +508,6 @@ class UrlBar extends React.Component { props.title = activeFrame.get('title') || '' props.scriptsBlocked = activeFrame.getIn(['noScript', 'blocked']) props.isSecure = activeFrame.getIn(['security', 'isSecure']) - props.hasLocationValueSuffix = urlbar.getIn(['suggestions', 'urlSuffix']) props.hasSuggestionMatch = urlbar.getIn(['suggestions', 'hasSuggestionMatch']) props.startLoadTime = activeFrame.get('startLoadTime') props.endLoadTime = activeFrame.get('endLoadTime') diff --git a/test/unit/app/common/lib/suggestionTest.js b/test/unit/app/common/lib/suggestionTest.js index f0cdde43010..1faf67ad6cc 100644 --- a/test/unit/app/common/lib/suggestionTest.js +++ b/test/unit/app/common/lib/suggestionTest.js @@ -209,6 +209,9 @@ describe('suggestion unit tests', function () { it('trailing hash is considered simple', function () { assert.equal(this.sort('https://brave.com/#', 'https://twitter.com'), 0) }) + it('Prefers https sipmle URLs', function () { + assert(this.sort('https://brave.com', 'http://brave.com') < 0) + }) }) describe('getSortByDomain', function () { before(function () { @@ -248,5 +251,51 @@ describe('suggestion unit tests', function () { assert(this.sort('https://www.google.com', 'https://www.google.com/extra') < 0) }) }) + describe('getSortForSuggestions', function () { + describe('with url entered as path', function () { + before(function () { + const userInputLower = 'brianbondy.com/projects' + const userInputParts = userInputLower.split('/') + const userInputHost = userInputParts[0] + const internalSort = suggestion.getSortForSuggestions(userInputLower, userInputHost) + this.sort = (url1, url2) => { + return internalSort( + { location: url1, parsedUrl: urlParse(url1) }, + { location: url2, parsedUrl: urlParse(url2) } + ) + } + }) + it('returns 0 when both urls are the same', function () { + assert.equal(this.sort('https://www.google.com', 'https://www.google.com'), 0) + }) + it('matches exact path if more specific path is specified', function () { + assert(this.sort('https://brianbondy.com', 'https://www.brianbondy.com/projects/2') > 0) + }) + }) + describe('with single string entered', function () { + before(function () { + const userInputLower = 'brianbondy.c' + const userInputParts = userInputLower.split('/') + const userInputHost = userInputParts[0] + const internalSort = suggestion.getSortForSuggestions(userInputLower, userInputHost) + this.sort = (url1, url2) => { + return internalSort( + { location: url1, parsedUrl: urlParse(url1) }, + { location: url2, parsedUrl: urlParse(url2) } + ) + } + }) + it('matches on domain name first', function () { + assert(this.sort('https://www.brianbondy.com', 'https://www.google.com/brianbondy.co') < 0) + }) + it('matches with or without protocol', function () { + assert(this.sort('https://www.2brianbondy.com', 'http://www.brianbondy.com') > 0) + assert(this.sort('https://brianbondy.com', 'www.brianbondy.com') < 0) + }) + it('non-wwww. matches before www.', function () { + assert(this.sort('https://brianbondy.com', 'www.brianbondy.com') < 0) + }) + }) + }) }) })