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

Commit

Permalink
Deprioritize search suggestions that start with 'http'
Browse files Browse the repository at this point in the history
Fix #9141

Test Plan:
1. automated tests related to search suggestions should pass
2. Disable all options except 'Autocomplete search term' under search in preferences
3. type 'bug' in the URL bar. you should not see any results that start with 'http'
4. type 'http' in the URL bar. you should see some results that start with 'http'.
  • Loading branch information
diracdeltas committed May 31, 2017
1 parent d70cc77 commit d37f325
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 7 deletions.
25 changes: 24 additions & 1 deletion app/common/lib/suggestion.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const _ = require('underscore')
const Immutable = require('immutable')
const {makeImmutable} = require('../../common/state/immutableUtil')
const {isUrl, aboutUrls, isNavigatableAboutPage, isSourceAboutUrl} = require('../../../js/lib/appUrlUtil')
const {isHttpOrHttps} = require('../../../js/lib/urlutil')
const suggestionTypes = require('../../../js/constants/suggestionTypes')
const getSetting = require('../../../js/settings').getSetting
const settings = require('../../../js/constants/settings')
Expand Down Expand Up @@ -302,6 +303,25 @@ const getSortByDomainForHosts = (userInputHost) => {
}
}

/**
* Returns a function that sorts search suggestion results.
* Results starting with 'http://' or 'https://' are de-prioritized.
*/
const getSortForSearchSuggestions = (userInput) => {
return (s1, s2) => {
if (userInput.startsWith('http')) {
return 0
}
if (!isHttpOrHttps(s1) && isHttpOrHttps(s2)) {
return -1
}
if (isHttpOrHttps(s1) && !isHttpOrHttps(s2)) {
return 1
}
return 0
}
}

/**
* Sorts 2 URLS by if they are a simple URL or not.
* Returns the normal -1, 1, or 0 result for sort functions.
Expand Down Expand Up @@ -526,11 +546,13 @@ const getSearchSuggestions = (state, tabId, urlLocationLower) => {
let suggestionsList = Immutable.List()
if (getSetting(settings.OFFER_SEARCH_SUGGESTIONS)) {
const searchResults = state.get('searchResults')
const sortHandler = getSortForSearchSuggestions(urlLocationLower)
if (searchResults) {
suggestionsList = mapListToElements({
data: searchResults,
maxResults: config.urlBarSuggestions.maxSearch,
type: suggestionTypes.SEARCH
type: suggestionTypes.SEARCH,
sortHandler
})
}
}
Expand Down Expand Up @@ -606,6 +628,7 @@ module.exports = {
sortingPriority,
sortByAccessCountWithAgeDecay,
getSortForSuggestions,
getSortForSearchSuggestions,
getSortByPath,
sortBySimpleURL,
getSortByDomainForSites,
Expand Down
27 changes: 21 additions & 6 deletions test/navbar-components/urlBarSuggestionsTest.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* global describe, it, before, beforeEach */
/* global describe, it, beforeEach */

const Brave = require('../lib/brave')
const settings = require('../../js/constants/settings')
Expand Down Expand Up @@ -205,19 +205,17 @@ describe('urlBarSuggestions', function () {
})

describe('search suggestions', function () {
Brave.beforeAll(this)
Brave.beforeEach(this)

before(function * () {
beforeEach(function * () {
yield this.app.client
.waitForUrl(Brave.newTabUrl)
.waitForBrowserWindow()
.waitForVisible(urlInput)
.changeSetting(settings.OFFER_SEARCH_SUGGESTIONS, true)
})

it('Finds search suggestions and performs a search when selected', function * () {
yield this.app.client
.changeSetting(settings.OFFER_SEARCH_SUGGESTIONS, true)

// Until a refactor happens with search suggestions,
// they are a bit fragile if you aren't actually typing.
// So this for loop avoids an intermittent failure.
Expand All @@ -239,4 +237,21 @@ describe('search suggestions', function () {
.keys(Brave.keys.ENTER)
.waitForInputText(urlInput, /google.*\/.*q=what.+is/)
})

it('does not offer URL suggestions if user is not typing a URL', function * () {
const input = 'bug'
for (let i = 0; i < input.length; i++) {
yield this.app.client
.pause(50)
.keys(input[i])
}
yield this.app.client
.waitForVisible(urlBarSuggestions)
.waitForExist(urlBarSuggestions + ' li.suggestionItem[data-index="2"]')
.waitUntil(function () {
return this.getText(urlBarSuggestions).then((text) => {
return text.includes('bug') && !text.includes('http')
})
})
})
})
22 changes: 22 additions & 0 deletions test/unit/app/common/lib/suggestionTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,28 @@ describe('suggestion unit tests', function () {
assert(this.sort('www.google.com', 'mygoogle.com') < 0)
})
})
describe('getSortForSearchSuggestions', function () {
it('0 if suggestions are not URLs', function () {
const sort = suggestion.getSortForSearchSuggestions('bug')
assert.equal(sort('foo', 'bar'), 0)
})
it('0 if suggestions are the same', function () {
const sort = suggestion.getSortForSearchSuggestions('bug')
assert.equal(sort('http://foo.com', 'http://foo.com'), 0)
})
it('0 if user input starts with HTTP', function () {
const sort = suggestion.getSortForSearchSuggestions('http://')
assert.equal(sort('http://foo.com', 'foo'), 0)
})
it('negative if second suggestion is HTTP', function () {
const sort = suggestion.getSortForSearchSuggestions('bug')
assert.equal(sort('bugs', 'http://bug'), -1)
})
it('positive if first suggestion is HTTP', function () {
const sort = suggestion.getSortForSearchSuggestions('bug')
assert.equal(sort('https://bug', 'bugs'), 1)
})
})
describe('getSortForSuggestions', function () {
describe('with url entered as path', function () {
before(function () {
Expand Down

0 comments on commit d37f325

Please sign in to comment.