Update getDetailFromTab to consider pinnedLocation

also normalize site key without trailing slash fix #10241 Auditors: @bsclifton, @bbondy Test Plan: 1. Go to feedly.com 2. Pin the tab 3. Bookmark it and change the location to https://feedly.com 4. Unpin the pinned tab 5. Relaunch Brave 6. There shouldn't be any pinned tab
brave · Aug 3, 2017 · 6eb2b32 · 6eb2b32
1 parent 526dc49
commit 6eb2b32
Show file tree

Hide file tree

Showing 6 changed files with 176 additions and 37 deletions.
diff --git a/app/renderer/reducers/frameReducer.js b/app/renderer/reducers/frameReducer.js
@@ -100,7 +100,12 @@ const frameReducer = (state, action, immutableAction) => {
       const pinned = immutableAction.getIn(['changeInfo', 'pinned'])
       if (pinned != null) {
         if (pinned) {
-          state = state.setIn(['frames', index, 'pinnedLocation'], tab.get('url'))
+          const history = state.getIn(['frames', index, 'history'])
+          if (history && history.size !== 0) {
+            state = state.setIn(['frames', index, 'pinnedLocation'], history.first())
+          } else {
+            state = state.setIn(['frames', index, 'pinnedLocation'], tab.get('url'))
+          }
         } else {
           state = state.deleteIn(['frames', index, 'pinnedLocation'])
         }
@@ -110,7 +115,12 @@ const frameReducer = (state, action, immutableAction) => {
       if (url != null && tab.get('pinned') === true) {
         const pinnedLocation = state.getIn(['frames', index, 'pinnedLocation'])
         if (!pinnedLocation || pinnedLocation === 'about:blank' || pinnedLocation === '') {
-          state = state.setIn(['frames', index, 'pinnedLocation'], tab.get('url'))
+          const history = state.getIn(['frames', index, 'history'])
+          if (history && history.size !== 0) {
+            state = state.setIn(['frames', index, 'pinnedLocation'], history.first())
+          } else {
+            state = state.setIn(['frames', index, 'pinnedLocation'], tab.get('url'))
+          }
         }
       }
 

diff --git a/app/sessionStore.js b/app/sessionStore.js
@@ -514,6 +514,18 @@ module.exports.runPostMigrations = (data) => {
     data.sites = sites
   }
 
+  // sites trailing slash migration
+  if (typeof data.sites === 'object') {
+    for (let key in data.sites) {
+      if (/.+|\d+|\d+/.test(key)) {
+        const site = data.sites[key]
+        const newKey = siteUtil.getSiteKey(Immutable.fromJS(site))
+        data.sites[newKey] = site
+        delete data.sites[key]
+      }
+    }
+  }
+
   return data
 }
 

diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js
@@ -76,6 +76,9 @@ module.exports.getSiteKey = function (siteDetail) {
     return folderId.toString()
   } else if (location) {
     location = UrlUtil.getLocationIfPDF(location)
+    if (location && location[location.length - 1] === '/') {
+      location = location.slice(0, -1)
+    }
     return location + '|' +
       (siteDetail.get('partitionNumber') || 0) + '|' +
       (siteDetail.get('parentFolderId') || 0)
@@ -584,6 +587,23 @@ module.exports.getDetailFromTab = function (tab, tag, sites) {
       }
     }
 
+    if (results.size === 0 && tag === siteTags.PINNED) {
+      let pinnedLocation = tab.getIn(['frame', 'pinnedLocation'])
+      if (!pinnedLocation) {
+        const history = tab.getIn(['frame', 'history'])
+        if (history && history.size !== 0) {
+          pinnedLocation = history.first()
+        }
+      }
+      if (pinnedLocation && pinnedLocation !== location) {
+        siteKey = module.exports.getSiteKey(makeImmutable({
+          location: pinnedLocation,
+          partitionNumber
+        }))
+        results = results.merge(getSitesBySubkey(sites, siteKey, tag))
+      }
+    }
+
     // update details which get returned below
     if (results.size > 0) {
       location = results.getIn([0, 'location'])

diff --git a/test/unit/app/common/state/siteCacheTest.js b/test/unit/app/common/state/siteCacheTest.js
@@ -7,8 +7,8 @@ const assert = require('assert')
 const Immutable = require('immutable')
 
 describe('siteCache', function () {
-  const testUrl1 = 'https://brave.com/'
-  const testUrl2 = 'http://example.com/'
+  const testUrl1 = 'https://brave.com'
+  const testUrl2 = 'http://example.com'
   const bookmark = Immutable.fromJS({
     lastAccessedTime: 123,
     objectId: [210, 115, 31, 176, 57, 212, 167, 120, 104, 88, 88, 27, 141, 36, 235, 226],

diff --git a/test/unit/app/sessionStoreTest.js b/test/unit/app/sessionStoreTest.js
@@ -910,4 +910,27 @@ describe('sessionStore unit tests', function () {
       }
     })
   })
+
+  describe('runPostMigrations', function () {
+    it('sites trailing slash migration', function () {
+      const data = {
+        sites: {
+          'https://brave.com/|0|0': {
+            location: 'https://brave.com/',
+            partitionNumber: 0
+          }
+        }
+      }
+      const expectedResult = {
+        sites: {
+          'https://brave.com|0|0': {
+            location: 'https://brave.com/',
+            partitionNumber: 0
+          }
+        }
+      }
+      const result = sessionStore.runPostMigrations(data)
+      assert.deepEqual(result, expectedResult)
+    })
+  })
 })