fix duplicated pinned top sites after update

fix #12941
-
Some users upgrading from 0.20.x still experience the bug
that unallow usage of top sites in the newtab page.
changes in here ensure that pinned top sites are deduped at startup

app/common/state/aboutNewTabState.js

@@ -3,7 +3,7 @@
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const Immutable = require('immutable')
-const {makeImmutable} = require('./immutableUtil')
+const {makeImmutable, removeDuplicatedEntriesFromList} = require('./immutableUtil')
 const topSites = require('../../browser/api/topSites')
 const newTabData = require('../../../js/data/newTabData')
 /**
@@ -36,10 +36,20 @@ const aboutNewTabState = {
     if (!props) {
       return state
     }
-    // list is only empty if there's no pinning interaction.
-    // in this case we include the default pinned top sites list
     if (state.getIn(['about', 'newtab', 'pinnedTopSites']).isEmpty()) {
+      // list is only empty if there's no pinning interaction.
+      // in this case we include the default pinned top sites list
       state = state.setIn(['about', 'newtab', 'pinnedTopSites'], defaultPinnedSite)
+    } else {
+      // if list is not empty there's a considerable chance that
+      // due to a bug in previous versions (see #12941) the user
+      // is not able to use topSites as there are duplicated pinned sites.
+      // in this case, dedupe them all
+      const pinnedTopSites = aboutNewTabState.getPinnedTopSites(state)
+      state = state.setIn(
+        ['about', 'newtab', 'pinnedTopSites'],
+        removeDuplicatedEntriesFromList(pinnedTopSites, 'location', true)
+      )
     }
     state = state.mergeIn(['about', 'newtab'], props.newTabPageDetail)
     return state.setIn(['about', 'newtab', 'updatedStamp'], new Date().getTime())

test/unit/app/common/state/aboutNewTabStateTest.js

@@ -4,6 +4,7 @@
 
 /* global describe, it, before, after */
 const aboutNewTabState = require('../../../../../app/common/state/aboutNewTabState')
+const {aboutNewTabMaxEntries} = require('../../../../../app/browser/api/topSites')
 const Immutable = require('immutable')
 const assert = require('assert')
 const sinon = require('sinon')
@@ -91,6 +92,39 @@ describe('aboutNewTabState unit test', function () {
       const updatedValue = state.getIn(['about', 'newtab', 'testing123'])
       assert.equal(updatedValue, 'TEST STRING')
     })
+
+    it('prevents pinnedTopSites from being duplicated while keeping empty spaces', function () {
+      const action = {newTabPageDetail: {}}
+      const fakePinnedSites = [
+        {location: 'https://cliftonforthecliftonthrone.com'},
+        {location: 'https://cliftonforthecliftonthrone.com'},
+        undefined,
+        {location: 'https://ichoosearizona.com'}
+      ]
+
+      const badState = defaultAppState.mergeIn(
+        ['about', 'newtab', 'pinnedTopSites'],
+        fakePinnedSites
+      )
+
+      const state = aboutNewTabState.mergeDetails(badState, action)
+      const updatedValue = state.getIn(['about', 'newtab', 'pinnedTopSites'])
+      const expectedDedupedValue = [
+        {location: 'https://cliftonforthecliftonthrone.com'},
+        null,
+        null,
+        {location: 'https://ichoosearizona.com'}
+      ]
+
+      // ensure other entries are filled up as well in the expected result
+      const nullArray = []
+      for (let i = 0; i < (aboutNewTabMaxEntries - expectedDedupedValue.length); i++) {
+        nullArray.push(null)
+      }
+      const expectedValue = expectedDedupedValue.concat(nullArray)
+
+      assert.deepEqual(JSON.stringify(updatedValue), JSON.stringify(expectedValue))
+    })
   })
 
   describe('clearTopSites', function () {