Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Commit

Permalink
Fix deleting history entry
Browse files Browse the repository at this point in the history
fix #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
  • Loading branch information
diracdeltas committed Aug 3, 2017
1 parent 00cc603 commit 67aef34
Show file tree
Hide file tree
Showing 8 changed files with 33 additions and 17 deletions.
13 changes: 10 additions & 3 deletions app/browser/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions js/actions/windowActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
},

Expand Down
4 changes: 2 additions & 2 deletions js/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions js/state/siteUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 3 additions & 0 deletions js/stores/appStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion js/stores/windowStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions test/unit/app/browser/reducers/sitesReducerTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
8 changes: 1 addition & 7 deletions test/unit/state/siteUtilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 67aef34

Please sign in to comment.