Bug 370013 Bookmarks toolbar folder should be a child of the bookmark menu folder, Bug 370020 No way to set an arbitrary folder as the bookmarks toolbar folder (r=mano)

2007-03-07 21:22:54 +00:00 · 2007-03-07 21:22:54 +00:00 · 982652e67c
--- a/browser/base/content/browser.js
+++ b/browser/base/content/browser.js
@ -308,6 +308,19 @@ function BookmarkThisTab()
 #endif
 }

+#ifdef MOZ_PLACES_BOOKMARKS
+/**
+ * Initialize the bookmarks toolbar
+ */
+function initBookmarksToolbar() {
+  var bt = document.getElementById("bookmarksBarContent");
+  if (!bt)
+    return;
+  bt.place =
+    PlacesUtils.getQueryStringForFolder(PlacesUtils.bookmarks.toolbarFolder);
+}
+#endif
+
 const gSessionHistoryObserver = {
  observe: function(subject, topic, data)
  {
@ -1005,6 +1018,8 @@ function delayedStartup()
          .controllers.appendController(BookmarksMenuController);
 #else
  PlacesMenuDNDController.init();
+
+  initBookmarksToolbar();
 #endif

  // called when we go into full screen, even if it is
--- a/browser/base/content/browser.xul
+++ b/browser/base/content/browser.xul
@ -324,7 +324,6 @@
 #ifdef MOZ_PLACES_BOOKMARKS
      <toolbaritem flex="1" id="personal-bookmarks" title="&bookmarksItem.title;">
         <hbox id="bookmarksBarContent" flex="1" type="places"
-               place="place:&amp;folder=3&amp;group=3&amp;expandQueries=1"
               context="placesContext" asyncinit="true" 
               onclick="BookmarksEventHandler.onClick(event);"
               oncommand="BookmarksEventHandler.onCommand(event);"
--- a/browser/components/migration/src/nsIEProfileMigrator.cpp
+++ b/browser/components/migration/src/nsIEProfileMigrator.cpp
@ -1478,7 +1478,7 @@ nsIEProfileMigrator::ParseFavoritesFolder(nsIFile* aDirectory,
 #endif
      if (bookmarkName.Equals(aPersonalToolbarFolderName)) {
 #ifdef MOZ_PLACES_BOOKMARKS
-        aBookmarksService->GetToolbarRoot(&folder);
+        aBookmarksService->GetToolbarFolder(&folder);
        // If we're here, it means the user's doing a _replace_ import which means
        // clear out the content of this folder, and replace it with the new content
        aBookmarksService->RemoveFolderChildren(folder);
--- a/browser/components/migration/src/nsOperaProfileMigrator.cpp
+++ b/browser/components/migration/src/nsOperaProfileMigrator.cpp
@ -1096,7 +1096,7 @@ nsOperaProfileMigrator::CopyBookmarks(PRBool aReplace)

 #ifdef MOZ_PLACES_BOOKMARKS
  PRInt64 toolbar;
-  rv = bms->GetToolbarRoot(&toolbar);
+  rv = bms->GetToolbarFolder(&toolbar);
  NS_ENSURE_SUCCESS(rv, rv);
 #else
  nsCOMPtr<nsIRDFResource> toolbar;
--- a/browser/components/migration/src/nsSafariProfileMigrator.cpp
+++ b/browser/components/migration/src/nsSafariProfileMigrator.cpp
@ -1021,7 +1021,7 @@ nsSafariProfileMigrator::ParseBookmarksFolder(CFArrayRef aChildren,
      if (title.EqualsLiteral("BookmarksBar") && aIsAtRootLevel) {
 #ifdef MOZ_PLACES_BOOKMARKS
        PRInt64 toolbarFolder;
-        aBookmarksService->GetToolbarRoot(&toolbarFolder);
+        aBookmarksService->GetToolbarFolder(&toolbarFolder);
 #else
        nsCOMPtr<nsIRDFResource> toolbarFolder;
        aBookmarksService->GetBookmarksToolbarFolder(getter_AddRefs(toolbarFolder));
--- a/browser/components/places/content/controller.js
+++ b/browser/components/places/content/controller.js
@ -278,6 +278,15 @@ PlacesController.prototype = {
 #endif
      }
      return false;
+    case "placesCmd_setAsBookmarksToolbarFolder":
+      if (this._view.hasSingleSelection) {
+        var selectedNode = this._view.selectedNode;
+        if (PlacesUtils.nodeIsFolder(selectedNode) &&
+            selectedNode.folderId != PlacesUtils.bookmarks.toolbarFolder) {
+          return true;
+        }
+      }
+      return false;
 #endif
    default:
      return false;
@ -361,6 +370,9 @@ PlacesController.prototype = {
    case "placesCmd_reload":
      this.reloadSelectedLivemarks();
      break;
+    case "placesCmd_setAsBookmarksToolbarFolder":
+      this.setBookmarksToolbarFolder();
+      break;
 #endif
    }
  },
@ -1109,6 +1121,18 @@ PlacesController.prototype = {
                      this._view.getSelectionNodes(), PlacesUtils.tm);
  },

+  /**
+   * Makes the selected node the bookmarks toolbar folder.
+   */
+  setBookmarksToolbarFolder: function PC_setBookmarksToolbarFolder() {
+    if (!this._view.hasSingleSelection)
+      return false;
+    var selectedNode = this._view.selectedNode;
+    var txn = new PlacesSetBookmarksToolbarTransaction(selectedNode.folderId);
+    PlacesUtils.tm.doTransaction(txn);
+  },
+
+
  /**
   * Creates a set of transactions for the removal of a range of items. A range is 
   * an array of adjacent nodes in a view.
@ -2169,6 +2193,26 @@ PlacesEditBookmarkMicrosummaryTransaction.prototype = {
  }
 };

+/**
+ * Set the bookmarks toolbar folder.
+ */
+function PlacesSetBookmarksToolbarTransaction(aFolderId) {
+  this._folderId = aFolderId;
+  this._oldFolderId = this.utils.toolbarFolder;
+  this.redoTransaction = this.doTransaction;
+}
+PlacesSetBookmarksToolbarTransaction.prototype = {
+  __proto__: PlacesBaseTransaction.prototype,
+  
+  doTransaction: function PSBTT_doTransaction() {
+    this.utils.bookmarks.toolbarFolder = this._folderId;
+  },
+
+  undoTransaction: function PSBTT_undoTransaction() {
+    this.utils.bookmarks.toolbarFolder = this._oldFolderId;
+  }
+};
+
 function goUpdatePlacesCommands() {
  goUpdateCommand("placesCmd_open");
  goUpdateCommand("placesCmd_open:window");
@ -2181,6 +2225,7 @@ function goUpdatePlacesCommands() {
  goUpdateCommand("placesCmd_new:separator");
  goUpdateCommand("placesCmd_show:info");
  goUpdateCommand("placesCmd_moveBookmarks");
+  goUpdateCommand("placesCmd_setAsBookmarksToolbarFolder");
  goUpdateCommand("placesCmd_reload");
  // XXXmano todo: sort commands handling
 #endif
--- a/browser/components/places/content/places.xul
+++ b/browser/components/places/content/places.xul
@ -213,6 +213,9 @@
          <menuitem command="placesCmd_moveBookmarks"
                    label="&cmd.moveBookmarks.menuLabel;"
                    accesskey="&cmd.moveBookmarks.menuAccesskey;"/>
+          <menuitem command="placesCmd_setAsBookmarksToolbarFolder"
+                    label="&cmd.personalToolbarFolder.menuLabel;"
+                    accesskey="&cmd.personalToolbarFolder.menuAccesskey;"/>
          <menuseparator/>
          <menuitem id="editReload"
                    command="placesCmd_reload"
--- a/browser/components/places/content/placesOverlay.xul
+++ b/browser/components/places/content/placesOverlay.xul
@ -93,6 +93,8 @@

    <command id="placesCmd_moveBookmarks"
             oncommand="goDoCommand('placesCmd_moveBookmarks');"/>
+    <command id="placesCmd_setAsBookmarksToolbarFolder"
+             oncommand="goDoCommand('placesCmd_setAsBookmarksToolbarFolder');"/>
 #endif
  </commandset>

--- a/browser/components/places/content/toolbar.xml
+++ b/browser/components/places/content/toolbar.xml
@ -360,11 +360,17 @@
          history.queryStringToQueries(val, queries, { }, options);
          if (!queries.value.length) 
            queries.value = [history.getNewQuery()];
-          this._result = 
-            history.executeQueries(queries.value, queries.value.length,
-                                   options.value);
-          this._result.root.containerOpen = true;
-          this._rebuild();
+          try {
+            this._result = 
+              history.executeQueries(queries.value, queries.value.length,
+                                     options.value);
+            this._result.root.containerOpen = true;
+            this._rebuild();
+          }
+          catch(ex) {
+            // Invalid query, or had no results.
+            // This is valid, eg: user deletes their bookmarks toolbar folder. 
+          }
          return val;
        ]]></setter>
      </property>
@ -479,34 +485,38 @@
            this.doRebuild();
        },
        onItemAdded: function TB_O_onItemAdded(id, bookmark, folder, index) {
-          if (folder == PlacesUtils.bookmarks.toolbarRoot) {
+          if (folder == PlacesUtils.bookmarks.toolbarFolder) {
            this._self._currentURIs[bookmark.spec] = true;
            this._numBatches ? this._batchedOperation = true : this.doRebuild();
          }
        },
        onItemRemoved: function TB_O_onItemRemoved(id, bookmark, folder, index) {
-          if (folder == PlacesUtils.bookmarks.toolbarRoot) {
+          if (folder == PlacesUtils.bookmarks.toolbarFolder) {
            delete this._self._currentURIs[bookmark.spec];
            this._numBatches ? this._batchedOperation = true : this.doRebuild();
          }
        },
        onItemChanged: function TB_O_onItemChanged(id, bookmark, property, value) {
+          if (property == "became_toolbar_folder") {
+            var place = PlacesUtils.getQueryStringForFolder(id);
+            this._self.place = place || null;
+          }
          this._numBatches ? this._batchedOperation = true : this.doRebuild();
        },
        onItemVisited: function TB_0_onItemVisited(bookmark, visitId, time) {
          //this._self._init();
        },
        onFolderAdded: function TB_O_onFolderAdded(folder, parent, index) {
-          if (parent == PlacesUtils.bookmarks.toolbarRoot)
+          if (parent == PlacesUtils.bookmarks.toolbarFolder)
            this._numBatches ? this._batchedOperation = true : this.doRebuild();
        },
        onFolderRemoved: function TB_O_onFolderRemoved(folder, parent, index) {
-          if (parent == PlacesUtils.bookmarks.toolbarRoot)
+          if (parent == PlacesUtils.bookmarks.toolbarFolder)
            this._numBatches ? this._batchedOperation = true : this.doRebuild();
        },
        onFolderMoved: function TB_O_onFolderMoved(folder, oldParent, oldIndex, newParent, newIndex) {
-          if (oldParent == PlacesUtils.bookmarks.toolbarRoot ||
-              newParent == PlacesUtils.bookmarks.toolbarRoot)
+          if (oldParent == PlacesUtils.bookmarks.toolbarFolder ||
+              newParent == PlacesUtils.bookmarks.toolbarFolder)
            this._numBatches ? this._batchedOperation = true : this.doRebuild();
        },
        onFolderChanged: function TB_O_onFolderChanged(folder, property) {
@ -519,11 +529,11 @@
          setTimeout(hitch(this._self, this._self._rebuild), 1);
        },
        onSeparatorAdded: function TB_O_onSeparatorAdded(parent, index) {
-          if (parent == PlacesUtils.bookmarks.toolbarRoot)
+          if (parent == PlacesUtils.bookmarks.toolbarFolder)
            this._numBatches ? this._batchedOperation = true : this.doRebuild();
        },
        onSeparatorRemoved: function TB_O_onSeparatorRemoved(parent, index) {
-          if (parent == PlacesUtils.bookmarks.toolbarRoot)
+          if (parent == PlacesUtils.bookmarks.toolbarFolder)
            this._numBatches ? this._batchedOperation = true : this.doRebuild();
        }
      })]]></field>
--- a/browser/components/places/content/utils.js
+++ b/browser/components/places/content/utils.js
@ -969,5 +969,18 @@ var PlacesUtils = {
          break;
      }
    });
+  },
+
+  /**
+   * Helper for getting a serialized Places query for a particular folder.
+   * @param aFolderId The folder id to get a query for.
+   * @return string serialized place URI
+   */
+  getQueryStringForFolder: function PU_getQueryStringForFolder(aFolderId) {
+    var options = this.history.getNewQueryOptions();
+    options.setGroupingMode([Ci.nsINavHistoryQueryOptions.GROUP_BY_FOLDER], 1);
+    var query = this.history.getNewQuery();
+    query.setFolders([aFolderId], 1);
+    return this.history.queriesToQueryString([query], 1, options);
  }
 };
--- a/browser/components/places/tests/bookmarks/test_bookmarks.js
+++ b/browser/components/places/tests/bookmarks/test_bookmarks.js
@ -125,23 +125,31 @@ bmsvc.addObserver(observer, false);
 var root = bmsvc.bookmarksRoot;

 // index at which items should begin
-var bmStartIndex = 3;
+var bmStartIndex = 4;

 // main
 function run_test() {
  // test roots
  do_check_true(bmsvc.placesRoot > 0);
  do_check_true(bmsvc.bookmarksRoot > 0);
-  do_check_true(bmsvc.toolbarRoot > 0);
+  do_check_true(bmsvc.toolbarFolder > 0);

-  // insert item
-  var newId = bmsvc.insertItem(root, uri("http://google.com/"), bmsvc.DEFAULT_INDEX);
+  // create a folder to hold all the tests
+  // this makes the tests more tolerant of changes to default_places.html
+  var testRoot = bmsvc.createFolder(root, "places bookmarks xpcshell tests", bmsvc.DEFAULT_INDEX);
+  do_check_eq(observer._folderAdded, testRoot);
+  do_check_eq(observer._folderAddedParent, root);
+  do_check_eq(observer._folderAddedIndex, bmStartIndex);
+  var testStartIndex = 0;
+
+  // insert a bookmark 
+  var newId = bmsvc.insertItem(testRoot, uri("http://google.com/"), bmsvc.DEFAULT_INDEX);
  do_check_eq(observer._itemAddedId, newId);
  do_check_eq(observer._itemAdded.spec, "http://google.com/");
-  do_check_eq(observer._itemAddedFolder, root);
-  do_check_eq(observer._itemAddedIndex, bmStartIndex);
+  do_check_eq(observer._itemAddedFolder, testRoot);
+  do_check_eq(observer._itemAddedIndex, testStartIndex);

-  // set item title
+  // set bookmark title
  bmsvc.setItemTitle(newId, "Google");
  do_check_eq(observer._itemChangedId, newId);
  do_check_eq(observer._itemChanged.spec, "http://google.com/");
@ -158,20 +166,22 @@ function run_test() {
    do_throw("getItemTitle accepted bad input");
  } catch(ex) {}

-  // test folder that the bm is in
+  // get the folder that the bookmark is in
  var folderId = bmsvc.getFolderIdForItem(newId);
-  do_check_eq(folderId, root);
+  do_check_eq(folderId, testRoot);

-  // create folder
-  var workFolder = bmsvc.createFolder(root, "Work", 3);
+  // create a folder at a specific index
+  var workFolder = bmsvc.createFolder(testRoot, "Work", 0);
  do_check_eq(observer._folderAdded, workFolder);
-  do_check_eq(observer._folderAddedParent, root);
-  do_check_eq(observer._folderAddedIndex, 3);
+  do_check_eq(observer._folderAddedParent, testRoot);
+  do_check_eq(observer._folderAddedIndex, 0);
+  
+  //XXX - test creating a folder at an invalid index

  //XXX - test setFolderTitle
  //XXX - test getFolderTitle

-  // insert item
+  // add item into subfolder, specifying index
  var newId2 = bmsvc.insertItem(workFolder, uri("http://developer.mozilla.org/"), 0);
  do_check_eq(observer._itemAddedId, newId2);
  do_check_eq(observer._itemAdded.spec, "http://developer.mozilla.org/");
@ -183,7 +193,7 @@ function run_test() {
  do_check_eq(observer._itemChanged.spec, "http://developer.mozilla.org/");
  do_check_eq(observer._itemChangedProperty, "title");

-  // insert item
+  // insert item into subfolder
  var newId3 = bmsvc.insertItem(workFolder, uri("http://msdn.microsoft.com/"), bmsvc.DEFAULT_INDEX);
  do_check_eq(observer._itemAddedId, newId3);
  do_check_eq(observer._itemAdded.spec, "http://msdn.microsoft.com/");
@ -202,7 +212,7 @@ function run_test() {
  do_check_eq(observer._itemRemovedFolder, workFolder);
  do_check_eq(observer._itemRemovedIndex, 0);

-  // insert item
+  // insert item into subfolder
  var newId4 = bmsvc.insertItem(workFolder, uri("http://developer.mozilla.org/"), bmsvc.DEFAULT_INDEX);
  do_check_eq(observer._itemAddedId, newId4);
  do_check_eq(observer._itemAdded.spec, "http://developer.mozilla.org/");
@ -210,13 +220,13 @@ function run_test() {
  do_check_eq(observer._itemAddedIndex, 1);
  
  // create folder
-  var homeFolder = bmsvc.createFolder(root, "Home", bmsvc.DEFAULT_INDEX);
+  var homeFolder = bmsvc.createFolder(testRoot, "Home", bmsvc.DEFAULT_INDEX);
  do_check_eq(observer._folderAdded, homeFolder);
-  do_check_eq(observer._folderAddedParent, root);
-  do_check_eq(observer._folderAddedIndex, 5);
+  do_check_eq(observer._folderAddedParent, testRoot);
+  do_check_eq(observer._folderAddedIndex, 2);

  // insert item
-  var newId5 = bmsvc.insertItem(homeFolder, uri("http://espn.com/"), 0);
+  var newId5 = bmsvc.insertItem(homeFolder, uri("http://espn.com/"), bmsvc.DEFAULT_INDEX);
  do_check_eq(observer._itemAddedId, newId5);
  do_check_eq(observer._itemAdded.spec, "http://espn.com/");
  do_check_eq(observer._itemAddedFolder, homeFolder);
@ -227,50 +237,52 @@ function run_test() {
  do_check_eq(observer._itemChanged.spec, "http://espn.com/");
  do_check_eq(observer._itemChangedProperty, "title");

-  // insert item
-  var newId6 = bmsvc.insertItem(root, uri("place:domain=google.com&group=1"), bmsvc.DEFAULT_INDEX);
+  // insert query item
+  var newId6 = bmsvc.insertItem(testRoot, uri("place:domain=google.com&group=1"), bmsvc.DEFAULT_INDEX);
  do_check_eq(observer._itemAdded.spec, "place:domain=google.com&group=1");
-  do_check_eq(observer._itemAddedFolder, root);
-  do_check_eq(observer._itemAddedIndex, 6);
+  do_check_eq(observer._itemAddedFolder, testRoot);
+  do_check_eq(observer._itemAddedIndex, 3);

  // change item
  bmsvc.setItemTitle(newId6, "Google Sites");
  do_check_eq(observer._itemChanged.spec, "place:domain=google.com&group=1");
  do_check_eq(observer._itemChangedProperty, "title");

-  // move folder
-  bmsvc.moveFolder(workFolder, root, bmsvc.DEFAULT_INDEX);
+  // move folder - move the "work" folder into the "home" folder
+  bmsvc.moveFolder(workFolder, homeFolder, bmsvc.DEFAULT_INDEX);
  do_check_eq(observer._folderMoved, workFolder);
-  do_check_eq(observer._folderMovedOldParent, root);
-  do_check_eq(observer._folderMovedOldIndex, 3);
-  do_check_eq(observer._folderMovedNewParent, root);
-  do_check_eq(observer._folderMovedNewIndex, 6);
+  do_check_eq(observer._folderMovedOldParent, testRoot);
+  do_check_eq(observer._folderMovedOldIndex, 0);
+  do_check_eq(observer._folderMovedNewParent, homeFolder);
+  do_check_eq(observer._folderMovedNewIndex, 1);
+  // try to get index of the folder from it's ex-parent
+  do_check_eq(bmsvc.indexOfFolder(testRoot, workFolder), -1);

  // Test expected failure of moving a folder to be its own parent
  try {
    bmsvc.moveFolder(workFolder, workFolder, bmsvc.DEFAULT_INDEX);
    do_throw("moveFolder() allowed moving a folder to be it's own parent.");
  } catch (e) {}
-  do_check_eq(bmsvc.indexOfFolder(root, workFolder), 6);
+  do_check_eq(bmsvc.indexOfFolder(homeFolder, workFolder), 1);

  // test insertSeparator
  // XXX - this should also query bookmarks for the folder children
  // and then test the node type at our index
  try {
-    bmsvc.insertSeparator(root, 1);
-    bmsvc.removeChildAt(root, 1);
+    bmsvc.insertSeparator(testRoot, 1);
+    bmsvc.removeChildAt(testRoot, 1);
  } catch(ex) {
    do_throw("insertSeparator: " + ex);
  }

  // test indexOfFolder
-  var tmpFolder = bmsvc.createFolder(root, "tmp", 2);
-  do_check_eq(bmsvc.indexOfFolder(root, tmpFolder), 2);
+  var tmpFolder = bmsvc.createFolder(testRoot, "tmp", 2);
+  do_check_eq(bmsvc.indexOfFolder(testRoot, tmpFolder), 2);

  // test setKeywordForURI
-  var kwTestItemId = bmsvc.insertItem(root, uri("http://keywordtest.com"), bmsvc.DEFAULT_INDEX);
+  var kwTestItemId = bmsvc.insertItem(testRoot, uri("http://keywordtest.com"), bmsvc.DEFAULT_INDEX);
  try {
-    var res = bmsvc.setKeywordForBookmark(kwTestItemId, "bar");
+    bmsvc.setKeywordForBookmark(kwTestItemId, "bar");
  } catch(ex) {
    do_throw("setKeywordForBookmark: " + ex);
  }
@ -288,13 +300,13 @@ function run_test() {
  do_check_eq("http://keywordtest.com/", u.spec);

  // test getBookmarkIdsForURI
-  var newId8 = bmsvc.insertItem(root, uri("http://foo8.com/"), 2);
+  var newId8 = bmsvc.insertItem(testRoot, uri("http://foo8.com/"), bmsvc.DEFAULT_INDEX);
  var b = bmsvc.getBookmarkIdsForURI(uri("http://foo8.com/"), {});
  do_check_eq(b[0], newId8);

  // test removeFolderChildren
  // 1) add/remove each child type (bookmark, separator, folder)
-  var tmpFolder = bmsvc.createFolder(root, "removeFolderChildren", bmsvc.DEFAULT_INDEX);
+  var tmpFolder = bmsvc.createFolder(testRoot, "removeFolderChildren", bmsvc.DEFAULT_INDEX);
  bmsvc.insertItem(tmpFolder, uri("http://foo9.com/"), bmsvc.DEFAULT_INDEX);
  bmsvc.createFolder(tmpFolder, "subfolder", bmsvc.DEFAULT_INDEX);
  bmsvc.insertSeparator(tmpFolder, bmsvc.DEFAULT_INDEX);
@ -323,7 +335,7 @@ function run_test() {
  } catch(ex) { do_throw("removeFolderChildren(): " + ex); }

  // test getItemURI
-  var newId9 = bmsvc.insertItem(root, uri("http://foo9.com/"), bmsvc.DEFAULT_INDEX);
+  var newId9 = bmsvc.insertItem(testRoot, uri("http://foo9.com/"), bmsvc.DEFAULT_INDEX);
  var placeURI = bmsvc.getItemURI(newId9);
  do_check_eq(placeURI.spec, "place:moz_bookmarks.id=" + newId9 + "&group=3");

@ -335,7 +347,7 @@ function run_test() {
    options.maxResults = 1;
    options.setGroupingMode([Ci.nsINavHistoryQueryOptions.GROUP_BY_FOLDER], 1);
    var query = histsvc.getNewQuery();
-    query.setFolders([root], 1);
+    query.setFolders([testRoot], 1);
    var result = histsvc.executeQuery(query, options);
    var rootNode = result.root;
    rootNode.containerOpen = true;
@ -344,7 +356,7 @@ function run_test() {
      var node = rootNode.getChild(i);
      do_check_true(node.bookmarkId > 0);
    }
-    root.containerOpen = false;
+    testRoot.containerOpen = false;
  }
  catch(ex) {
    do_throw("bookmarks query: " + ex);
@ -356,22 +368,22 @@ function run_test() {
    var mURI = uri("http://multiple.uris.in.query");

    // add 2 bookmarks
-    bmsvc.insertItem(root, mURI, bmsvc.DEFAULT_INDEX);
-    bmsvc.insertItem(root, mURI, bmsvc.DEFAULT_INDEX);
+    bmsvc.insertItem(testRoot, mURI, bmsvc.DEFAULT_INDEX);
+    bmsvc.insertItem(testRoot, mURI, bmsvc.DEFAULT_INDEX);

    // query
    var options = histsvc.getNewQueryOptions();
    options.maxResults = 2;
    options.setGroupingMode([Ci.nsINavHistoryQueryOptions.GROUP_BY_FOLDER], 1);
    var query = histsvc.getNewQuery();
-    query.setFolders([root], 1);
+    query.setFolders([testRoot], 1);
    query.uri = mURI;
    var result = histsvc.executeQuery(query, options);
    var rootNode = result.root;
    rootNode.containerOpen = true;
    var cc = rootNode.childCount;
    do_check_eq(cc, 2);
-    root.containerOpen = false;
+    testRoot.containerOpen = false;
  }
  catch(ex) {
    do_throw("bookmarks query: " + ex);
@ -379,21 +391,32 @@ function run_test() {
  */

  // test change bookmark uri
-  var newId10 = bmsvc.insertItem(root, uri("http://foo10.com/"), bmsvc.DEFAULT_INDEX);
+  var newId10 = bmsvc.insertItem(testRoot, uri("http://foo10.com/"), bmsvc.DEFAULT_INDEX);
  bmsvc.changeBookmarkURI(newId10, uri("http://foo11.com/"));
  do_check_eq(observer._itemChangedId, newId10);
  do_check_eq(observer._itemChanged.spec, "http://foo11.com/");
  do_check_eq(observer._itemChangedProperty, "uri");
  do_check_eq(observer._itemChangedValue, "");

-  var newId11 = bmsvc.insertItem(root, uri("http://foo11.com/"), bmsvc.DEFAULT_INDEX);
+  // test getBookmarkURI
+  var newId11 = bmsvc.insertItem(testRoot, uri("http://foo11.com/"), bmsvc.DEFAULT_INDEX);
  var bmURI = bmsvc.getBookmarkURI(newId11);
  do_check_eq("http://foo11.com/", bmURI.spec);

  // test getItemIndex
-  var newId12 = bmsvc.insertItem(root, uri("http://foo11.com/"), 1);
+  var newId12 = bmsvc.insertItem(testRoot, uri("http://foo11.com/"), 1);
  var bmIndex = bmsvc.getItemIndex(newId12);
  do_check_eq(1, bmIndex);
+
+  // test changing the bookmarks toolbar folder
+  var oldToolbarFolder = bmsvc.toolbarFolder;
+  var newToolbarFolderId = bmsvc.createFolder(testRoot, "new toolbar folder", -1);
+  bmsvc.toolbarFolder = newToolbarFolderId;
+  do_check_eq(bmsvc.toolbarFolder, newToolbarFolderId);
+  do_check_eq(observer._itemChangedId, newToolbarFolderId);
+  do_check_eq(observer._itemChanged.spec, bmsvc.getFolderURI(newToolbarFolderId).spec);
+  do_check_eq(observer._itemChangedProperty, "became_toolbar_folder");
+  do_check_eq(observer._itemChangedValue, "");
 }

 // XXXDietrich - get this section up to date
--- a/browser/locales/en-US/chrome/browser/places/default_places.html
+++ b/browser/locales/en-US/chrome/browser/places/default_places.html
@ -8,13 +8,13 @@
 <DL><p>
    <DT><A HREF="place:&beginTime=-2592000000000&beginTimeRef=1&endTime=7200000000&endTimeRef=2&sort=4&type=1">History</A>
    <DD>Shows all browsing history
-    <DT><H3 BOOKMARKS_MENU="true">Bookmarks Menu</H3>
+    <DT><H3 BOOKMARKS_MENU="true">Bookmarks</H3>
    <DD>Add bookmarks to this folder to see them displayed on the Bookmarks Menu
    <DL><p>
-    </DL><p>
-    <DT><H3 PERSONAL_TOOLBAR_FOLDER="true">Bookmarks Toolbar</H3>
-    <DD>Add bookmarks to this folder to see them displayed on the Bookmarks Toolbar
-    <DL><p>
+      <DT><H3 PERSONAL_TOOLBAR_FOLDER="true">Bookmarks Toolbar Folder</H3>
+      <DD>Add bookmarks to this folder to see them displayed on the Bookmarks Toolbar
+      <DL><p>
+      </DL><p>
    </DL><p>
    <DT><A HREF="place:&annotation=livemark%2FfeedURI" ICON_URI="chrome://browser/skin/places/livemarkItem.png">Subscriptions</A>
    <DD>Shows all Subscribed Feeds
--- a/browser/locales/en-US/chrome/browser/places/places.dtd
+++ b/browser/locales/en-US/chrome/browser/places/places.dtd
@ -190,6 +190,10 @@
         "Move Bookmark(s)...">
 <!ENTITY cmd.moveBookmarks.menuAccesskey
         "M">
+<!ENTITY cmd.personalToolbarFolder.menuLabel
+         "Set as Bookmarks Toolbar Folder">
+<!ENTITY cmd.personalToolbarFolder.menuAccesskey
+         "b">

 <!ENTITY col.title.label
  "Title">
--- a/extensions/metrics/src/nsProfileCollector.cpp
+++ b/extensions/metrics/src/nsProfileCollector.cpp
@ -464,7 +464,7 @@ nsProfileCollector::LogBookmarks(nsIMetricsEventItem *profile)
  LogBookmarkLocation(bookmarksItem, NS_LITERAL_CSTRING("root"),
                      &counter, root, PR_FALSE);

-  rv = bmSvc->GetToolbarRoot(&root);
+  rv = bmSvc->GetToolbarFolder(&root);
  NS_ENSURE_SUCCESS(rv, rv);

  LogBookmarkLocation(bookmarksItem, NS_LITERAL_CSTRING("toolbar"),
--- a/toolkit/components/places/public/nsINavBookmarksService.idl
+++ b/toolkit/components/places/public/nsINavBookmarksService.idl
@ -54,7 +54,7 @@ interface nsITransaction;
 * Observer for bookmark changes.
 */

-[scriptable, uuid(d0f520f0-b08b-11db-abbd-0800200c9a66)]
+[scriptable, uuid(860d786d-9bba-4011-a396-486a87af8f07)]
 interface nsINavBookmarkObserver : nsISupports
 {
  /**
@ -194,11 +194,6 @@ interface nsINavBookmarksService : nsISupports
   */
  readonly attribute PRInt64 bookmarksRoot;

-  /**
-   * The folder ID of the personal toolbar root.
-   */
-  readonly attribute PRInt64 toolbarRoot;
-
  /**
   * The folder ID of the top-level folders that contain the tag "folders".
   *
@ -207,6 +202,11 @@ interface nsINavBookmarksService : nsISupports
   */
  readonly attribute PRInt64 tagRoot;

+  /**
+   * The folder ID of the personal toolbar.
+   */
+  attribute PRInt64 toolbarFolder;
+
  /**
   * This value should be used for APIs that allow passing in an index
   * where an index is not known, or not required to be specified.
--- a/toolkit/components/places/src/nsBookmarksHTML.cpp
+++ b/toolkit/components/places/src/nsBookmarksHTML.cpp
@ -275,11 +275,12 @@ protected:
  nsCOMPtr<nsIAnnotationService> mAnnotationService;
  nsCOMPtr<nsILivemarkService> mLivemarkService;

-  // if set, we will move root items to where we find them. This should be
-  // set when we are loading the default places html file, and should be
-  // unset when doing normal imports so that, for example, the toolbar folder
-  // will be a child of the menu in old bookmarks.html, and we don't want
-  // to reparent it on import.
+  // If set, we will move root items to from their existing position
+  // in the hierarchy, to where we find them in the bookmarks file
+  // being imported. This should be set when we are loading 
+  // the default places html file, and should be unset when doing
+  // normal imports so that root folders will not get moved  when
+  // importing bookmarks.html files.
  PRBool mAllowRootChanges;

  // if set, this is an import of initial bookmarks.html content,
@ -835,13 +836,25 @@ BookmarkContentSink::NewFrame()
      }
      break;
    case BookmarkImportFrame::Container_Toolbar:
-      // toolbar root
-      rv = mBookmarksService->GetToolbarRoot(&ourID);
+      // get toolbar folder
+      PRInt64 btf;
+      rv = mBookmarksService->GetToolbarFolder(&btf);
      NS_ENSURE_SUCCESS(rv, rv);
-      if (mAllowRootChanges) {
-        updateFolder = PR_TRUE;
+      if (!btf) {
+        // create new folder
+        rv = mBookmarksService->CreateFolder(CurFrame().mContainerID,
+                                            containerName,
+                                            mBookmarksService->DEFAULT_INDEX, &ourID);
+        NS_ENSURE_SUCCESS(rv, rv);
+        // there's no toolbar folder, so make us the toolbar folder
+        rv = mBookmarksService->SetToolbarFolder(ourID);
+        NS_ENSURE_SUCCESS(rv, rv);
+        // set favicon
        SetFaviconForFolder(ourID, NS_LITERAL_CSTRING(BOOKMARKS_TOOLBAR_ICON_URI));
      }
+      else {
+        ourID = btf;
+      }
      break;
    default:
      NS_NOTREACHED("Unknown container type");
@ -852,7 +865,7 @@ BookmarkContentSink::NewFrame()
 #endif

  if (updateFolder) {
-    // move the menu/toolbar folder to the current position
+    // move the menu folder to the current position
    mBookmarksService->MoveFolder(ourID, CurFrame().mContainerID, -1);
    mBookmarksService->SetFolderTitle(ourID, containerName);
 #ifdef DEBUG_IMPORT
@ -1160,7 +1173,7 @@ static const char kIndent[] = "    ";

 static const char kPlacesRootAttribute[] = " PLACES_ROOT=\"true\"";
 static const char kBookmarksRootAttribute[] = " BOOKMARKS_MENU=\"true\"";
-static const char kToolbarRootAttribute[] = " PERSONAL_TOOLBAR_FOLDER=\"true\"";
+static const char kToolbarFolderAttribute[] = " PERSONAL_TOOLBAR_FOLDER=\"true\"";
 static const char kIconAttribute[] = " ICON=\"";
 static const char kIconURIAttribute[] = " ICON_URI=\"";
 static const char kHrefAttribute[] = " HREF=\"";
@ -1345,8 +1358,8 @@ nsNavBookmarks::WriteContainerHeader(PRInt64 aFolder, const nsCString& aIndent,
  } else if (aFolder == mBookmarksRoot) {
    rv = aOutput->Write(kBookmarksRootAttribute, sizeof(kBookmarksRootAttribute)-1, &dummy);
    if (NS_FAILED(rv)) return rv;
-  } else if (aFolder == mToolbarRoot) {
-    rv = aOutput->Write(kToolbarRootAttribute, sizeof(kToolbarRootAttribute)-1, &dummy);
+  } else if (aFolder == mToolbarFolder) {
+    rv = aOutput->Write(kToolbarFolderAttribute, sizeof(kToolbarFolderAttribute)-1, &dummy);
    if (NS_FAILED(rv)) return rv;
  }

@ -1609,10 +1622,10 @@ nsNavBookmarks::WriteContainerContents(PRInt64 aFolder, const nsCString& aIndent
    if (items[i]->IsFolder()) {
      // bookmarks folder
      PRInt64 folderId = items[i]->GetAsFolder()->mFolderId;
-      if (aFolder == mRoot && (folderId == mToolbarRoot ||
+      if (aFolder == mRoot && (folderId == mToolbarFolder ||
                               folderId == mBookmarksRoot)) {
-        // don't write out the bookmarks menu or the toolbar folder from the
-        // places root. When writing to bookmarks.html, these are reparented
+        // don't write out the bookmarks menu folder from the
+        // places root. When writing to bookmarks.html, it is reparented
        // to the menu, which is the root of the namespace. This provides
        // better backwards compatability.
        continue;
@ -1704,10 +1717,6 @@ nsNavBookmarks::ExportBookmarksHTML(nsIFile* aBookmarksFile)
  rv = WriteContainer(mRoot, indent, strm);
  if (NS_FAILED(rv)) return rv;

-  // toolbar
-  rv = WriteContainer(mToolbarRoot, indent, strm);
-  if (NS_FAILED(rv)) return rv;
-
  // bookmarks menu contents
  rv = WriteContainerContents(mBookmarksRoot, EmptyCString(), strm);
  if (NS_FAILED(rv)) return rv;
--- a/toolkit/components/places/src/nsNavBookmarks.cpp
+++ b/toolkit/components/places/src/nsNavBookmarks.cpp
@ -78,7 +78,7 @@ nsNavBookmarks* nsNavBookmarks::sInstance = nsnull;
 #define ANNO_FOLDER_READONLY BOOKMARKS_ANNO_PREFIX "readonly"

 nsNavBookmarks::nsNavBookmarks()
-  : mRoot(0), mBookmarksRoot(0), mToolbarRoot(0), mTagRoot(0), mBatchLevel(0),
+  : mRoot(0), mBookmarksRoot(0), mTagRoot(0), mToolbarFolder(0), mBatchLevel(0),
    mBatchHasTransaction(PR_FALSE)
 {
  NS_ASSERTION(!sInstance, "Multiple nsNavBookmarks instances!");
@ -254,6 +254,9 @@ nsNavBookmarks::Init()
  rv = InitRoots();
  NS_ENSURE_SUCCESS(rv, rv);

+  rv = InitToolbarFolder();
+  NS_ENSURE_SUCCESS(rv, rv);
+
  rv = transaction.Commit();
  NS_ENSURE_SUCCESS(rv, rv);

@ -402,10 +405,6 @@ nsNavBookmarks::InitRoots()
  rv = CreateRoot(getRootStatement, NS_LITERAL_CSTRING("menu"), &mBookmarksRoot, nsnull);
  NS_ENSURE_SUCCESS(rv, rv);

-  getRootStatement->Reset();
-  rv = CreateRoot(getRootStatement, NS_LITERAL_CSTRING("toolbar"), &mToolbarRoot, nsnull);
-  NS_ENSURE_SUCCESS(rv, rv);
-
  getRootStatement->Reset();
  rv = CreateRoot(getRootStatement, NS_LITERAL_CSTRING("tags"), &mTagRoot, nsnull);
  NS_ENSURE_SUCCESS(rv, rv);
@ -448,6 +447,28 @@ nsNavBookmarks::InitRoots()
  return NS_OK;
 }

+/**
+ * Initialize the toolbar folder
+ */
+nsresult
+nsNavBookmarks::InitToolbarFolder()
+{
+  mozIStorageConnection *dbConn = DBConn();
+
+  nsCOMPtr<mozIStorageStatement> statement;
+  nsresult rv = dbConn->CreateStatement(NS_LITERAL_CSTRING("SELECT id from moz_bookmarks_folders WHERE type = 'toolbar'"),
+                                        getter_AddRefs(statement));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  PRBool hasResult;
+  rv = statement->ExecuteStep(&hasResult);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (hasResult) {
+    rv = statement->GetInt64(0, &mToolbarFolder);
+    NS_ENSURE_SUCCESS(rv, rv);
+  }
+  return NS_OK;
+}

 // nsNavBookmarks::CreateRoot
 //
@ -802,9 +823,50 @@ nsNavBookmarks::GetBookmarksRoot(PRInt64 *aRoot)
 }

 NS_IMETHODIMP
-nsNavBookmarks::GetToolbarRoot(PRInt64 *aRoot)
+nsNavBookmarks::GetToolbarFolder(PRInt64 *aFolderId)
 {
-  *aRoot = mToolbarRoot;
+  *aFolderId = mToolbarFolder;
+  return NS_OK;
+}
+
+NS_IMETHODIMP
+nsNavBookmarks::SetToolbarFolder(PRInt64 aFolderId)
+{
+  mozIStorageConnection *dbConn = DBConn();
+  mozStorageTransaction transaction(dbConn, PR_FALSE);
+
+  // XXX - validate that input is a valid folder id
+
+  // unset old toolbar folder
+  nsCAutoString buffer;
+  buffer.AssignLiteral("UPDATE moz_bookmarks_folders SET type = '' WHERE id = ");
+  buffer.AppendInt(mToolbarFolder);
+
+  nsresult rv = dbConn->ExecuteSimpleSQL(buffer);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // set new toolbar folder
+  buffer = "";
+  buffer.AssignLiteral("UPDATE moz_bookmarks_folders SET type = 'toolbar' ");
+  buffer.AppendLiteral("WHERE id = ");
+  buffer.AppendInt(aFolderId);
+
+  rv = dbConn->ExecuteSimpleSQL(buffer);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  // commit
+  rv = transaction.Commit();
+  NS_ENSURE_SUCCESS(rv, rv);
+  
+  // update local
+  mToolbarFolder = aFolderId;
+
+  // notify observers
+  nsCOMPtr<nsIURI> folderURI;
+  rv = GetFolderURI(aFolderId, getter_AddRefs(folderURI));
+  ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver,
+                      OnItemChanged(mToolbarFolder, folderURI, NS_LITERAL_CSTRING("became_toolbar_folder"),
+                                    EmptyString()));
  return NS_OK;
 }

@ -1694,7 +1756,7 @@ nsNavBookmarks::ResultNodeForFolder(PRInt64 aID,
  NS_ENSURE_SUCCESS(rv, rv);
  NS_ASSERTION(results, "ResultNodeForFolder expects a valid folder id");

-  // type (empty for normal ones, nonempty for container providers)
+  // type
  nsCAutoString folderType;
  rv = mDBGetFolderInfo->GetUTF8String(kGetFolderInfoIndex_Type, folderType);
  NS_ENSURE_SUCCESS(rv, rv);
--- a/toolkit/components/places/src/nsNavBookmarks.h
+++ b/toolkit/components/places/src/nsNavBookmarks.h
@ -114,6 +114,7 @@ private:
  ~nsNavBookmarks();

  nsresult InitRoots();
+  nsresult InitToolbarFolder();
  nsresult CreateRoot(mozIStorageStatement* aGetRootStatement,
                      const nsCString& name, PRInt64* aID,
                      PRBool* aWasCreated);
@ -135,9 +136,11 @@ private:
  nsMaybeWeakPtrArray<nsINavBookmarkObserver> mObservers;
  PRInt64 mRoot;
  PRInt64 mBookmarksRoot;
-  PRInt64 mToolbarRoot;
  PRInt64 mTagRoot;

+  // personal toolbar folder
+  PRInt64 mToolbarFolder;
+
  // the level of nesting of batches, 0 when no batches are open
  PRInt32 mBatchLevel;

--- a/toolkit/components/places/src/nsNavHistory.cpp
+++ b/toolkit/components/places/src/nsNavHistory.cpp
@ -220,7 +220,7 @@ static const char* gXpcomShutdown = "xpcom-shutdown";
 const char nsNavHistory::kAnnotationPreviousEncoding[] = "history/encoding";

 nsIAtom* nsNavHistory::sMenuRootAtom = nsnull;
-nsIAtom* nsNavHistory::sToolbarRootAtom = nsnull;
+nsIAtom* nsNavHistory::sToolbarFolderAtom = nsnull;
 nsIAtom* nsNavHistory::sSessionStartAtom = nsnull;
 nsIAtom* nsNavHistory::sSessionContinueAtom = nsnull;
 nsIAtom* nsNavHistory::sContainerAtom = nsnull;
@ -244,7 +244,7 @@ nsNavHistory::nsNavHistory() : mNowValid(PR_FALSE),
  gHistoryService = this;

  sMenuRootAtom = NS_NewAtom("menu-root");
-  sToolbarRootAtom = NS_NewAtom("toolbar-root");
+  sToolbarFolderAtom = NS_NewAtom("toolbar-folder");
  sSessionStartAtom = NS_NewAtom("session-start");
  sSessionContinueAtom = NS_NewAtom("session-continue");
  sContainerAtom = NS_NewAtom("container");
@ -261,7 +261,7 @@ nsNavHistory::~nsNavHistory()
  gHistoryService = nsnull;

  NS_IF_RELEASE(sMenuRootAtom);
-  NS_IF_RELEASE(sToolbarRootAtom);
+  NS_IF_RELEASE(sToolbarFolderAtom);
  NS_IF_RELEASE(sSessionStartAtom);
  NS_IF_RELEASE(sSessionContinueAtom);
  NS_IF_RELEASE(sContainerAtom);
--- a/toolkit/components/places/src/nsNavHistory.h
+++ b/toolkit/components/places/src/nsNavHistory.h
@ -243,7 +243,7 @@ public:
  static const PRInt32 kGetInfoIndex_SessionId;

  static nsIAtom* sMenuRootAtom;
-  static nsIAtom* sToolbarRootAtom;
+  static nsIAtom* sToolbarFolderAtom;
  static nsIAtom* sSessionStartAtom;
  static nsIAtom* sSessionContinueAtom;
  static nsIAtom* sContainerAtom;
--- a/toolkit/components/places/src/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/src/nsNavHistoryResult.cpp
@ -4751,17 +4751,17 @@ nsNavHistoryResultTreeViewer::GetCellProperties(PRInt32 row, nsITreeColumn *col,
    return NS_ERROR_INVALID_ARG;

  nsNavHistoryResultNode *node = mVisibleElements[row];
-  PRInt64 folderId, bookmarksRootId, toolbarRootId;
+  PRInt64 folderId, bookmarksRootId, toolbarFolderId;
  node->GetFolderId(&folderId);

  nsCOMPtr<nsINavBookmarksService> bms(do_GetService(
                                            NS_NAVBOOKMARKSSERVICE_CONTRACTID));
  bms->GetBookmarksRoot(&bookmarksRootId);
-  bms->GetToolbarRoot(&toolbarRootId);
+  bms->GetToolbarFolder(&toolbarFolderId);
  if (bookmarksRootId == folderId)
    properties->AppendElement(nsNavHistory::sMenuRootAtom);
-  else if (toolbarRootId == folderId)
-    properties->AppendElement(nsNavHistory::sToolbarRootAtom);
+  else if (toolbarFolderId == folderId)
+    properties->AppendElement(nsNavHistory::sToolbarFolderAtom);
  
  if (mShowSessions && node->mSessionID != 0) {
    if (row == 0 ||