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

Commit

Permalink
URLs without query parameters, paths or hash components should sort b…
Browse files Browse the repository at this point in the history
…efore URLs with these items.

Auditors: @bbondy

Fixes: #4861

Test Plan:

  0. Start with new session file
  1. visit google.com
  2. visit google.com and do a search
  3. Open a number browser and type goo
  4. Ensure google.com is the first auto-selected option
  • Loading branch information
aekeus committed Oct 17, 2016
1 parent b361e50 commit 1e55aa0
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 3 deletions.
17 changes: 17 additions & 0 deletions app/renderer/lib/suggestion.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* 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) => {
Expand Down Expand Up @@ -82,3 +83,19 @@ module.exports.sortByAccessCountWithAgeDecay = (s1, s2) => {
)
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
}
}
13 changes: 10 additions & 3 deletions js/components/urlBarSuggestions.js
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,16 @@ class UrlBarSuggestions extends ImmutableComponent {
if (pos1 - pos2 !== 0) {
return pos1 - pos2
} else {
// If there's a tie on the match location, use the age
// decay modified access count
return suggestion.sortByAccessCountWithAgeDecay(s1, s2)
// 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)
}
}
}
}
Expand Down
9 changes: 9 additions & 0 deletions test/unit/lib/urlSuggestionTest.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* global describe, it */
const suggestion = require('../../../app/renderer/lib/suggestion')
const assert = require('assert')
const Immutable = require('immutable')

require('../braveUnit')

Expand All @@ -12,4 +13,12 @@ describe('suggestion', function () {
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')
})
})

0 comments on commit 1e55aa0

Please sign in to comment.