Merge pull request #10069 from brave/fix/delete-history

Fix deleting history entry
brave · Aug 4, 2017 · 0c923e3 · 0c923e3
1 parent b18a41c
commit 0c923e3
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 17 deletions.
diff --git a/app/browser/menu.js b/app/browser/menu.js
@@ -673,8 +673,15 @@ const doAction = (state, action) => {
       }
     case windowConstants.WINDOW_CLEAR_CLOSED_FRAMES:
       {
-        closedFrames = new Immutable.OrderedMap()
-        lastClosedUrl = null
+        if (!action.location) {
+          closedFrames = new Immutable.OrderedMap()
+          lastClosedUrl = null
+        } else {
+          closedFrames = closedFrames.delete(action.location)
+          if (lastClosedUrl === action.location) {
+            lastClosedUrl = null
+          }
+        }
         updateRecentlyClosedMenuItems(state)
         break
       }
@@ -717,7 +724,7 @@ const doAction = (state, action) => {
       }
     case appConstants.APP_REMOVE_SITE:
       {
-        if (action.tag === siteTags.BOOKMARK || action.tag === siteTags.BOOKMARK_FOLDER) {
+        if (!action.tag || action.tag === siteTags.BOOKMARK || action.tag === siteTags.BOOKMARK_FOLDER) {
           createMenu(state)
         }
         break

diff --git a/js/actions/windowActions.js b/js/actions/windowActions.js
@@ -214,10 +214,13 @@ const windowActions = {
 
   /**
    * Dispatches a message to the store to clear closed frames
+   * @param {string=} location - If specified, only clear frames with this
+   *   location.
    */
-  clearClosedFrames: function () {
+  clearClosedFrames: function (location) {
     dispatch({
-      actionType: windowConstants.WINDOW_CLEAR_CLOSED_FRAMES
+      actionType: windowConstants.WINDOW_CLEAR_CLOSED_FRAMES,
+      location
     })
   },
 

diff --git a/js/entry.js b/js/entry.js
@@ -66,8 +66,8 @@ ipc.on(messages.APP_STATE_CHANGE, (e, action) => {
     : appStoreRenderer.state = Immutable.fromJS(action.state)
 })
 
-ipc.on(messages.CLEAR_CLOSED_FRAMES, () => {
-  windowActions.clearClosedFrames()
+ipc.on(messages.CLEAR_CLOSED_FRAMES, (e, location) => {
+  windowActions.clearClosedFrames(location)
 })
 
 window.addEventListener('beforeunload', function (e) {

diff --git a/js/state/siteUtil.js b/js/state/siteUtil.js
@@ -417,6 +417,11 @@ module.exports.removeSite = function (state, siteDetail, tag, reorder = true, sy
     site = site.set('tags', tags)
     return state.setIn(stateKey, site)
   } else {
+    const siteDetailTags = siteDetail.get('tags')
+    if (!tag && (!siteDetailTags || siteDetailTags.size === 0)) {
+      // Delete the site from history
+      return state.deleteIn(stateKey)
+    }
     site = site.set('lastAccessedTime', undefined)
     return state.setIn(stateKey, site)
   }

diff --git a/js/stores/appStore.js b/js/stores/appStore.js
@@ -472,6 +472,9 @@ const handleAppAction = (action) => {
     case appConstants.APP_REMOVE_SITE:
       calculateTopSites(true)
       appState = aboutHistoryState.setHistory(appState, action)
+      if (!action.tag && siteUtil.isHistoryEntry(action.siteDetail)) {
+        BrowserWindow.getAllWindows().forEach((wnd) => wnd.webContents.send(messages.CLEAR_CLOSED_FRAMES, action.siteDetail.get('location')))
+      }
       break
     case appConstants.APP_SET_DATA_FILE_ETAG:
       appState = appState.setIn([action.resourceName, 'etag'], action.etag)

diff --git a/js/stores/windowStore.js b/js/stores/windowStore.js
@@ -369,7 +369,12 @@ const doAction = (action) => {
       }
       break
     case windowConstants.WINDOW_CLEAR_CLOSED_FRAMES:
-      windowState = windowState.set('closedFrames', new Immutable.List())
+      if (!action.location) {
+        windowState = windowState.set('closedFrames', new Immutable.List())
+      } else {
+        windowState = windowState.set('closedFrames',
+          windowState.get('closedFrames').filterNot((frame) => frame.get('location') === action.location))
+      }
       break
     case windowConstants.WINDOW_SET_PREVIEW_FRAME:
       windowState = frameStateUtil.setPreviewFrameKey(windowState, action.frameKey, true)

diff --git a/test/unit/app/browser/reducers/sitesReducerTest.js b/test/unit/app/browser/reducers/sitesReducerTest.js
@@ -107,8 +107,7 @@ describe('sitesReducerTest', function () {
       let newState = sitesReducer(state, action)
       action.actionType = appConstants.APP_REMOVE_SITE
       newState = sitesReducer(newState, action).toJS()
-      assert.equal(Object.keys(newState.sites).length, 1)
-      assert.equal(Object.keys(newState.sites)[0].lastAccessedTime, undefined)
+      assert.equal(Object.keys(newState.sites).length, 0)
     })
   })
   describe('APP_MOVE_SITE', function () {

diff --git a/test/unit/state/siteUtilTest.js b/test/unit/state/siteUtilTest.js
@@ -702,13 +702,7 @@ describe('siteUtil', function () {
           location: testUrl1,
           lastAccessedTime: 123
         }
-        const expectedSites = {
-          'https://brave.com/|0|0': {
-            tags: [],
-            location: testUrl1,
-            lastAccessedTime: undefined
-          }
-        }
+        const expectedSites = {}
         const siteKey = siteUtil.getSiteKey(Immutable.fromJS(siteDetail))
         let sites = {}
         sites[siteKey] = siteDetail