Skip to content

Commit

Permalink
Sort simple URL suggestions first again
Browse files Browse the repository at this point in the history
Fix brave#11057

Since this has to do with muon url parsing libraries it needed a test inside the muon test as well

This was introduced when we switched URL parsing libraries because now parsed URL attributes are not null but instead an empty string.
  • Loading branch information
bbondy authored and syuan100 committed Nov 9, 2017
1 parent aec4915 commit 9e8b302
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 3 deletions.
6 changes: 3 additions & 3 deletions app/common/lib/suggestion.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ const sortByAccessCountWithAgeDecay = (s1, s2) => {
*/
const isSimpleDomainNameValue = (site) => isParsedUrlSimpleDomainNameValue(urlParse(getURL(site)))
const isParsedUrlSimpleDomainNameValue = (parsed) => {
if ((parsed.hash === null || parsed.hash === '#') &&
parsed.search === null && parsed.query === null && parsed.pathname === '/') {
if ((!parsed.hash || parsed.hash === '#') &&
!parsed.search && !parsed.query && parsed.pathname === '/') {
return true
} else {
return false
Expand Down Expand Up @@ -168,7 +168,7 @@ const shouldNormalizeLocation = (input) => {
const virtualSite = (sites) => {
// array of sites without paths or query params
const simple = sites.filter((parsed) => {
return (parsed.hash === null && parsed.search === null && parsed.query === null && parsed.pathname === '/')
return (!parsed.hash && !parsed.search && !parsed.query && parsed.pathname === '/')
})
// if there are no simple locations then we will build and return one
if (simple.length === 0) {
Expand Down
6 changes: 6 additions & 0 deletions js/muonTest.entry.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const urlParse = require('../app/common/urlParse')
const urlUtil = require('./lib/urlutil')
const suggestion = require('../app/common/lib/suggestion')
const assert = require('assert')

const assertEqual = (actual, expected, name) => {
Expand Down Expand Up @@ -70,6 +71,11 @@ const runTests = () => {
assertEqual(urlUtil.getOrigin('http://http/test'), 'http://http', 'getOriginSchemeHost')
assertEqual(urlUtil.getOrigin(''), null, 'getOriginNull')
assertEqual(urlUtil.getOrigin('abc'), null, 'getOriginInvalid')
console.log(suggestion.isSimpleDomainNameValue('http://http/test'))
assertEqual(suggestion.isSimpleDomainNameValue('http://test.com') &&
suggestion.isSimpleDomainNameValue('http://test.com/') &&
suggestion.isSimpleDomainNameValue('http://test.com#') &&
!suggestion.isSimpleDomainNameValue('http://test.com/test'), true, 'suggestionSimpleCheck')
}

runTests()
4 changes: 4 additions & 0 deletions test/misc-components/muonTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,8 @@ describe('muon tests', function () {
.waitForTextValue('#getOriginNull', 'success')
.waitForTextValue('#getOriginInvalid', 'success')
})
it('suggestion', function * () {
yield this.app.client
.waitForTextValue('#suggestionSimpleCheck', 'success')
})
})
8 changes: 8 additions & 0 deletions test/unit/app/common/lib/suggestionTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ describe('suggestion unit tests', function () {
assert.ok(suggestion.isSimpleDomainNameValue(siteSimple) === true, 'simple site returns 1')
assert.ok(suggestion.isSimpleDomainNameValue(siteComplex) === false, 'complex site returns 0')
})
it('URLs that end in a slash are simple', function () {
const siteSimple = Immutable.Map({ location: 'http://www.site.com/' })
assert.equal(suggestion.isSimpleDomainNameValue(siteSimple), true)
})
it('URLs that end in a hash are simple', function () {
const siteSimple = Immutable.Map({ location: 'http://www.site.com#' })
assert.ok(suggestion.isSimpleDomainNameValue(siteSimple), true)
})
})

describe('shouldNormalizeLocation', function () {
Expand Down

0 comments on commit 9e8b302

Please sign in to comment.