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

Commit

Permalink
Fix allow adding bookmark folders
Browse files Browse the repository at this point in the history
Auditors: @darkdh

Fix #7229

I decided to stick with needing a fodlerId to avoid future bugs with infinite recursion
  • Loading branch information
bbondy committed Feb 14, 2017
1 parent bcbafec commit 27f447f
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 5 deletions.
4 changes: 3 additions & 1 deletion app/renderer/components/addEditBookmarkHanger.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,9 @@ class AddEditBookmarkHanger extends ImmutableComponent {
: 'bookmarkAdded'
}
get isFolder () {
return siteUtil.isFolder(this.props.currentDetail)
// Fake a folderId property so that the bookmark is considered a bookmark folder.
// This is ImmutableJS so it doesn't actually set a value, it just returns a new one.
return siteUtil.isFolder(this.props.currentDetail.set('folderId', 0))
}
setDefaultFocus () {
this.bookmarkName.select()
Expand Down
2 changes: 1 addition & 1 deletion js/state/siteUtil.js
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ module.exports.isHistoryEntry = function (siteDetail) {
if (siteDetail.get('location').startsWith('about:')) {
return false
}
return !!siteDetail.get('lastAccessedTime') && !module.exports.isFolder(siteDetail)
return !!siteDetail.get('lastAccessedTime') && !isBookmarkFolder(siteDetail.get('tags'))
}
return false
}
Expand Down
14 changes: 11 additions & 3 deletions test/unit/state/siteUtilTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -954,7 +954,8 @@ describe('siteUtil', function () {
tags: [siteTags.BOOKMARK]
})
const siteDetail2 = Immutable.fromJS({
tags: [siteTags.BOOKMARK_FOLDER]
tags: [siteTags.BOOKMARK_FOLDER],
folderId: 1
})
assert.equal(siteUtil.isEquivalent(siteDetail1, siteDetail2), false)
})
Expand Down Expand Up @@ -996,12 +997,19 @@ describe('siteUtil', function () {
})

describe('isFolder', function () {
it('returns true if the input is a siteDetail and has a `BOOKMARK_FOLDER` tag', function () {
it('returns true if the input is a siteDetail and has a `BOOKMARK_FOLDER` tag and a folder ID', function () {
const siteDetail = Immutable.fromJS({
tags: [siteTags.BOOKMARK_FOLDER]
tags: [siteTags.BOOKMARK_FOLDER],
folderId: 1
})
assert.equal(siteUtil.isFolder(siteDetail), true)
})
it('returns false if the input does not have a folderId', function () {
const siteDetail = Immutable.fromJS({
tags: [siteTags.BOOKMARK_FOLDER]
})
assert.equal(siteUtil.isFolder(siteDetail), false)
})
it('returns false if the input does not have a `BOOKMARK_FOLDER` tag', function () {
const siteDetail = Immutable.fromJS({
tags: [siteTags.BOOKMARK]
Expand Down

1 comment on commit 27f447f

@darkdh
Copy link
Member

@darkdh darkdh commented on 27f447f Feb 14, 2017

Choose a reason for hiding this comment

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

++

Please sign in to comment.