From 1e55aa023479eb695ceff9566a2ede1945cdc220 Mon Sep 17 00:00:00 2001 From: Aubrey Keus Date: Mon, 17 Oct 2016 10:40:45 -0400 Subject: [PATCH] URLs without query parameters, paths or hash components should sort before 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 --- app/renderer/lib/suggestion.js | 17 +++++++++++++++++ js/components/urlBarSuggestions.js | 13 ++++++++++--- test/unit/lib/urlSuggestionTest.js | 9 +++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/app/renderer/lib/suggestion.js b/app/renderer/lib/suggestion.js index 30f461fd0c6..afcce81d6cd 100644 --- a/app/renderer/lib/suggestion.js +++ b/app/renderer/lib/suggestion.js @@ -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) => { @@ -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 + } +} diff --git a/js/components/urlBarSuggestions.js b/js/components/urlBarSuggestions.js index 0ac8e0a1e54..b4a17131339 100644 --- a/js/components/urlBarSuggestions.js +++ b/js/components/urlBarSuggestions.js @@ -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) + } } } } diff --git a/test/unit/lib/urlSuggestionTest.js b/test/unit/lib/urlSuggestionTest.js index 1e89181f4e5..01d82f1f8f9 100644 --- a/test/unit/lib/urlSuggestionTest.js +++ b/test/unit/lib/urlSuggestionTest.js @@ -1,6 +1,7 @@ /* global describe, it */ const suggestion = require('../../../app/renderer/lib/suggestion') const assert = require('assert') +const Immutable = require('immutable') require('../braveUnit') @@ -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') + }) }) +