Fixes Other bookmarks not being usable

Resolves brave#10157 Resolves brave#4202 Auditors: Test Plan:
syuan100 · Nov 9, 2017 · 393f1d8 · 393f1d8
1 parent 891fc73
commit 393f1d8
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 11 deletions.
diff --git a/app/common/lib/bookmarkFoldersUtil.js b/app/common/lib/bookmarkFoldersUtil.js
@@ -130,7 +130,7 @@ const buildFolder = (folderDetails, folders) => {
     title: folderDetails.get('title'),
     folderId: Number(key),
     key: key.toString(),
-    parentFolderId: ~~folderDetails.get('parentFolderId', 0),
+    parentFolderId: Number(folderDetails.get('parentFolderId', 0)),
     partitionNumber: Number(folderDetails.get('partitionNumber', 0)),
     objectId: folderDetails.get('objectId', null),
     type: siteTags.BOOKMARK_FOLDER,

diff --git a/app/common/lib/bookmarkUtil.js b/app/common/lib/bookmarkUtil.js
@@ -191,7 +191,7 @@ const buildBookmark = (state, bookmarkDetail) => {
   return makeImmutable({
     title: bookmarkDetail.get('title', ''),
     location: bookmarkDetail.get('location'),
-    parentFolderId: ~~bookmarkDetail.get('parentFolderId', 0),
+    parentFolderId: Number(bookmarkDetail.get('parentFolderId', 0)),
     partitionNumber: Number(dataItem.get('partitionNumber', 0)),
     objectId: bookmarkDetail.get('objectId', null),
     favicon: dataItem.get('favicon'),

diff --git a/app/common/state/bookmarkFoldersState.js b/app/common/state/bookmarkFoldersState.js
@@ -65,7 +65,7 @@ const bookmarkFoldersState = {
 
     const newFolder = oldFolder.merge(makeImmutable({
       title: folderDetails.get('title'),
-      parentFolderId: ~~folderDetails.get('parentFolderId', 0)
+      parentFolderId: Number(folderDetails.get('parentFolderId', 0))
     }))
 
     if (oldFolder.get('parentFolderId') !== newFolder.get('parentFolderId')) {
@@ -90,7 +90,7 @@ const bookmarkFoldersState = {
       syncActions.removeSites([folder.toJS()])
     }
 
-    folders.filter(folder => folder.get('parentFolderId') === ~~folderKey)
+    folders.filter(folder => folder.get('parentFolderId') === Number(folderKey))
       .map(folder => {
         state = bookmarksState.removeBookmarksByParentId(state, folder.get('folderId'))
         state = bookmarkFoldersState.removeFolder(state, folder.get('folderId'))
@@ -150,7 +150,7 @@ const bookmarkFoldersState = {
         : destinationItem.get('folderId')
 
       state = bookmarkOrderCache.removeCacheKey(state, folder.get('parentFolderId'), folderKey)
-      folder = folder.set('parentFolderId', ~~parentFolderId)
+      folder = folder.set('parentFolderId', Number(parentFolderId))
       const newKey = bookmarkFoldersUtil.getKey(folder)
       state = state.deleteIn([STATE_SITES.BOOKMARK_FOLDERS, folderKey])
       state = bookmarkOrderCache.addFolderToCache(state, folder.get('parentFolderId'), newKey)

diff --git a/app/common/state/bookmarksState.js b/app/common/state/bookmarksState.js
@@ -148,7 +148,7 @@ const bookmarksState = {
     const removedBookmarks = []
     const bookmarks = bookmarksState.getBookmarks(state)
       .filter(bookmark => {
-        if (bookmark.get('parentFolderId') !== ~~parentFolderId) {
+        if (bookmark.get('parentFolderId') !== Number(parentFolderId)) {
           return true
         }
         if (syncEnabled) {
@@ -205,14 +205,17 @@ const bookmarksState = {
 
     // move bookmark into a new folder
     if (moveIntoParent || destinationItem.get('parentFolderId') !== bookmark.get('parentFolderId')) {
-      const parentFolderId = destinationItem.get('type') === siteTags.BOOKMARK
+      let parentFolderId = destinationItem.get('type') === siteTags.BOOKMARK
         ? destinationItem.get('parentFolderId')
         : destinationItem.get('folderId')
 
+      if (parentFolderId == null) {
+        parentFolderId = destinationKey
+      }
+
       state = bookmarkOrderCache.removeCacheKey(state, bookmark.get('parentFolderId'), bookmarkKey)
       state = bookmarkLocationCache.removeCacheKey(state, bookmark.get('location'), bookmarkKey)
-
-      bookmark = bookmark.set('parentFolderId', ~~parentFolderId)
+      bookmark = bookmark.set('parentFolderId', Number(parentFolderId))
       const newKey = bookmarkUtil.getKey(bookmark)
 
       state = state.deleteIn([STATE_SITES.BOOKMARKS, bookmarkKey])

diff --git a/app/renderer/components/bookmarks/addEditBookmarkFolderForm.js b/app/renderer/components/bookmarks/addEditBookmarkFolderForm.js
@@ -89,7 +89,7 @@ class AddEditBookmarkFolderForm extends React.Component {
 
   onParentFolderChange (e) {
     this.setState({
-      parentFolderId: ~~e.target.value
+      parentFolderId: Number(e.target.value)
     })
   }
 
@@ -185,6 +185,7 @@ class AddEditBookmarkFolderForm extends React.Component {
               defaultValue={this.state.parentFolderId}
               onChange={this.onParentFolderChange} >
               <option value='0' data-l10n-id='bookmarksToolbar' />
+              <option value='-1' data-l10n-id='otherBookmarks' />
               {
                 this.props.folders.map((folder) => <option value={folder.folderId}>{folder.label}</option>)
               }

diff --git a/app/renderer/components/bookmarks/addEditBookmarkForm.js b/app/renderer/components/bookmarks/addEditBookmarkForm.js
@@ -100,7 +100,7 @@ class AddEditBookmarkForm extends React.Component {
 
   onParentFolderChange (e) {
     this.setState({
-      parentFolderId: ~~e.target.value
+      parentFolderId: Number(e.target.value)
     })
   }
 
@@ -221,6 +221,7 @@ class AddEditBookmarkForm extends React.Component {
               defaultValue={this.state.parentFolderId}
               onChange={this.onParentFolderChange} >
               <option value='0' data-l10n-id='bookmarksToolbar' />
+              <option value='-1' data-l10n-id='otherBookmarks' />
               {
                 this.props.folders.map((folder) => <option value={folder.folderId}>{folder.label}</option>)
               }