diff --git a/js/components/urlBarSuggestions.js b/js/components/urlBarSuggestions.js index 0278aa26b75..9194c2c377b 100644 --- a/js/components/urlBarSuggestions.js +++ b/js/components/urlBarSuggestions.js @@ -21,6 +21,7 @@ const eventUtil = require('../lib/eventUtil') const cx = require('../lib/classSet') const locale = require('../l10n') const windowStore = require('../stores/windowStore') +const suggestion = require('../lib/suggestion') class UrlBarSuggestions extends ImmutableComponent { constructor (props) { @@ -149,8 +150,8 @@ class UrlBarSuggestions extends ImmutableComponent { })) index += suggestions.size } - addToItems(bookmarkSuggestions, 'bookmarksTitle', locale.translation('bookmarksSuggestionTitle'), 'fa-star-o') addToItems(historySuggestions, 'historyTitle', locale.translation('historySuggestionTitle'), 'fa-clock-o') + addToItems(bookmarkSuggestions, 'bookmarksTitle', locale.translation('bookmarksSuggestionTitle'), 'fa-star-o') addToItems(aboutPagesSuggestions, 'aboutPagesTitle', locale.translation('aboutPagesSuggestionTitle'), null) addToItems(tabSuggestions, 'tabsTitle', locale.translation('tabsSuggestionTitle'), 'fa-external-link') addToItems(searchSuggestions, 'searchTitle', locale.translation('searchSuggestionTitle'), 'fa-search') @@ -254,8 +255,9 @@ class UrlBarSuggestions extends ImmutableComponent { if (pos1 - pos2 !== 0) { return pos1 - pos2 } else { - // If there's a tie on the match location, use the shorter URL - return s1.get('location').length - s2.get('location').length + // If there's a tie on the match location, use the age + // decay modified access count + return suggestion.sortByAccessCountWithAgeDecay(s1, s2) } } } diff --git a/js/constants/appConfig.js b/js/constants/appConfig.js index add9ca97053..ad6cf10e0a3 100644 --- a/js/constants/appConfig.js +++ b/js/constants/appConfig.js @@ -85,6 +85,9 @@ module.exports = { baseUrl: `${updateHost}/1/releases`, winBaseUrl: `${winUpdateHost}/multi-channel/releases/CHANNEL/` }, + urlSuggestions: { + ageDecayConstant: 50 + }, defaultSettings: { 'adblock.customRules': '', 'general.language': null, // null means to use the OS lang diff --git a/js/lib/suggestion.js b/js/lib/suggestion.js new file mode 100644 index 00000000000..82dd52cf710 --- /dev/null +++ b/js/lib/suggestion.js @@ -0,0 +1,84 @@ +/* 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 appConfig = require('../constants/appConfig') + +const sigmoid = (t) => { + return 1 / (1 + Math.pow(Math.E, -t)) +} + +const ONE_DAY = 1000 * 60 * 60 * 24 + +/* + * Calculate the sorting priority for a history item based on number of + * accesses and time since last access + * + * @param {number} count - The number of times this site has been accessed + * @param {number} currentTime - Current epoch millisecnds + * @param {boolean} lastAccessedTime - Epoch milliseconds of last access + * + */ +module.exports.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 + const ageFactor = 1 - ((sigmoid(ageInDays / ageDecayConstant) - 0.5) * 2) + // sorting priority + // console.log(count, ageInDays, ageFactor, count * ageFactor) + return count * ageFactor +} + +/* + * Sort two history items by priority + * + * @param {ImmutableObject} s1 - first history item + * @param {ImmutableObject} s2 - second history item + * + * Return the relative order of two site entries taking into consideration + * the number of times the site has been accessed and the length of time + * since the last access. + * + * The base sort order is determined by the count attribute of the site + * entry. A modifier is then computed based on the length of time since + * the last access. A sigmoid function is used to weight more recent + * entries higher than entries in the past. This is not a linear function, + * entries in the far past with many counts will still be discounted + * heavily as the sigmoid modifier will cancel most of the count + * base parameter. + * + * Below is a sample comparison of two sites that have been accessed + * recently (but not at the identical time). Each site is accessed + * 9 times. The count is discounted by an aging factor calculated + * using the sigmoid decay function. + * + * http://www.gm.ca/gm/ + * + * ageInDays 0.17171469907407408 + * ageFactor 0.9982828546969802 + * count 9 + * priority 0.9982828546969802 + * + * http://www.gm.com/index.html + * + * ageInDays 0.17148791666666666 + * ageFactor 0.9982851225143763 + * count 9 + * priority 0.9982851225143763 + * + */ +module.exports.sortByAccessCountWithAgeDecay = (s1, s2) => { + const s1Priority = module.exports.sortingPriority( + s1.get('count') || 0, + (new Date()).getTime(), + s1.get('lastAccessedTime') || (new Date()).getTime(), + appConfig.urlSuggestions.ageDecayConstant + ) + const s2Priority = module.exports.sortingPriority( + s2.get('count') || 0, + (new Date()).getTime(), + s2.get('lastAccessedTime') || (new Date()).getTime(), + appConfig.urlSuggestions.ageDecayConstant + ) + return s2Priority - s1Priority +} diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js index 25693841d84..c7775cde1d0 100644 --- a/js/state/siteUtil.js +++ b/js/state/siteUtil.js @@ -155,6 +155,10 @@ module.exports.addSite = function (sites, siteDetail, tag, originalSiteDetail) { if (siteDetail.get('themeColor') || oldSite && oldSite.get('themeColor')) { site = site.set('themeColor', siteDetail.get('themeColor') || oldSite.get('themeColor')) } + if (site.get('tags').size === 0) { + // Increment the visit count for history items + site = site.set('count', ((oldSite || site).get('count') || 0) + 1) + } if (index === -1) { return sites.push(site) diff --git a/test/unit/lib/urlSuggestionTest.js b/test/unit/lib/urlSuggestionTest.js new file mode 100644 index 00000000000..c6971d22f5f --- /dev/null +++ b/test/unit/lib/urlSuggestionTest.js @@ -0,0 +1,15 @@ +/* global describe, it */ +const suggestion = require('../../../js/lib/suggestion') +const assert = require('assert') + +require('../braveUnit') + +const AGE_DECAY = 50 + +describe('suggestion', function () { + it('sorts sites correctly', function () { + assert.ok(suggestion.sortingPriority(10, 100, 50, AGE_DECAY) > suggestion.sortingPriority(10, 100, 40, AGE_DECAY), 'newer sites with equal access counts sort earlier') + assert.ok(suggestion.sortingPriority(10, 100, 50, AGE_DECAY) < suggestion.sortingPriority(11, 100, 40, AGE_DECAY), 'Sites with higher access counts sort earlier (unless time delay overriden)') + assert.ok(suggestion.sortingPriority(10, 10000000000, 10000000000, AGE_DECAY) > suggestion.sortingPriority(11, 10000000000, 1000000000, AGE_DECAY), 'much newer sites without lower counts sort with higher priority') + }) +}) diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js index 830726ab641..b9da6216a45 100644 --- a/test/unit/state/siteUtilTest.js +++ b/test/unit/state/siteUtilTest.js @@ -155,7 +155,7 @@ describe('siteUtil', function () { describe('addSite', function () { describe('sites list does not have this siteDetail', function () { - it('returns updated site list including the new site', function () { + it('returns updated site list including the new bookmark site', function () { const sites = Immutable.fromJS([]) const siteDetail = Immutable.fromJS({ lastAccessedTime: 123, @@ -171,6 +171,24 @@ describe('siteUtil', function () { }) }) + describe('with a new history item', function () { + it('returns updated site list including the new site', function () { + const sites = Immutable.fromJS([]) + const siteDetail = Immutable.fromJS({ + lastAccessedTime: 123, + tags: [], + location: testUrl1, + title: 'sample', + parentFolderId: 0, + partitionNumber: 0, + count: 1 + }) + const processedSites = siteUtil.addSite(sites, siteDetail) + const expectedSites = sites.push(siteDetail) + assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) + }) + }) + describe('sites list already has this siteDetail', function () { it('uses parentFolderId, partitionNumber, and favicon values from old siteDetail if null', function () { const oldSiteDetail = Immutable.fromJS({