Merge pull request #5728 from cezaraugusto/feature/newtab/5638

Stop about pages from being shown on topSites
brave · Nov 21, 2016 · 99aadab · 99aadab
2 parents 2239454 + bbf0851
commit 99aadab
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 1 deletion.
diff --git a/app/common/state/aboutNewTabState.js b/app/common/state/aboutNewTabState.js
@@ -5,6 +5,7 @@
 const Immutable = require('immutable')
 const {makeImmutable} = require('./immutableUtil')
 const siteUtil = require('../../../js/state/siteUtil')
+const {isSourceAboutUrl} = require('../../../js/lib/appUrlUtil')
 const aboutNewTabMaxEntries = 100
 
 const compareSites = (site1, site2) => {
@@ -67,6 +68,7 @@ const getTopSites = (state) => {
   // remove folders; sort by visit count; enforce a max limit
   const sites = (state.get('sites') || new Immutable.List())
     .filter((site) => !siteUtil.isFolder(site))
+    .filter((site) => !isSourceAboutUrl(site.get('location')))
     .sort(sortCountDescending)
     .slice(0, aboutNewTabMaxEntries)
 

diff --git a/test/about/newTabTest.js b/test/about/newTabTest.js
@@ -1,7 +1,7 @@
 /* global describe, it, before, beforeEach */
 
 const Brave = require('../lib/brave')
-const {urlInput} = require('../lib/selectors')
+const {urlInput, navigator, navigatorBookmarked, navigatorNotBookmarked, doneButton} = require('../lib/selectors')
 const {getTargetAboutUrl} = require('../../js/lib/appUrlUtil')
 const aboutNewTabUrl = getTargetAboutUrl('about:newtab')
 
@@ -36,6 +36,31 @@ describe('about:newtab tests', function () {
       .loadUrl(aboutNewTabUrl)
   }
 
+  function * addDemoAboutPages (client) {
+    yield client
+      .addSite({ location: 'about:about' })
+      .addSite({ location: 'about:adblock' })
+      .addSite({ location: 'about:autofill' })
+      .addSite({ location: 'about:blank' })
+      .addSite({ location: 'about:bookmarks' })
+      .addSite({ location: 'about:brave' })
+      .addSite({ location: 'about:certerror' })
+      .addSite({ location: 'about:config' })
+      .addSite({ location: 'about:downloads' })
+      .addSite({ location: 'about:error' })
+      .addSite({ location: 'about:extensions' })
+      .addSite({ location: 'about:flash' })
+      .addSite({ location: 'about:history' })
+      .addSite({ location: 'about:newtab' })
+      .addSite({ location: 'about:passwords' })
+      .addSite({ location: 'about:preferences' })
+      .addSite({ location: 'about:safebrowsing' })
+      .addSite({ location: 'about:styles' })
+      .waitForExist('.tab[data-frame-key="1"]')
+      .tabByIndex(0)
+      .url(aboutNewTabUrl)
+  }
+
   describe('page content', function () {
     Brave.beforeAll(this)
 
@@ -133,5 +158,28 @@ describe('about:newtab tests', function () {
         .moveToObject('.timeSaved')
         .waitForVisible('.pinnedTopSite')
     })
+
+    it('doesn\'t show about pages on topSites grid', function * () {
+      // Adding about pages shouldn't add them to topSites grid
+      yield addDemoAboutPages(this.app.client)
+
+      // Bookmarking an about:page should not add it to grid as well
+      yield this.app.client
+        .tabByUrl(aboutNewTabUrl)
+        .windowParentByUrl(aboutNewTabUrl)
+        .waitForVisible(navigator)
+        .moveToObject(navigator)
+        .waitForVisible(navigatorNotBookmarked)
+        .click(navigatorNotBookmarked)
+        .waitForVisible(doneButton)
+        .click(doneButton)
+        .moveToObject(navigator)
+        .waitForVisible(navigatorBookmarked)
+
+      yield reloadNewTab(this.app.client)
+
+      yield this.app.client
+        .waitForExist('.topSitesElementFavicon', 3000, true)
+    })
   })
 })