Merge pull request #4809 from brave/url-frequency

Change URL suggestions to consider last access timestamp and access count

app/renderer/lib/suggestion.js

@@ -0,0 +1,101 @@
+/* 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 urlParser = require('url')
+const appConfig = require('../../../js/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
+}
+
+/*
+ * Return a 1 if the url is 'simple' as in without query, search or
+ * hash components. Return 0 otherwise.
+ *
+ * @param {ImmutableObject} site - object represent history entry
+ *
+ */
+module.exports.simpleDomainNameValue = (site) => {
+  const parsed = urlParser.parse(site.get('location'))
+  if (parsed.hash === null && parsed.search === null && parsed.query === null && parsed.pathname === '/') {
+    return 1
+  } else {
+    return 0
+  }
+}

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('../../app/renderer/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,18 +255,26 @@ 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
+          // 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)
+          }
         }
       }
     }
 
-    // bookmarks
-    if (getSetting(settings.BOOKMARK_SUGGESTIONS)) {
+    // history
+    if (getSetting(settings.HISTORY_SUGGESTIONS)) {
       suggestions = suggestions.concat(mapListToElements({
         data: props.sites,
-        maxResults: config.urlBarSuggestions.maxBookmarkSites,
-        type: suggestionTypes.BOOKMARK,
+        maxResults: config.urlBarSuggestions.maxHistorySites,
+        type: suggestionTypes.HISTORY,
         clickHandler: navigateClickHandler((site) => {
           return site.get('location')
         }),
@@ -277,17 +286,17 @@ class UrlBarSuggestions extends ImmutableComponent {
           const location = site.get('location') || ''
           return (title.toLowerCase().includes(urlLocationLower) ||
             location.toLowerCase().includes(urlLocationLower)) &&
-            site.get('tags') && site.get('tags').includes(siteTags.BOOKMARK)
+            (!site.get('tags') || site.get('tags').size === 0)
         }
       }))
     }
 
-    // history
-    if (getSetting(settings.HISTORY_SUGGESTIONS)) {
+    // bookmarks
+    if (getSetting(settings.BOOKMARK_SUGGESTIONS)) {
       suggestions = suggestions.concat(mapListToElements({
         data: props.sites,
-        maxResults: config.urlBarSuggestions.maxHistorySites,
-        type: suggestionTypes.HISTORY,
+        maxResults: config.urlBarSuggestions.maxBookmarkSites,
+        type: suggestionTypes.BOOKMARK,
         clickHandler: navigateClickHandler((site) => {
           return site.get('location')
         }),
@@ -299,7 +308,7 @@ class UrlBarSuggestions extends ImmutableComponent {
           const location = site.get('location') || ''
           return (title.toLowerCase().includes(urlLocationLower) ||
             location.toLowerCase().includes(urlLocationLower)) &&
-            (!site.get('tags') || site.get('tags').size === 0)
+            site.get('tags') && site.get('tags').includes(siteTags.BOOKMARK)
         }
       }))
     }

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

js/state/siteUtil.js

@@ -124,6 +124,10 @@ const mergeSiteDetails = (oldSiteDetail, newSiteDetail, tag, folderId) => {
   if (newSiteDetail.get('themeColor') || oldSiteDetail && oldSiteDetail.get('themeColor')) {
     site = site.set('themeColor', newSiteDetail.get('themeColor') || oldSiteDetail.get('themeColor'))
   }
+  if (site.get('tags').size === 0) {
+    // Increment the visit count for history items
+    site = site.set('count', ((oldSiteDetail || site).get('count') || 0) + 1)
+  }
 
   return site
 }

test/unit/lib/urlSuggestionTest.js

@@ -0,0 +1,24 @@
+/* global describe, it */
+const suggestion = require('../../../app/renderer/lib/suggestion')
+const assert = require('assert')
+const Immutable = require('immutable')
+
+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')
+  })
+
+  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')
+  })
+})
+

test/unit/state/siteUtilTest.js

@@ -27,6 +27,10 @@ describe('siteUtil', function () {
     parentFolderId: 0,
     tags: [siteTags.BOOKMARK_FOLDER]
   })
+  const siteMinFields = Immutable.fromJS({
+    location: testUrl1,
+    title: 'sample'
+  })
 
   describe('getSiteIndex', function () {
     it('returns -1 if sites is falsey', function () {
@@ -178,7 +182,17 @@ describe('siteUtil', function () {
       const expectedSites = Immutable.fromJS([bookmarkAllFields])
       assert.deepEqual(processedSites.getIn([0, 'tags']), expectedSites.getIn([0, 'tags']))
     })
-
+    describe('record count', function () {
+      var processedSites
+      it('create history record with count', function () {
+        processedSites = siteUtil.addSite(emptySites, siteMinFields)
+        assert.deepEqual(processedSites.getIn([0, 'count']), 1)
+      })
+      it('increments count for history item', function () {
+        processedSites = siteUtil.addSite(processedSites, siteMinFields)
+        assert.deepEqual(processedSites.getIn([0, 'count']), 2)
+      })
+    })
     describe('for new entries (oldSite is null)', function () {
       describe('when adding bookmark', function () {
         it('preserves existing siteDetail fields', function () {
@@ -192,7 +206,6 @@ describe('siteUtil', function () {
           assert.deepEqual(processedSites.getIn([0, 'tags']).toJS(), [siteTags.BOOKMARK])
         })
       })
-
       describe('when adding bookmark folder', function () {
         it('assigns a folderId', function () {
           const processedSites = siteUtil.addSite(emptySites, folderMinFields)