Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Change URL suggestions to consider last access timestamp and access c…
Browse files Browse the repository at this point in the history
…ount

  * 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
  • Loading branch information
aekeus committed Oct 13, 2016
1 parent 66774ae commit c2982e6
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 4 deletions.
8 changes: 5 additions & 3 deletions js/components/urlBarSuggestions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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)
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions js/constants/appConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
84 changes: 84 additions & 0 deletions js/lib/suggestion.js
Original file line number Diff line number Diff line change
@@ -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
}
4 changes: 4 additions & 0 deletions js/state/siteUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions test/unit/lib/urlSuggestionTest.js
Original file line number Diff line number Diff line change
@@ -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')
})
})
20 changes: 19 additions & 1 deletion test/unit/state/siteUtilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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({
Expand Down

0 comments on commit c2982e6

Please sign in to comment.