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

Bookmarked history entries now show context menu in about:history #3336

Merged
merged 1 commit into from
Aug 23, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/windowActions.md
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,12 @@ The new frame is expected to appear at the index it was last closed at



### clearClosedFrames()

Dispatches a message to the store to clear closed frames



### setActiveFrame(frameProps)

Dispatches a message to the store to set a new frame as the active frame.
Expand Down
5 changes: 3 additions & 2 deletions js/contextMenus.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ function siteDetailTemplateInit (siteDetail, activeFrame) {
let deleteLabel
let deleteTag

if (siteUtil.isBookmark(siteDetail)) {
if (siteUtil.isBookmark(siteDetail) && activeFrame) {
deleteLabel = 'deleteBookmark'
deleteTag = siteTags.BOOKMARK
} else if (siteUtil.isFolder(siteDetail)) {
Expand Down Expand Up @@ -969,7 +969,8 @@ function onHamburgerMenu (location, e) {

function onMainContextMenu (nodeProps, frame, contextMenuType) {
if (contextMenuType === 'bookmark' || contextMenuType === 'bookmark-folder') {
onSiteDetailContextMenu(Immutable.fromJS(nodeProps), Immutable.fromJS({ location: '', title: '', partitionNumber: frame.get('partitionNumber') }))
const activeFrame = Immutable.fromJS({ location: '', title: '', partitionNumber: frame.get('partitionNumber') })
onSiteDetailContextMenu(Immutable.fromJS(nodeProps), activeFrame)
} else if (contextMenuType === 'history') {
onSiteDetailContextMenu(Immutable.fromJS(nodeProps))
} else if (contextMenuType === 'download') {
Expand Down
13 changes: 8 additions & 5 deletions js/state/siteUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,15 @@ module.exports.removeSite = function (sites, siteDetail, tag) {
return sites
}
const tags = sites.getIn([index, 'tags'])
// If there are no tags and the removeSite call was called without a specific tag
// then remove the entry completely.
if (tags.size === 0 && !tag) {
sites = sites.splice(index, 1)
return sites
}
// If called without tags and entry has no tags, remove the entry
return sites.splice(index, 1)
} else if (tags.size > 0 && !tag) {
// If called without tags BUT entry has tags, null out lastAccessedTime.
// This is a bookmark entry that we want to clear history for (but NOT delete/untag bookmark)
return sites.setIn([index, 'lastAccessedTime'], null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hah I was going to suggest that as the fix, thanks.

}
// Else, remove the specified tag
return sites
.setIn([index, 'parentFolderId'], 0)
.setIn([index, 'tags'], tags.toSet().remove(tag).toList())
Expand Down
29 changes: 21 additions & 8 deletions test/unit/state/siteUtilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,14 +181,27 @@ describe('siteUtil', function () {
const expectedSites = sites.setIn([0, 'parentFolderId'], 0).setIn([0, 'tags'], Immutable.List([]))
assert.deepEqual(processedSites, expectedSites)
})
it('deletes a history entry (no tag specified)', function () {
const siteDetail = {
tags: [],
location: testUrl1
}
const sites = Immutable.fromJS([siteDetail])
const processedSites = siteUtil.removeSite(sites, Immutable.fromJS(siteDetail))
assert.deepEqual(processedSites, Immutable.fromJS([]))
describe('called with tag=null/undefined', function () {
it('deletes a history entry (has no tags)', function () {
const siteDetail = {
tags: [],
location: testUrl1
}
const sites = Immutable.fromJS([siteDetail])
const processedSites = siteUtil.removeSite(sites, Immutable.fromJS(siteDetail))
assert.deepEqual(processedSites, Immutable.fromJS([]))
})
it('nulls out the lastAccessedTime for a bookmarked entry (has tag)', function () {
const siteDetail = {
location: testUrl1,
tags: [siteTags.BOOKMARK],
lastAccessedTime: 123
}
const sites = Immutable.fromJS([siteDetail])
const processedSites = siteUtil.removeSite(sites, Immutable.fromJS(siteDetail))
const expectedSites = sites.setIn([0, 'lastAccessedTime'], null)
assert.deepEqual(processedSites, expectedSites)
})
})
})

Expand Down