Bug 376253: Places bookmarks import/export unit tests + assorted fixes uncovered by the tests. r=mano, r=dietrich

2007-05-16 23:06:48 -07:00 · 2007-05-16 23:06:48 -07:00 · c9ebcce8b9
--- a/browser/components/migration/src/nsBrowserProfileMigratorUtils.cpp
+++ b/browser/components/migration/src/nsBrowserProfileMigratorUtils.cpp
@ -298,7 +298,7 @@ ImportBookmarksHTML(nsIFile* aBookmarksFile,
  NS_ENSURE_TRUE(localFile, NS_ERROR_FAILURE);
  nsCOMPtr<nsIPlacesImportExportService> importer = do_GetService(NS_PLACESIMPORTEXPORTSERVICE_CONTRACTID, &rv);
  NS_ENSURE_SUCCESS(rv, rv);
-  rv = importer->ImportHTMLFromFileToFolder(localFile, folder);
+  rv = importer->ImportHTMLFromFileToFolder(localFile, folder, PR_TRUE);
  return rv;
 #else
  nsCOMPtr<nsIRDFResource> folder;
--- a/browser/components/nsBrowserGlue.js
+++ b/browser/components/nsBrowserGlue.js
@ -209,7 +209,7 @@ BrowserGlue.prototype = {
        var importer = 
          Components.classes["@mozilla.org/browser/places/import-export-service;1"]
                    .getService(Components.interfaces.nsIPlacesImportExportService);
-        importer.importHTMLFromFile(bookmarksFile);
+        importer.importHTMLFromFile(bookmarksFile, true);
      } catch(ex) {
      } finally {
        prefService.setBoolPref("browser.places.importBookmarksHTML", false);
--- a/browser/components/places/content/places.js
+++ b/browser/components/places/content/places.js
@ -291,7 +291,7 @@ var PlacesOrganizer = {
        var importer = Cc["@mozilla.org/browser/places/import-export-service;1"].
                       getService(Ci.nsIPlacesImportExportService);
        var file = fp.file.QueryInterface(Ci.nsILocalFile);
-        importer.importHTMLFromFile(file);
+        importer.importHTMLFromFile(file, false);
      }
    }
  },
--- a/browser/components/places/public/nsIPlacesImportExportService.idl
+++ b/browser/components/places/public/nsIPlacesImportExportService.idl
@ -45,20 +45,20 @@ interface nsILocalFile;
 * and exporting Places data.
 */

-[scriptable, uuid(212fd626-e07e-11db-8314-0800200c9a66)]
+[scriptable, uuid(21c00314-fa63-11db-8314-0800200c9a66)]
 interface nsIPlacesImportExportService: nsISupports
 {
  /**
   * Loads the given bookmarks.html file and merges it with the current
   * bookmarks hierarchy.
   */
-  void importHTMLFromFile(in nsILocalFile aFile);
+  void importHTMLFromFile(in nsILocalFile aFile, in boolean aIsInitialImport);


  /**
   * Loads the given bookmarks.html file and puts it in the given folder
   */
-  void importHTMLFromFileToFolder(in nsILocalFile aFile, in PRInt64 aFolder);
+  void importHTMLFromFileToFolder(in nsILocalFile aFile, in PRInt64 aFolder, in boolean aIsInitialImport);


  /**
--- a/browser/components/places/src/nsPlacesImportExportService.cpp
+++ b/browser/components/places/src/nsPlacesImportExportService.cpp
@ -129,8 +129,9 @@ static NS_DEFINE_CID(kParserCID, NS_PARSER_CID);
 #define BOOKMARKSS_MENU_ICON_URI "chrome://browser/skin/places/bookmarksMenu.png"
 #define BOOKMARKSS_TOOLBAR_ICON_URI "chrome://browser/skin/places/bookmarksToolbar.png"

-// define to get debugging messages on console about import
+// define to get debugging messages on console about import/export
 //#define DEBUG_IMPORT
+//#define DEBUG_EXPORT

 #if defined(XP_WIN) || defined(XP_OS2)
 #define NS_LINEBREAK "\015\012"
@ -828,10 +829,10 @@ BookmarkContentSink::HandleLinkBegin(const nsIParserNode& node)
    return;

  // attempt to get a property for the supposedly pre-existing bookmark
+  PRInt64 parent;
  if (frame.mPreviousId > 0) {
-    PRInt32 index;
-    rv = mBookmarksService->GetItemIndex(frame.mPreviousId, &index);
-    if (NS_FAILED(rv))
+    rv = mBookmarksService->GetFolderIdForItem(frame.mPreviousId, &parent);
+    if (NS_FAILED(rv) || frame.mContainerID != parent)
      frame.mPreviousId = 0;
  }

@ -913,15 +914,41 @@ BookmarkContentSink::HandleLinkEnd()
    // The bookmark is actually a livemark.  Create it here.
    // (It gets created here instead of in HandleLinkBegin()
    // because we need to know the title before creating it.)
+
+    // check id validity
    if (frame.mPreviousId > 0) {
-      // It's a pre-existing livemark, so update its properties
-      rv = mLivemarkService->SetSiteURI(frame.mPreviousId, frame.mPreviousLink);
-      NS_ASSERTION(NS_SUCCEEDED(rv), "SetSiteURI failed!");
-      rv = mLivemarkService->SetFeedURI(frame.mPreviousId, frame.mPreviousFeed);
-      NS_ASSERTION(NS_SUCCEEDED(rv), "SetFeedURI failed!");
-      rv = mBookmarksService->SetItemTitle(frame.mPreviousId, frame.mPreviousText);
-      NS_ASSERTION(NS_SUCCEEDED(rv), "SetItemTitle failed!");
-    } else {
+      PRInt64 parent;
+      nsresult rv = mBookmarksService->GetFolderIdForItem(frame.mPreviousId, &parent);
+      if (NS_FAILED(rv) || parent != frame.mContainerID) {
+        frame.mPreviousId = 0;
+      }
+    }
+
+    PRBool isLivemark = PR_FALSE;
+    if (frame.mPreviousId > 0) {
+      mLivemarkService->IsLivemark(frame.mPreviousId, &isLivemark);
+      if (isLivemark) {
+        // It's a pre-existing livemark, so update its properties
+#ifdef DEBUG_IMPORT
+        PrintNesting();
+        printf("Updating livemark '%s' %lld\n",
+               NS_ConvertUTF16toUTF8(frame.mPreviousText).get(), frame.mPreviousId);
+#endif
+        rv = mLivemarkService->SetSiteURI(frame.mPreviousId, frame.mPreviousLink);
+        NS_ASSERTION(NS_SUCCEEDED(rv), "SetSiteURI failed!");
+        rv = mLivemarkService->SetFeedURI(frame.mPreviousId, frame.mPreviousFeed);
+        NS_ASSERTION(NS_SUCCEEDED(rv), "SetFeedURI failed!");
+        rv = mBookmarksService->SetItemTitle(frame.mPreviousId, frame.mPreviousText);
+        NS_ASSERTION(NS_SUCCEEDED(rv), "SetItemTitle failed!");
+      }
+    }
+
+    if (!isLivemark) {
+#ifdef DEBUG_IMPORT
+      PrintNesting();
+      printf("Creating livemark '%s' %lld\n",
+             NS_ConvertUTF16toUTF8(frame.mPreviousText).get(), frame.mPreviousId);
+#endif
      if (mIsImportDefaults) {
        rv = mLivemarkService->CreateLivemarkFolderOnly(mBookmarksService,
                                                   frame.mContainerID,
@ -941,11 +968,6 @@ BookmarkContentSink::HandleLinkEnd()
        NS_ASSERTION(NS_SUCCEEDED(rv), "CreateLivemark failed!");
      }
    }
-#ifdef DEBUG_IMPORT
-    PrintNesting();
-    printf("Creating livemark '%s'\n",
-           NS_ConvertUTF16toUTF8(frame.mPreviousText).get());
-#endif
  }
  else if (frame.mPreviousLink) {
 #ifdef DEBUG_IMPORT
@ -963,31 +985,62 @@ BookmarkContentSink::HandleLinkEnd()
 //
 //    Inserts a separator into the current container
 void
-BookmarkContentSink::HandleSeparator(const nsIParserNode& node)
+BookmarkContentSink::HandleSeparator(const nsIParserNode& aNode)
 {
  BookmarkImportFrame& frame = CurFrame();

-  // bookmarks.html contains a separator between the toolbar menu and the
-  // rest of the items.  Since we pull the toolbar menu out into the top level,
-  // we want to skip over this separator since it looks out of place.
-  if (frame.mLastContainerType != BookmarkImportFrame::Container_Toolbar) {
+  // create the separator
+
+  // check for pre-existing id
+  PRInt64 id = 0;
+  nsAutoString idAttr;
+  PRInt32 attrCount = aNode.GetAttributeCount();
+  for (PRInt32 i = 0; i < attrCount; i ++) {
+    const nsAString& key = aNode.GetKeyAt(i);
+    if (key.LowerCaseEqualsLiteral(KEY_ID_LOWER)) {
+      idAttr = aNode.GetValueAt(i);
+    }
+  }
+
+  idAttr.Trim(kWhitespace);
+  id = ConvertImportedIdToInternalId(NS_ConvertUTF16toUTF8(idAttr));
+
+  // check id validity
+  if (id > 0) {
+    PRInt64 parent;
+    nsresult rv = mBookmarksService->GetFolderIdForItem(id, &parent);
+    if (NS_FAILED(rv) || parent != frame.mContainerID) {
+      id = 0;
+    }
+  }
+
+  if (id > 0) {
+    PRUint16 type;
+    nsresult rv = mBookmarksService->GetItemType(id, &type);
+    if (NS_FAILED(rv) || type != mBookmarksService->TYPE_SEPARATOR) {
+      id = 0;
+    }
+  }
+
+  if (id == 0) {
    // create the separator
+
 #ifdef DEBUG_IMPORT
    PrintNesting();
    printf("--------\n");
 #endif
+
    PRInt64 itemId;
    mBookmarksService->InsertSeparator(frame.mContainerID,
                                       mBookmarksService->DEFAULT_INDEX,
                                       &itemId);
-
    // Import separator title if set
    nsAutoString name;
-    PRInt32 attrCount = node.GetAttributeCount();
+    PRInt32 attrCount = aNode.GetAttributeCount();
    for (PRInt32 i = 0; i < attrCount; i ++) {
-      const nsAString& key = node.GetKeyAt(i);
+      const nsAString& key = aNode.GetKeyAt(i);
      if (key.LowerCaseEqualsLiteral(KEY_NAME_LOWER))
-        name = node.GetValueAt(i);
+        name = aNode.GetValueAt(i);
    }
    name.Trim(kWhitespace);

@ -1013,6 +1066,16 @@ BookmarkContentSink::NewFrame()
  CurFrame().ConsumeHeading(&containerName, &containerType, &ourID);

  PRBool updateFolder = PR_FALSE;
+
+  if (ourID > 0) {
+    // Check that folder id is valid.
+    PRInt64 ourParentId;
+    rv = mBookmarksService->GetFolderIdForItem(ourID, &ourParentId);
+    if (NS_FAILED(rv) || ourParentId != CurFrame().mContainerID) {
+      ourID = 0;
+    }
+  }
+
  if (ourID == 0) {
    switch (containerType) {
      case BookmarkImportFrame::Container_Normal:
@ -1858,6 +1921,16 @@ nsPlacesImportExportService::WriteSeparator(nsINavHistoryResultNode* aItem,
  rv = aItem->GetItemId(&itemId);
  NS_ENSURE_SUCCESS(rv, rv);

+  // write id
+  rv = aOutput->Write(kIdAttribute, sizeof(kIdAttribute)-1, &dummy);
+  NS_ENSURE_SUCCESS(rv, rv);
+  nsCAutoString id;
+  id.AppendInt(itemId);
+  rv = aOutput->Write(id.get(), id.Length(), &dummy);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = aOutput->Write(kQuoteStr, sizeof(kQuoteStr)-1, &dummy);
+  NS_ENSURE_SUCCESS(rv, rv);
+
  nsAutoString title;
  rv = mBookmarksService->GetItemTitle(itemId, title);
  if (NS_SUCCEEDED(rv) && !title.IsEmpty()) {
@ -1939,9 +2012,6 @@ nsPlacesImportExportService::WriteContainerContents(PRInt64 aFolder, const nsACS
  // query for just this folder
  rv = query->SetFolders(&aFolder, 1);
  NS_ENSURE_SUCCESS(rv, rv);
-  // query for just bookmarks (necessary?)
-  rv = query->SetOnlyBookmarked(PR_TRUE);
-  NS_ENSURE_SUCCESS(rv, rv);

  // group by folder (necessary? doesn't SetFolders trigger this?)
  const PRUint16 groupMode = nsINavHistoryQueryOptions::GROUP_BY_FOLDER;
@ -2006,19 +2076,19 @@ nsPlacesImportExportService::WriteContainerContents(PRInt64 aFolder, const nsACS
 // nsIPlacesImportExportService::ImportHTMLFromFile
 //
 NS_IMETHODIMP
-nsPlacesImportExportService::ImportHTMLFromFile(nsILocalFile* aFile)
+nsPlacesImportExportService::ImportHTMLFromFile(nsILocalFile* aFile, PRBool aIsInitialImport)
 {
  // this version is exposed on the interface and disallows changing of roots
-  return ImportHTMLFromFileInternal(aFile, PR_FALSE, 0, PR_FALSE);
+  return ImportHTMLFromFileInternal(aFile, PR_FALSE, 0, aIsInitialImport);
 }

 // nsIPlacesImportExportService::ImportHTMLFromFileToFolder
 //
 NS_IMETHODIMP
-nsPlacesImportExportService::ImportHTMLFromFileToFolder(nsILocalFile* aFile, PRInt64 aFolderId)
+nsPlacesImportExportService::ImportHTMLFromFileToFolder(nsILocalFile* aFile, PRInt64 aFolderId, PRBool aIsInitialImport)
 {
  // this version is exposed on the interface and disallows changing of roots
-  return ImportHTMLFromFileInternal(aFile, PR_FALSE, aFolderId, PR_FALSE);
+  return ImportHTMLFromFileInternal(aFile, PR_FALSE, aFolderId, aIsInitialImport);
 }

 nsresult
@ -2037,6 +2107,14 @@ nsPlacesImportExportService::ImportHTMLFromFileInternal(nsILocalFile* aFile,
  printf("\nImporting %s\n", NS_ConvertUTF16toUTF8(path).get());
 #endif

+  // confirm file exists
+  PRBool exists;
+  rv = file->Exists(&exists);
+  NS_ENSURE_SUCCESS(rv, rv);
+  if (!exists) {
+    return NS_ERROR_INVALID_ARG;
+  }
+
  // wrap the import in a transaction to make it faster
  mBookmarksService->BeginUpdateBatch();

@ -2115,6 +2193,11 @@ nsPlacesImportExportService::ExportHTMLToFile(nsILocalFile* aBookmarksFile)
  if (!aBookmarksFile)
    return NS_ERROR_NULL_POINTER;

+#ifdef DEBUG_EXPORT
+  aBookmarksFile->GetPath(path);
+  printf("\nExporting %s\n", NS_ConvertUTF16toUTF8(path).get());
+#endif
+
  nsresult rv = EnsureServiceState();
  NS_ENSURE_SUCCESS(rv, rv);

@ -2169,10 +2252,6 @@ nsPlacesImportExportService::ExportHTMLToFile(nsILocalFile* aBookmarksFile)
  nsCAutoString indent;
  indent.Assign(kIndent);

-  // places root
-  rv = WriteContainer(mPlacesRoot, indent, strm);
-  NS_ENSURE_SUCCESS(rv, rv);
-
  // bookmarks menu contents
  rv = WriteContainerContents(mBookmarksRoot, EmptyCString(), strm);
  NS_ENSURE_SUCCESS(rv, rv);
--- a/browser/components/places/tests/unit/head_bookmarks.js
+++ b/browser/components/places/tests/unit/head_bookmarks.js
@ -83,13 +83,19 @@ function uri(spec) {
  return iosvc.newURI(spec, null, null);
 }

-// Delete a previously created sqlite file
-function clearDB() {
+function cleanUp() {
  try {
+    // Delete a previously created sqlite file
    var file = dirSvc.get('ProfD', Ci.nsIFile);
    file.append("places.sqlite");
    if (file.exists())
      file.remove(false);
+
+    // Delete exported bookmarks html file
+    file = firSvc.get('ProfD', Ci.nsIFile);
+    file.append("bookmarks.exported.html");
+    if (file.exists())
+      file.remove(false);
  } catch(ex) { dump("Exception: " + ex); }
 }
-clearDB();
+cleanUp();
--- a/browser/components/places/tests/unit/tail_bookmarks.js
+++ b/browser/components/places/tests/unit/tail_bookmarks.js
@ -37,5 +37,5 @@

 // put cleanup of the bookmarks test here.

-// remove bookmarks file
-clearDB();
+// remove bookmarks files
+cleanUp();
--- a/browser/components/places/tests/unit/test_bookmarks_html.js
+++ b/browser/components/places/tests/unit/test_bookmarks_html.js
@ -57,6 +57,13 @@ try {
  do_throw("Could not get annotation service\n");
 } 

+// Get livemark service
+try {
+  var livemarksvc = Cc["@mozilla.org/browser/livemark-service;2"].getService(Ci.nsILivemarkService);
+} catch(ex) {
+  do_throw("Could not get livemark service\n");
+} 
+
 // Get microsummary service
 try {
  var mssvc = Cc["@mozilla.org/microsummary/service;1"].getService(Ci.nsIMicrosummaryService);
@ -64,51 +71,101 @@ try {
  do_throw("Could not get microsummary service\n");
 }

+// Get io service
+try {
+  var iosvc = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
+} catch (ex) {
+  do_throw("Could not get io service\n");
+}
+
+
 const DESCRIPTION_ANNO = "bookmarkProperties/description";
+const LOAD_IN_SIDEBAR_ANNO = "bookmarkProperties/loadInSidebar";
+const POST_DATA_ANNO = "URIProperties/POSTData";

 // main
 function run_test() {
  // get places import/export service
  var importer = Cc["@mozilla.org/browser/places/import-export-service;1"].getService(Ci.nsIPlacesImportExportService);

-  // get file pointers
+  // file pointer to legacy bookmarks file
  var bookmarksFileOld = do_get_file("browser/components/places/tests/unit/bookmarks.preplaces.html");
+  // file pointer to a new places-exported bookmarks file
  var bookmarksFileNew = dirSvc.get("ProfD", Ci.nsILocalFile);
  bookmarksFileNew.append("bookmarks.exported.html");

-  // create bookmarks.exported.html if necessary
-  if (!bookmarksFileNew.exists())
-    bookmarksFileNew.create(Ci.nsILocalFile.NORMAL_FILE_TYPE, 0600);
+  // create bookmarks.exported.html
+  if (bookmarksFileNew.exists())
+    bookmarksFileNew.remove(false);
+  bookmarksFileNew.create(Ci.nsILocalFile.NORMAL_FILE_TYPE, 0600);
  if (!bookmarksFileNew.exists())
    do_throw("couldn't create file: bookmarks.exported.html");

-  // Test importing a non-Places canonical bookmarks file.
-  // 1. import bookmarks.preplaces.html
-  // 2. run the test-suite
+  // Test importing a pre-Places canonical bookmarks file.
+  // 1. empty bookmarks db
+  // 2. import bookmarks.preplaces.html
+  // 3. run the test-suite
  bmsvc.removeFolderChildren(bmsvc.bookmarksRoot);
-  importer.importHTMLFromFile(bookmarksFileOld);
+  try {
+    importer.importHTMLFromFile(bookmarksFileOld, true);
+  } catch(ex) { do_throw("couldn't import legacy bookmarks file: " + ex); }
  testCanonicalBookmarks(bmsvc.bookmarksRoot); 

  // Test exporting a Places canonical bookmarks file.
  // 1. export to bookmarks.exported.html
-  // 2. import bookmarks.exported.html
-  // 3. run the test-suite
-  /*
-  importer.exportHTMLToFile(bookmarksFileNew);
+  // 2. empty bookmarks db
+  // 3. import bookmarks.exported.html
+  // 4. run the test-suite
+  try {
+    importer.exportHTMLToFile(bookmarksFileNew);
+  } catch(ex) { do_throw("couldn't export to file: " + ex); }
  bmsvc.removeFolderChildren(bmsvc.bookmarksRoot);
-  importer.importHTMLFromFile(bookmarksFileNew);
+  try {
+    importer.importHTMLFromFile(bookmarksFileNew, true);
+  } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
  testCanonicalBookmarks(bmsvc.bookmarksRoot); 
-  */
+
+  // Test importing a pre-Places canonical bookmarks file to a specific folder.
+  // 1. create a new folder
+  // 2. import bookmarks.preplaces.html to that folder
+  // 3. run the test-suite
+  var testFolder = bmsvc.createFolder(bmsvc.bookmarksRoot, "test-import", bmsvc.DEFAULT_INDEX);
+  try {
+    importer.importHTMLFromFileToFolder(bookmarksFileOld, testFolder, true);
+  } catch(ex) { do_throw("couldn't import the exported file to folder: " + ex); }
+  testCanonicalBookmarks(testFolder);
+  bmsvc.removeFolder(testFolder);

  // Test importing a Places canonical bookmarks file to a specific folder.
  // 1. create a new folder
  // 2. import bookmarks.exported.html to that folder
  // 3. run the test-suite
-  /*
-  var testFolder = bmsvc.createFolder(bmsvc.bookmarksRoot, "test", bmsvc.DEFAULT_INDEX);
-  importer.importHTMLFromFileToFolder(bookmarksFileNew, testFolder);
+  var testFolder = bmsvc.createFolder(bmsvc.bookmarksRoot, "test-import", bmsvc.DEFAULT_INDEX);
+  try {
+    importer.importHTMLFromFileToFolder(bookmarksFileNew, testFolder, true);
+  } catch(ex) { do_throw("couldn't import the exported file to folder: " + ex); }
  testCanonicalBookmarks(testFolder); 
-  */
+  bmsvc.removeFolder(testFolder);
+
+  // Test importing the exported bookmarks.html file *on top of* the existing
+  // bookmarks. This tests import of IDs. If we support IDs correctly, there
+  // should be no difference after the import.
+  // 1. empty bookmarks db
+  // 2. import the exported bookmarks file
+  // 3. export to file
+  // 3. import the exported bookmarks file
+  // 4. run the test-suite
+  bmsvc.removeFolderChildren(bmsvc.bookmarksRoot);
+  try {
+    importer.importHTMLFromFile(bookmarksFileNew, true);
+  } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
+  try {
+    importer.exportHTMLToFile(bookmarksFileNew);
+  } catch(ex) { do_throw("couldn't export to file: " + ex); }
+  try {
+    importer.importHTMLFromFile(bookmarksFileNew, true);
+  } catch(ex) { do_throw("couldn't import the exported file: " + ex); }
+  testCanonicalBookmarks(bmsvc.bookmarksRoot);

  /*
  XXX if there are new fields we add to the bookmarks HTML format
@ -119,88 +176,104 @@ function run_test() {
  3. run the test-suite
  4. run the additional-test-suite
  */
-
-  /*
-  XXX could write another more generic testsuite:
-  - a function that fetches all descendant nodes of a folder with all non-static
-    node properties removed, and serializes w/ toSource()
-  - import a file, get serialization
-  - export it, re-import, get serialization
-  - do_check_eq(str1, str2)
-  */
 }

 // Tests a bookmarks datastore that has a set of bookmarks, etc
 // that flex each supported field and feature.
 function testCanonicalBookmarks(aFolder) {
  // query to see if the deleted folder and items have been imported
-  try {
-    var query = histsvc.getNewQuery();
-    query.setFolders([bmsvc.bookmarksRoot], 1);
-    var result = histsvc.executeQuery(query, histsvc.getNewQueryOptions());
-    var rootNode = result.root;
-    rootNode.containerOpen = true;
-    // get test folder
-    var testFolder = rootNode.getChild(3);
-    do_check_eq(testFolder.type, testFolder.RESULT_TYPE_FOLDER);
-    do_check_eq(testFolder.title, "test");
-    testFolder = testFolder.QueryInterface(Ci.nsINavHistoryQueryResultNode);
-    do_check_eq(testFolder.hasChildren, true);
-    // folder description
-    do_check_true(annosvc.itemHasAnnotation(testFolder.itemId,
-                                            DESCRIPTION_ANNO));
-    do_check_eq("folder test comment",
-                annosvc.getItemAnnotationString(testFolder.itemId,
-                                                DESCRIPTION_ANNO));
-    // open test folder, and test the children 
-    testFolder.containerOpen = true;
-    var cc = testFolder.childCount;
-    // XXX Bug 380468
-    // do_check_eq(cc, 2);
-    do_check_eq(cc, 1);
+  var query = histsvc.getNewQuery();
+  query.setFolders([aFolder], 1);
+  var result = histsvc.executeQuery(query, histsvc.getNewQueryOptions());
+  var rootNode = result.root;
+  rootNode.containerOpen = true;
+  do_check_eq(rootNode.childCount, 6);

-    // test bookmark 1
-    var testBookmark1 = testFolder.getChild(0);
-    // url
-    do_check_eq("http://test/post", testBookmark1.uri);
-    // title
-    do_check_eq("test post keyword", testBookmark1.title);
-    // keyword
-    do_check_eq("test", bmsvc.getKeywordForBookmark(testBookmark1.itemId));
-    // sidebar
-    // add date 
-    // last modified
-    // post data
-    // last charset 
-    // description 
-    do_check_true(annosvc.itemHasAnnotation(testBookmark1.itemId,
-                                            DESCRIPTION_ANNO));
-    do_check_eq("item description",
-                annosvc.getItemAnnotationString(testBookmark1.itemId,
-                                                DESCRIPTION_ANNO));
-    /*
-    // XXX Bug 380468
-    // test bookmark 2
-    var testBookmark2 = testFolder.getChild(1);
-    // url
-    do_check_eq("http://test/micsum", testBookmark2.uri);
-    // title
-    do_check_eq("test microsummary", testBookmark2.title);
-    // check that it's a microsummary
-    var micsum = mssvc.getMicrosummary(testBookmark2.itemId);
-    if (!micsum)
-      do_throw("Could not import microsummary");
-    // check generator uri
-    var generator = micsum.generator;
-    do_check_eq("urn:source:http://dietrich.ganx4.com/mozilla/test-microsummary.xml", generator.uri.spec);
-    // expiration and generated title can change, so don't test them
-    */
+  // bookmarks toolbar
+  var toolbar = rootNode.getChild(2);
+  toolbar.QueryInterface(Ci.nsINavHistoryQueryResultNode);
+  toolbar.containerOpen = true;
+  do_check_eq(toolbar.childCount, 2);
+  

-    // clean up
-    testFolder.containerOpen = false;
-    rootNode.containerOpen = false;
-  }
-  catch(ex) {
-    do_throw("bookmarks query tests failed: " + ex);
-  }
+  // livemark
+  var livemark = toolbar.getChild(1);
+  // title
+  do_check_eq("Latest Headlines", livemark.title);
+  // livemark check
+  do_check_true(livemarksvc.isLivemark(livemark.itemId));
+  // site url
+  do_check_eq("http://en-us.fxfeeds.mozilla.com/en-US/firefox/livebookmarks/",
+              livemarksvc.getSiteURI(livemark.itemId).spec);
+  // feed url
+  do_check_eq("http://en-us.fxfeeds.mozilla.com/en-US/firefox/headlines.xml",
+              livemarksvc.getFeedURI(livemark.itemId).spec);
+
+  toolbar.containerOpen = false;
+
+  // get test folder
+  var testFolder = rootNode.getChild(5);
+  do_check_eq(testFolder.type, testFolder.RESULT_TYPE_FOLDER);
+  do_check_eq(testFolder.title, "test");
+  testFolder = testFolder.QueryInterface(Ci.nsINavHistoryQueryResultNode);
+  do_check_eq(testFolder.hasChildren, true);
+  // folder description
+  do_check_true(annosvc.itemHasAnnotation(testFolder.itemId,
+                                          DESCRIPTION_ANNO));
+  do_check_eq("folder test comment",
+              annosvc.getItemAnnotationString(testFolder.itemId,
+                                              DESCRIPTION_ANNO));
+  // open test folder, and test the children
+  testFolder.containerOpen = true;
+  var cc = testFolder.childCount;
+  // XXX Bug 380468
+  // do_check_eq(cc, 2);
+  do_check_eq(cc, 1);
+
+  // test bookmark 1
+  var testBookmark1 = testFolder.getChild(0);
+  // url
+  do_check_eq("http://test/post", testBookmark1.uri);
+  // title
+  do_check_eq("test post keyword", testBookmark1.title);
+  // keyword
+  do_check_eq("test", bmsvc.getKeywordForBookmark(testBookmark1.itemId));
+  // sidebar
+  do_check_true(annosvc.itemHasAnnotation(testBookmark1.itemId,
+                                          LOAD_IN_SIDEBAR_ANNO));
+  // add date 
+  // last modified
+  // post data
+  var pageURI = iosvc.newURI(testBookmark1.uri, "", null);
+  do_check_true(annosvc.pageHasAnnotation(pageURI, POST_DATA_ANNO));
+  do_check_eq("hidden1%3Dbar&text1%3D%25s",
+              annosvc.getPageAnnotationString(pageURI, POST_DATA_ANNO));
+  // last charset 
+  // description 
+  do_check_true(annosvc.itemHasAnnotation(testBookmark1.itemId,
+                                          DESCRIPTION_ANNO));
+  do_check_eq("item description",
+              annosvc.getItemAnnotationString(testBookmark1.itemId,
+                                              DESCRIPTION_ANNO));
+
+  /*
+  // XXX Bug 380468
+  // test bookmark 2
+  var testBookmark2 = testFolder.getChild(1);
+  // url
+  do_check_eq("http://test/micsum", testBookmark2.uri);
+  // title
+  do_check_eq("test microsummary", testBookmark2.title);
+  // check that it's a microsummary
+  var micsum = mssvc.getMicrosummary(testBookmark2.itemId);
+  if (!micsum)
+    do_throw("Could not import microsummary");
+  // check generator uri
+  var generator = micsum.generator;
+  do_check_eq("urn:source:http://dietrich.ganx4.com/mozilla/test-microsummary.xml", generator.uri.spec);
+  // expiration and generated title can change, so don't test them
+  */
+  // clean up
+  testFolder.containerOpen = false;
+  rootNode.containerOpen = false;
 }
--- a/toolkit/components/places/src/nsNavBookmarks.cpp
+++ b/toolkit/components/places/src/nsNavBookmarks.cpp
@ -1252,6 +1252,10 @@ nsNavBookmarks::RemoveFolder(PRInt64 aFolder)
  rv = transaction.Commit();
  NS_ENSURE_SUCCESS(rv, rv);

+  if (aFolder == mToolbarFolder) {
+    mToolbarFolder = 0;
+  }
+
  ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver,
                      OnItemRemoved(aFolder, parent, index))