diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js index 48402cc5ce..840d89186b 100644 --- a/js/state/siteUtil.js +++ b/js/state/siteUtil.js @@ -7,6 +7,7 @@ const normalizeUrl = require('normalize-url') const siteTags = require('../constants/siteTags') const settings = require('../constants/settings') const getSetting = require('../settings').getSetting +const UrlUtil = require('../lib/urlutil') const urlParse = require('url').parse const isBookmark = (tags) => { @@ -337,9 +338,19 @@ module.exports.getDetailFromFrame = function (frame, tag) { * @param favicon favicon URL */ module.exports.updateSiteFavicon = function (sites, location, favicon) { + if (UrlUtil.isNotURL(location)) { + return sites + } + const matchingIndices = [] sites.filter((site, index) => { + if (isBookmarkFolder(site.get('tags'))) { + return false + } + if (UrlUtil.isNotURL(site.get('location'))) { + return false + } if (normalizeUrl(site.get('location')) === normalizeUrl(location)) { matchingIndices.push(index) return true diff --git a/test/unit/common/state/aboutNewTabStateTest.js b/test/unit/common/state/aboutNewTabStateTest.js index ed46338528..e45236607b 100644 --- a/test/unit/common/state/aboutNewTabStateTest.js +++ b/test/unit/common/state/aboutNewTabStateTest.js @@ -56,6 +56,14 @@ describe('aboutNewTabState', function () { }, tag: siteTags.BOOKMARK } + const historyAction = { + siteDetail: { + title: 'Brave', + location: 'https://brave.com', + lastAccessedTime: testTime + }, + tag: undefined + } describe('mergeDetails', function () { it('updates the `updatedStamp` value on success', function () { @@ -106,12 +114,6 @@ describe('aboutNewTabState', function () { assert.equal(updatedValue, bookmarkAction.siteDetail.location) }) - it('removes the tags when adding to the sites list', function () { - const state = aboutNewTabState.addSite(defaultAppState, bookmarkAction) - const updatedValue = state.getIn(['about', 'newtab', 'sites', 0, 'tags']) - assert.deepEqual(updatedValue.toJS(), []) - }) - it('will add lastAccessedTime to the siteDetail if missing from history entry', function () { const action = {siteDetail: {location: 'https://brave.com'}} const state = aboutNewTabState.addSite(defaultAppState, action) @@ -145,10 +147,10 @@ describe('aboutNewTabState', function () { }) it('removes the entry from the sites list', function () { - const stateWithSite = aboutNewTabState.addSite(defaultAppState, bookmarkAction) + const stateWithSite = aboutNewTabState.addSite(defaultAppState, historyAction) assert.equal(stateWithSite.size, 1) - const state = aboutNewTabState.removeSite(stateWithSite, bookmarkAction) + const state = aboutNewTabState.removeSite(stateWithSite, historyAction) const sites = state.getIn(['about', 'newtab', 'sites']) assert.equal(sites.size, 0) }) diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js index 4d95faa0ea..281a329f46 100644 --- a/test/unit/state/siteUtilTest.js +++ b/test/unit/state/siteUtilTest.js @@ -479,25 +479,45 @@ describe('siteUtil', function () { describe('updateSiteFavicon', function () { it('updates the favicon for all matching entries', function () { - const siteDetail1 = Immutable.fromJS({ - tags: [siteTags.BOOKMARK], - location: testUrl1, - title: 'bookmarked site' - }) - const siteDetail2 = Immutable.fromJS({ - tags: [], - location: testUrl1, - title: 'visited site' - }) - const sites = Immutable.fromJS([siteDetail1, siteDetail2]) + const sites = Immutable.fromJS([bookmarkMinFields, siteMinFields]) const processedSites = siteUtil.updateSiteFavicon(sites, testUrl1, '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 updatedSiteDetail1 = bookmarkMinFields.set('favicon', 'https://brave.com/favicon.ico') + const updatedSiteDetail2 = siteMinFields.set('favicon', 'https://brave.com/favicon.ico') const expectedSites = Immutable.fromJS([updatedSiteDetail1, updatedSiteDetail2]) assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) }) + describe('when searching for matches', function () { + it('disregards folders', function () { + const sites = siteUtil.addSite(emptySites, folderMinFields) + const processedSites = siteUtil.updateSiteFavicon(sites, testUrl1, 'https://brave.com/favicon.ico') + assert.deepEqual(processedSites.toJS(), sites.toJS()) + }) + it('ensures entry.location is truthy', function () { + const invalidSite = Immutable.fromJS({ + title: 'sample' + }) + const sites = siteUtil.addSite(emptySites, invalidSite) + const processedSites = siteUtil.updateSiteFavicon(sites, testUrl1, 'https://brave.com/favicon.ico') + assert.deepEqual(processedSites.toJS(), sites.toJS()) + }) + it('ensures input and entry.location are valid URLs', function () { + const invalidSite = Immutable.fromJS({ + title: 'sample', + location: '......not a real URL' + }) + const sites = siteUtil.addSite(emptySites, invalidSite) + const processedSites = siteUtil.updateSiteFavicon(sites, '......not a real URL', 'https://brave.com/favicon.ico') + assert.deepEqual(processedSites.toJS(), sites.toJS()) + }) + it('ensures input is truthy', function () { + const sites = siteUtil.addSite(emptySites, bookmarkMinFields) + const processedSites = siteUtil.updateSiteFavicon(sites, undefined, 'https://brave.com/favicon.ico') + assert.deepEqual(processedSites.toJS(), sites.toJS()) + }) + }) + describe('normalizes the URL when searching for matches', function () { it('normalizes trailing slashes', function () { const siteDetail1 = Immutable.fromJS({ @@ -526,16 +546,11 @@ describe('siteUtil', function () { 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 sites = Immutable.fromJS([siteDetail1, siteMinFields]) 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 updatedSiteDetail2 = siteMinFields.set('favicon', 'https://brave.com/favicon.ico') const expectedSites = Immutable.fromJS([updatedSiteDetail1, updatedSiteDetail2]) assert.deepEqual(processedSites.toJS(), expectedSites.toJS()) @@ -547,16 +562,11 @@ describe('siteUtil', function () { 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 sites = Immutable.fromJS([siteDetail1, siteMinFields]) 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 updatedSiteDetail2 = siteMinFields.set('favicon', 'https://brave.com/favicon.ico') const expectedSites = Immutable.fromJS([updatedSiteDetail1, updatedSiteDetail2]) assert.deepEqual(processedSites.toJS(), expectedSites.toJS())