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

URL is normalized when updating favicon for a given site #5445

Merged
merged 1 commit into from
Nov 7, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion js/state/siteUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

'use strict'
const Immutable = require('immutable')
const normalizeUrl = require('normalize-url')
const siteTags = require('../constants/siteTags')
const settings = require('../constants/settings')
const getSetting = require('../settings').getSetting
Expand Down Expand Up @@ -339,7 +340,7 @@ module.exports.updateSiteFavicon = function (sites, location, favicon) {
const matchingIndices = []

sites.filter((site, index) => {
if (site.get('location') === location) {
if (normalizeUrl(site.get('location')) === normalizeUrl(location)) {
matchingIndices.push(index)
return true
}
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
"ledger-publisher": "^0.8.90",
"lru_cache": "^1.0.0",
"moment": "^2.15.1",
"normalize-url": "^1.7.0",
"punycode": "^2.0.0",
"qr-image": "^3.1.0",
"random-lib": "2.1.0",
Expand Down
88 changes: 87 additions & 1 deletion test/unit/state/siteUtilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ describe('siteUtil', function () {
const siteDetail2 = Immutable.fromJS({
tags: [],
location: testUrl1,
title: 'bookmarked site'
title: 'visited site'
})
const sites = Immutable.fromJS([siteDetail1, siteDetail2])
const processedSites = siteUtil.updateSiteFavicon(sites, testUrl1, 'https://brave.com/favicon.ico')
Expand All @@ -497,6 +497,92 @@ describe('siteUtil', function () {

assert.deepEqual(processedSites.toJS(), expectedSites.toJS())
})

describe('normalizes the URL when searching for matches', function () {
it('normalizes trailing slashes', function () {
const siteDetail1 = Immutable.fromJS({
tags: [siteTags.BOOKMARK],
location: 'https://brave.com',
title: 'bookmarked site'
})
const siteDetail2 = Immutable.fromJS({
tags: [],
location: 'https://brave.com/',
title: 'visited site'
})

const sites = Immutable.fromJS([siteDetail1, siteDetail2])
const processedSites = siteUtil.updateSiteFavicon(sites, 'https://brave.com/', 'https://brave.com/favicon.ico')
const updatedSiteDetail1 = siteDetail1.set('favicon', 'https://brave.com/favicon.ico')
const updatedSiteDetail2 = siteDetail2.set('favicon', 'https://brave.com/favicon.ico')
const expectedSites = Immutable.fromJS([updatedSiteDetail1, updatedSiteDetail2])

assert.deepEqual(processedSites.toJS(), expectedSites.toJS())
})

it('normalizes port numbers', function () {
const siteDetail1 = Immutable.fromJS({
tags: [siteTags.BOOKMARK],
location: 'https://brave.com:443',
title: 'bookmarked site'
})
const siteDetail2 = Immutable.fromJS({
tags: [],
location: 'https://brave.com/',
title: 'visited site'
})

const sites = Immutable.fromJS([siteDetail1, siteDetail2])
const processedSites = siteUtil.updateSiteFavicon(sites, 'https://brave.com/', 'https://brave.com/favicon.ico')
const updatedSiteDetail1 = siteDetail1.set('favicon', 'https://brave.com/favicon.ico')
const updatedSiteDetail2 = siteDetail2.set('favicon', 'https://brave.com/favicon.ico')
const expectedSites = Immutable.fromJS([updatedSiteDetail1, updatedSiteDetail2])

assert.deepEqual(processedSites.toJS(), expectedSites.toJS())
})

it('strips www', function () {
const siteDetail1 = Immutable.fromJS({
tags: [siteTags.BOOKMARK],
location: 'https://www.brave.com/',
title: 'bookmarked site'
})
const siteDetail2 = Immutable.fromJS({
tags: [],
location: 'https://brave.com/',
title: 'visited site'
})

const sites = Immutable.fromJS([siteDetail1, siteDetail2])
const processedSites = siteUtil.updateSiteFavicon(sites, 'https://brave.com/', 'https://brave.com/favicon.ico')
const updatedSiteDetail1 = siteDetail1.set('favicon', 'https://brave.com/favicon.ico')
const updatedSiteDetail2 = siteDetail2.set('favicon', 'https://brave.com/favicon.ico')
const expectedSites = Immutable.fromJS([updatedSiteDetail1, updatedSiteDetail2])

assert.deepEqual(processedSites.toJS(), expectedSites.toJS())
})

it('removes the fragment', function () {
const siteDetail1 = Immutable.fromJS({
tags: [siteTags.BOOKMARK],
location: 'https://www.brave.com/#contact',
title: 'bookmarked site'
})
const siteDetail2 = Immutable.fromJS({
tags: [],
location: 'https://brave.com/#people',
title: 'visited site'
})

const sites = Immutable.fromJS([siteDetail1, siteDetail2])
const processedSites = siteUtil.updateSiteFavicon(sites, 'https://brave.com/#about', 'https://brave.com/favicon.ico')
const updatedSiteDetail1 = siteDetail1.set('favicon', 'https://brave.com/favicon.ico')
const updatedSiteDetail2 = siteDetail2.set('favicon', 'https://brave.com/favicon.ico')
const expectedSites = Immutable.fromJS([updatedSiteDetail1, updatedSiteDetail2])

assert.deepEqual(processedSites.toJS(), expectedSites.toJS())
})
})
})

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