From ebfc338388db4fab3679d4330d7b4868e521c466 Mon Sep 17 00:00:00 2001 From: yan Date: Thu, 16 Mar 2017 22:05:34 +0000 Subject: [PATCH] remove Brave from autosuggest and history (again) fix #7655 Test Plan: 1. automated tests should pass 2. open clean instance of brave 3. type 'fa' into urlbar. you should not see brave's facebook page in the autosuggest. 4. go to about:history. there should be no brave sites. --- .../reducers/urlBarSuggestionsReducer.js | 6 +++++- docs/state.md | 2 +- js/data/newTabData.js | 19 ++++++++----------- js/state/siteUtil.js | 5 +++++ test/about/historyTest.js | 5 +++++ test/components/urlBarSuggestionsTest.js | 10 +++++++--- test/unit/state/siteUtilTest.js | 8 ++++++++ 7 files changed, 39 insertions(+), 16 deletions(-) diff --git a/app/renderer/reducers/urlBarSuggestionsReducer.js b/app/renderer/reducers/urlBarSuggestionsReducer.js index 6ecf5ab7fa0..5b06147bc27 100644 --- a/app/renderer/reducers/urlBarSuggestionsReducer.js +++ b/app/renderer/reducers/urlBarSuggestionsReducer.js @@ -125,7 +125,11 @@ const updateUrlSuffix = (state, suggestionList) => { const generateNewSuggestionsList = (state) => { const activeFrameKey = state.get('activeFrameKey') const urlLocation = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'location'])) - const sites = appStoreRenderer.state.get('sites') + let sites = appStoreRenderer.state.get('sites') + if (sites) { + // Filter out Brave default newtab sites + sites = sites.filterNot((site) => Immutable.is(site.get('tags'), new Immutable.List(['default'])) && site.get('lastAccessedTime') === 1) + } const searchResults = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'suggestions', 'searchResults'])) const frameSearchDetail = state.getIn(activeFrameStatePath(state).concat(['navbar', 'urlbar', 'searchDetail'])) const searchDetail = state.get('searchDetail') diff --git a/docs/state.md b/docs/state.md index 74bfe0bb013..e8f67742a94 100644 --- a/docs/state.md +++ b/docs/state.md @@ -232,7 +232,7 @@ AppStore originalSeed: Array., // only set for bookmarks that have been synced before a sync profile reset parentFolderId: number, // set for bookmarks and bookmark folders only partitionNumber: number, // optionally specifies a specific session - tags: [string], // empty, 'bookmark', 'bookmark-folder', 'pinned', or 'reader' + tags: [string], // empty, 'bookmark', 'bookmark-folder', 'pinned', 'default', or 'reader' themeColor: string, // CSS compatible color string title: string } // folder: folderId; bookmark/history: location + partitionNumber + parentFolderId diff --git a/js/data/newTabData.js b/js/data/newTabData.js index 38fe8cfc7fc..ccc0b4e286c 100644 --- a/js/data/newTabData.js +++ b/js/data/newTabData.js @@ -5,10 +5,7 @@ const {getBraveExtUrl} = require('../lib/appUrlUtil') const iconPath = getBraveExtUrl('img/newtab/defaultTopSitesIcon') -/** - * Let lastAccessedTime be the first-time user see the new tab page - */ -const now = Date.now() +const now = 1 module.exports.pinnedTopSites = [ { @@ -17,7 +14,7 @@ module.exports.pinnedTopSites = [ "lastAccessedTime": now, "location": "https://twitter.com/brave", "partitionNumber": 0, - "tags": [], + "tags": ['default'], "themeColor": "rgb(255, 255, 255)", "title": "Brave Software (@brave) | Twitter" } @@ -30,7 +27,7 @@ module.exports.topSites = [ "lastAccessedTime": now, "location": "https://twitter.com/brave", "partitionNumber": 0, - "tags": [], + "tags": ['default'], "themeColor": "rgb(255, 255, 255)", "title": "Brave Software (@brave) | Twitter" }, { @@ -39,7 +36,7 @@ module.exports.topSites = [ "lastAccessedTime": now, "location": "https://www.facebook.com/BraveSoftware/", "partitionNumber": 0, - "tags": [], + "tags": ['default'], "themeColor": "rgb(59, 89, 152)", "title": "Brave Software | Facebook" }, { @@ -48,7 +45,7 @@ module.exports.topSites = [ "lastAccessedTime": now, "location": "https://www.youtube.com/bravesoftware", "partitionNumber": 0, - "tags": [], + "tags": ['default'], "themeColor": "#E62117", "title": "Brave Browser - YouTube" }, { @@ -57,7 +54,7 @@ module.exports.topSites = [ "lastAccessedTime": now, "location": "https://brave.com/", "partitionNumber": 0, - "tags": [], + "tags": ['default'], "themeColor": "rgb(255, 255, 255)", "title": "Brave Software | Building a Better Web" }, { @@ -66,7 +63,7 @@ module.exports.topSites = [ "lastAccessedTime": now, "location": "https://itunes.apple.com/app/brave-web-browser/id1052879175?mt=8", "partitionNumber": 0, - "tags": [], + "tags": ['default'], "themeColor": "rgba(255, 255, 255, 1)", "title": "Brave Web Browser: Fast with built-in adblock on the App Store" }, { @@ -75,7 +72,7 @@ module.exports.topSites = [ "lastAccessedTime": now, "location": "https://play.google.com/store/apps/details?id=com.brave.browser", "partitionNumber": 0, - "tags": [], + "tags": ['default'], "themeColor": "rgb(241, 241, 241)", "title": "Brave Browser: Fast AdBlock – Apps para Android no Google Play" } diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js index 6ce61f951d1..b5f2c2fb0fd 100644 --- a/js/state/siteUtil.js +++ b/js/state/siteUtil.js @@ -580,6 +580,11 @@ module.exports.isHistoryEntry = function (siteDetail) { if (siteDetail.get('location').startsWith('about:')) { return false } + if (Immutable.is(siteDetail.get('tags'), new Immutable.List(['default'])) && + siteDetail.get('lastAccessedTime') === 1) { + // This is a Brave default newtab site + return false + } return !!siteDetail.get('lastAccessedTime') && !isBookmarkFolder(siteDetail.get('tags')) } return false diff --git a/test/about/historyTest.js b/test/about/historyTest.js index 6c434408d49..8c6e3c767a0 100644 --- a/test/about/historyTest.js +++ b/test/about/historyTest.js @@ -48,6 +48,11 @@ describe('about:history', function () { yield addDemoSites(this.app.client) }) + it('does not display Brave default sites', function * () { + yield this.app.client + .waitForVisible('table.sortableTable td.title[data-sort="Brave"]') + .waitForElementCount('td.time', 4) + }) it('displays entries with title as: title or URL', function * () { yield this.app.client .waitForVisible('table.sortableTable td.title[data-sort="Brave"]') diff --git a/test/components/urlBarSuggestionsTest.js b/test/components/urlBarSuggestionsTest.js index 9c68d468d03..366a859532e 100644 --- a/test/components/urlBarSuggestionsTest.js +++ b/test/components/urlBarSuggestionsTest.js @@ -35,13 +35,17 @@ describe('urlBarSuggestions', function () { .waitForElementFocus(urlInput) }) + it('does not show Brave default sites', function * () { + yield this.app.client.ipcSend('shortcut-focus-url') + .waitForElementFocus(urlInput) + .setValue(urlInput, 'twitter') + .waitForElementCount('li.suggestionItem', 1) + }) + it('show suggestion when single letter is typed in', function * () { yield this.app.client.ipcSend('shortcut-focus-url') .waitForElementFocus(urlInput) .setInputText(urlInput, 'a') - .waitUntil(function () { - return this.getValue(urlInput).then((val) => val === 'a') - }) .waitForExist(urlBarSuggestions) }) diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js index b184f7fc02b..c5f59df8974 100644 --- a/test/unit/state/siteUtilTest.js +++ b/test/unit/state/siteUtilTest.js @@ -1212,6 +1212,14 @@ describe('siteUtil', function () { }) assert.equal(siteUtil.isHistoryEntry(siteDetail), false) }) + it('returns false for a brave default site', function () { + const siteDetail = Immutable.fromJS({ + location: testUrl1, + tags: ['default'], + lastAccessedTime: 1 + }) + assert.equal(siteUtil.isHistoryEntry(siteDetail), false) + }) it('returns false if input is falsey', function () { assert.equal(siteUtil.isHistoryEntry(null), false) assert.equal(siteUtil.isHistoryEntry(undefined), false)