Merge pull request #5445 from bsclifton/normalize-bookmark-url

URL is normalized when updating favicon for a given site
brave · Nov 7, 2016 · 4d54312 · 4d54312
2 parents de10197 + 0cfda2f
commit 4d54312
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 2 deletions.
diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js
@@ -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
@@ -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
     }

diff --git a/package.json b/package.json
@@ -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",

diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js
@@ -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')
@@ -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 () {