From c2982e678940adadd08ec774c338cdf8bb2ffc4f Mon Sep 17 00:00:00 2001 From: Aubrey Keus Date: Wed, 12 Oct 2016 10:19:05 -0400 Subject: [PATCH] Change URL suggestions to consider last access timestamp and access count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Re-order results to show history items before bookmark items * Record number of times site is visited * Re-order history items based on number of times visited and last access timestamp * Update and create tests Fixes: #3049 Auditors: @bbondy Test plan: A new attribute, count, is added to history items in the sites section of save-session-1. This attribute records the number of times a site has been accessed. The test plan is broken in two components, one to verify the count attribute is incremented on access, the second to verify that the count and last access time are used when sorting history suggestions. Scenario 1 Part A 1. Clear save-session-1 2. Access a site 3. Close browser 4. Verify that the count attribute for the saved site in save-session-1 has a value of 1 Part B 1. Open browser 2. Navigate to site from Part A 3. Close browser 4. Verify that the count attribute for the saved site in save-session-1 has an incremented value Scenario 2 1. Clear save-session-1 2. Open browser 3. Visit gm.com and gm.ca 4. Close browser 5. Verify that the count attributes of the two new sites are identical (1) 6. Open browser to a new tab 7. Enter ‘gm’ into the url bar 8. Verify that the most recently visited of the two gm sites is the first entry in the history auto-suggest list --- js/components/urlBarSuggestions.js | 8 +-- js/constants/appConfig.js | 3 ++ js/lib/suggestion.js | 84 ++++++++++++++++++++++++++++++ js/state/siteUtil.js | 4 ++ test/unit/lib/urlSuggestionTest.js | 15 ++++++ test/unit/state/siteUtilTest.js | 20 ++++++- 6 files changed, 130 insertions(+), 4 deletions(-) create mode 100644 js/lib/suggestion.js create mode 100644 test/unit/lib/urlSuggestionTest.js 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({