Remove merge into toolbar option from importer

fix #7194

Auditors: @bsclifton, @bbondy

Test Plan:

a. getNextFolderName covered by unittest

b. Import bookmark:
1. Import bookmarks from other browser or html
2. There will always a folder contains the imported bookmarks
3. If the folder name already exist,
  it will be "Imported from XXX (1)", "Imported from XXX (2)" ... etc

app/extensions/brave/locales/en-US/app.properties

@@ -242,7 +242,6 @@ importDataCloseBrowserWarning=Please make sure the selected browser is closed be
 importSuccess=Your data has been imported to Brave successfully.
 closeFirefoxWarning=Firefox must be closed during data import. Please close and try again.
 favoritesOrBookmarks=Favorites/Bookmarks
-mergeIntoBookmarksToolbar=Merge Favorites into Bookmarks Toolbar
 cookies=Cookies
 licenseTextOk=Ok
 closeFirefoxWarningOk=Ok

app/importer.js

@@ -21,7 +21,6 @@ const tabMessageBox = require('./browser/tabMessageBox')
 const {makeImmutable} = require('./common/state/immutableUtil')
 const tabState = require('./common/state/tabState')
 
-var isMergeFavorites = false
 var isImportingBookmarks = false
 var hasBookmarks
 var importedSites
@@ -31,9 +30,6 @@ exports.init = () => {
 }
 
 exports.importData = (selected) => {
-  if (selected.get('mergeFavorites')) {
-    isMergeFavorites = true
-  }
   if (selected.get('favorites')) {
     isImportingBookmarks = true
     const sites = AppStore.getState().get('sites')
@@ -48,9 +44,6 @@ exports.importData = (selected) => {
 
 exports.importHTML = (selected) => {
   isImportingBookmarks = true
-  if (selected.get('mergeFavorites')) {
-    isMergeFavorites = true
-  }
   const sites = AppStore.getState().get('sites')
   hasBookmarks = sites.find(
     (site) => siteUtil.isBookmark(site) || siteUtil.isFolder(site)
@@ -69,7 +62,6 @@ exports.importHTML = (selected) => {
 }
 
 importer.on('update-supported-browsers', (e, detail) => {
-  isMergeFavorites = false
   isImportingBookmarks = false
   if (BrowserWindow.getFocusedWindow()) {
     BrowserWindow.getFocusedWindow().webContents.send(messages.IMPORTER_LIST, detail)
@@ -124,27 +116,15 @@ importer.on('add-bookmarks', (e, bookmarks, topLevelFolder) => {
   let pathMap = {}
   let sites = []
   let topLevelFolderId = 0
-  if (!isMergeFavorites) {
-    topLevelFolderId = nextFolderIdObject.id++
-    sites.push({
-      customTitle: topLevelFolder,
-      folderId: topLevelFolderId,
-      parentFolderId: 0,
-      lastAccessedTime: 0,
-      creationTime: (new Date()).getTime(),
-      tags: [siteTags.BOOKMARK_FOLDER]
-    })
-  } else {
-    // Merge into existing bookmark toolbar
-    pathMap[topLevelFolder] = topLevelFolderId
-    pathMap['Bookmarks Toolbar'] = 0 // Firefox
-    pathMap['Bookmarks Bar'] = 0 // Chrome on mac
-    pathMap['Other Bookmarks'] = -1 // Chrome on mac
-    pathMap['Bookmarks bar'] = 0 // Chrome on win/linux
-    pathMap['Other bookmarks'] = -1 // Chrome on win/linux
-    pathMap['Bookmark Bar'] = 0 // Safari
-    pathMap['Links'] = 0 // Edge, IE
-  }
+  topLevelFolderId = nextFolderIdObject.id++
+  sites.push({
+    customTitle: siteUtil.getNextFolderName(AppStore.getState().get('sites'), topLevelFolder),
+    folderId: topLevelFolderId,
+    parentFolderId: 0,
+    lastAccessedTime: 0,
+    creationTime: (new Date()).getTime(),
+    tags: [siteTags.BOOKMARK_FOLDER]
+  })
   for (let i = 0; i < bookmarks.length; ++i) {
     let path = bookmarks[i].path
     let parentFolderId = getParentFolderId(path, pathMap, sites, topLevelFolderId, nextFolderIdObject)

app/renderer/components/importBrowserDataPanel.js

@@ -27,16 +27,11 @@ class ImportBrowserDataPanel extends ImmutableComponent {
     super()
     this.onToggleHistory = this.onToggleSetting.bind(this, 'history')
     this.onToggleFavorites = this.onToggleSetting.bind(this, 'favorites')
-    this.onToggleMergeFavorites = this.onToggleSetting.bind(this, 'mergeFavorites')
     this.onToggleCookies = this.onToggleSetting.bind(this, 'cookies')
     this.onImport = this.onImport.bind(this)
     this.onChange = this.onChange.bind(this)
   }
   onToggleSetting (setting, e) {
-    if (setting === 'favorites') {
-      this.props.importBrowserDataSelected =
-        this.props.importBrowserDataSelected.set('mergeFavorites', e.target.value)
-    }
     windowActions.setImportBrowserDataSelected(this.props.importBrowserDataSelected.set(setting, e.target.value))
   }
   get browserData () {
@@ -84,15 +79,13 @@ class ImportBrowserDataPanel extends ImmutableComponent {
     this.props.importBrowserDataSelected = this.props.importBrowserDataSelected.set('index', e.target.value)
     this.props.importBrowserDataSelected = this.props.importBrowserDataSelected.set('history', false)
     this.props.importBrowserDataSelected = this.props.importBrowserDataSelected.set('favorites', false)
-    this.props.importBrowserDataSelected = this.props.importBrowserDataSelected.set('mergeFavorites', false)
     this.props.importBrowserDataSelected = this.props.importBrowserDataSelected.set('cookies', false)
     let importBrowserDataSelected = this.props.importBrowserDataSelected
     if (this.supportHistory) {
       importBrowserDataSelected = importBrowserDataSelected.set('history', true)
     }
     if (this.supportFavorites) {
       importBrowserDataSelected = importBrowserDataSelected.set('favorites', true)
-      importBrowserDataSelected = importBrowserDataSelected.set('mergeFavorites', true)
     }
     if (this.supportCookies) {
       importBrowserDataSelected = importBrowserDataSelected.set('cookies', true)
@@ -108,7 +101,6 @@ class ImportBrowserDataPanel extends ImmutableComponent {
       }
       if (this.supportFavorites) {
         this.props.importBrowserDataSelected = this.props.importBrowserDataSelected.set('favorites', true)
-        this.props.importBrowserDataSelected = this.props.importBrowserDataSelected.set('mergeFavorites', true)
       }
       if (this.supportCookies) {
         this.props.importBrowserDataSelected = this.props.importBrowserDataSelected.set('cookies', true)
@@ -149,14 +141,6 @@ class ImportBrowserDataPanel extends ImmutableComponent {
             onClick={this.onToggleFavorites}
             disabled={!this.supportFavorites}
           />
-          <div className={css(styles.subSectionMargin)} data-test-id='importBrowserSubDataOptions'>
-            <SwitchControl
-              rightl10nId='mergeIntoBookmarksToolbar'
-              checkedOn={this.props.importBrowserDataSelected.get('mergeFavorites')}
-              onClick={this.onToggleMergeFavorites}
-              disabled={!this.props.importBrowserDataSelected.get('favorites')}
-            />
-          </div>
           <SwitchControl
             rightl10nId='cookies'
             checkedOn={this.props.importBrowserDataSelected.get('cookies')}
@@ -182,9 +166,6 @@ class ImportBrowserDataPanel extends ImmutableComponent {
 const styles = StyleSheet.create({
   dropdownWrapper: {
     marginBottom: `calc(${globalStyles.spacing.dialogInsideMargin} / 2)`
-  },
-  subSectionMargin: {
-    marginLeft: globalStyles.spacing.dialogInsideMargin
   }
 })
 

docs/state.md

@@ -505,7 +505,6 @@ WindowStore
     favorites: boolean,
     history: boolean,
     index: string,
-    mergeFavorites: boolean,
     type: number
   },
   lastAppVersion: string, // version of the last file that was saved

js/state/siteUtil.js

@@ -132,6 +132,26 @@ module.exports.getNextFolderId = (sites) => {
   return (maxIdItem ? (maxIdItem.get('folderId') || 0) : 0) + 1
 }
 
+module.exports.getNextFolderName = (sites, name) => {
+  if (!sites) {
+    return name
+  }
+  const site = sites.find((site) =>
+    isBookmarkFolder(site.get('tags')) &&
+    site.get('customTitle') === name
+  )
+  if (!site) {
+    return name
+  }
+  const filenameFormat = /(.*) \((\d+)\)/
+  let result = filenameFormat.exec(name)
+  if (!result) {
+    return module.exports.getNextFolderName(sites, name + ' (1)')
+  }
+  const nextNum = parseInt(result[2]) + 1
+  return module.exports.getNextFolderName(sites, result[1] + ' (' + nextNum + ')')
+}
+
 const mergeSiteLastAccessedTime = (oldSiteDetail, newSiteDetail, tag) => {
   const newTime = newSiteDetail && newSiteDetail.get('lastAccessedTime')
   const oldTime = oldSiteDetail && oldSiteDetail.get('lastAccessedTime')

test/unit/state/siteUtilTest.js

@@ -169,6 +169,62 @@ describe('siteUtil', function () {
     })
   })
 
+  describe('getNextFolderName', function () {
+    it('returns original name when no duplicate', function () {
+      const sites = Immutable.fromJS({
+        'key1': {
+          folderId: 0,
+          customTitle: 'abc',
+          tags: [siteTags.BOOKMARK_FOLDER]
+        }
+      })
+      assert.equal(siteUtil.getNextFolderName(sites, 'def'), 'def')
+    })
+    it('returns original name if sites is falsey', function () {
+      assert.equal(siteUtil.getNextFolderName(null, 'abc'), 'abc')
+    })
+    it('returns first duplicate name', function () {
+      const sites = Immutable.fromJS({
+        'key1': {
+          folderId: 0,
+          customTitle: 'abc',
+          tags: [siteTags.BOOKMARK_FOLDER]
+        }
+      })
+      assert.equal(siteUtil.getNextFolderName(sites, 'abc'), 'abc (1)')
+    })
+    it('returns non first duplicate name', function () {
+      const sites = Immutable.fromJS({
+        'key1': {
+          folderId: 0,
+          customTitle: 'abc',
+          tags: [siteTags.BOOKMARK_FOLDER]
+        },
+        'key2': {
+          folderId: 1,
+          customTitle: 'abc (1)',
+          tags: [siteTags.BOOKMARK_FOLDER]
+        }
+      })
+      assert.equal(siteUtil.getNextFolderName(sites, 'abc'), 'abc (2)')
+    })
+    it('returns non first duplicate name from duplicate name', function () {
+      const sites = Immutable.fromJS({
+        'key1': {
+          folderId: 0,
+          customTitle: 'abc',
+          tags: [siteTags.BOOKMARK_FOLDER]
+        },
+        'key2': {
+          folderId: 1,
+          customTitle: 'abc (1)',
+          tags: [siteTags.BOOKMARK_FOLDER]
+        }
+      })
+      assert.equal(siteUtil.getNextFolderName(sites, 'abc (1)'), 'abc (2)')
+    })
+  })
+
   describe('addSite', function () {
     it('gets the tag from siteDetail if not provided', function () {
       const processedSites = siteUtil.addSite(emptySites, bookmarkAllFields)