From 67aef3459a97e4efb7072c02b77b5bbfb00a15ce Mon Sep 17 00:00:00 2001 From: yan Date: Wed, 19 Jul 2017 16:14:40 -0700 Subject: [PATCH] Fix deleting history entry fix https://github.com/brave/browser-laptop/issues/8761 test plan: 1. open a new tab, go to some site 2. close the tab 3. open about:history. right-click to delete the site. it should disappear from the list 4. open the History menu. you should no longer see the site under 'Recently Closed'. 5. repeat above steps but opening/closing two tabs and selecting both of them to delete instead of only one --- app/browser/menu.js | 13 ++++++++++--- js/actions/windowActions.js | 7 +++++-- js/entry.js | 4 ++-- js/state/siteUtil.js | 5 +++++ js/stores/appStore.js | 3 +++ js/stores/windowStore.js | 7 ++++++- test/unit/app/browser/reducers/sitesReducerTest.js | 3 +-- test/unit/state/siteUtilTest.js | 8 +------- 8 files changed, 33 insertions(+), 17 deletions(-) diff --git a/app/browser/menu.js b/app/browser/menu.js index a5b550a8619..36b40c22711 100644 --- 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 } @@ -720,7 +727,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 index 9f8c5f73149..8dbd5e7d66c 100644 --- 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 index 70c04743ace..d9b6fca7dc9 100644 --- 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 index 5320b15a9c7..cbb6ce0920c 100644 --- 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 index 04a72501ba5..c5d735d05ca 100644 --- 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 index 4e09138b53f..a76dccdbce9 100644 --- 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 index 15d5bc560c7..d51c44c4c7e 100644 --- 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 index f45653f3ca0..0cfed8ffa5b 100644 --- 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