fix for bug #381145: export doesn't always write out toolbar folder attribute in some circumstances.r=mano/dietrich

browser/components/places/src/nsPlacesImportExportService.cpp

@@ -286,7 +286,6 @@ nsEscapeHTML(const char * string)
 NS_IMPL_ISUPPORTS1(nsPlacesImportExportService, nsIPlacesImportExportService)
 
 nsPlacesImportExportService::nsPlacesImportExportService()
-  : mPlacesRoot(0), mBookmarksRoot(0), mToolbarFolder(0)
 {
   nsresult rv;
   mHistoryService = do_GetService(NS_NAVHISTORYSERVICE_CONTRACTID, &rv);
@@ -301,10 +300,6 @@ nsPlacesImportExportService::nsPlacesImportExportService()
   NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "could not get livemark service");
   mMicrosummaryService = do_GetService("@mozilla.org/microsummary/service;1", &rv);
   NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "could not get microsummary service");
-
-  mBookmarksService->GetPlacesRoot(&mPlacesRoot);
-  mBookmarksService->GetBookmarksRoot(&mBookmarksRoot);
-  mBookmarksService->GetToolbarFolder(&mToolbarFolder);
 }
 
 nsPlacesImportExportService::~nsPlacesImportExportService()
@@ -635,9 +630,11 @@ BookmarkContentSink::HandleHead1Begin(const nsIParserNode& node)
                    "This can only be set at the beginning.");
         return;
       }
-      PRInt64 mPlacesRoot;
-      mBookmarksService->GetPlacesRoot(&mPlacesRoot);
-      CurFrame().mContainerID = mPlacesRoot;
+
+      PRInt64 placesRoot;
+      nsresult rv = mBookmarksService->GetPlacesRoot(&placesRoot);
+      NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "could not get placesRoot");
+      CurFrame().mContainerID = placesRoot;
       break;
     }
   }
@@ -1106,10 +1103,10 @@ BookmarkContentSink::NewFrame()
         break;
       case BookmarkImportFrame::Container_Toolbar:
         // get toolbar folder
-        PRInt64 bookmarkToolbarFolder;
-        rv = mBookmarksService->GetToolbarFolder(&bookmarkToolbarFolder);
+        PRInt64 toolbarFolder;
+        rv = mBookmarksService->GetToolbarFolder(&toolbarFolder);
         NS_ENSURE_SUCCESS(rv, rv);
-        if (!bookmarkToolbarFolder) {
+        if (!toolbarFolder) {
           // create new folder
           rv = mBookmarksService->CreateFolder(CurFrame().mContainerID,
                                               containerName,
@@ -1120,7 +1117,7 @@ BookmarkContentSink::NewFrame()
           NS_ENSURE_SUCCESS(rv, rv);
         }
         else {
-          ourID = bookmarkToolbarFolder;
+          ourID = toolbarFolder;
         }
         break;
       default:
@@ -1540,14 +1537,26 @@ nsPlacesImportExportService::WriteContainerHeader(PRInt64 aFolder, const nsACStr
   rv = aOutput->Write(kContainerIntro, sizeof(kContainerIntro)-1, &dummy);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  PRInt64 placesRoot;
+  rv = mBookmarksService->GetPlacesRoot(&placesRoot);
+  NS_ENSURE_SUCCESS(rv,rv);
+
+  PRInt64 bookmarksRoot;
+  rv = mBookmarksService->GetBookmarksRoot(&bookmarksRoot);
+  NS_ENSURE_SUCCESS(rv,rv);
+
+  PRInt64 toolbarFolder;
+  rv = mBookmarksService->GetToolbarFolder(&toolbarFolder);
+  NS_ENSURE_SUCCESS(rv,rv);
+
   // " PERSONAL_TOOLBAR_FOLDER="true"", etc.
-  if (aFolder == mPlacesRoot) {
+  if (aFolder == placesRoot) {
     rv = aOutput->Write(kPlacesRootAttribute, sizeof(kPlacesRootAttribute)-1, &dummy);
     NS_ENSURE_SUCCESS(rv, rv);
-  } else if (aFolder == mBookmarksRoot) {
+  } else if (aFolder == bookmarksRoot) {
     rv = aOutput->Write(kBookmarksRootAttribute, sizeof(kBookmarksRootAttribute)-1, &dummy);
     NS_ENSURE_SUCCESS(rv, rv);
-  } else if (aFolder == mToolbarFolder) {
+  } else if (aFolder == toolbarFolder) {
     rv = aOutput->Write(kToolbarFolderAttribute, sizeof(kToolbarFolderAttribute)-1, &dummy);
     NS_ENSURE_SUCCESS(rv, rv);
   }
@@ -1915,8 +1924,8 @@ nsPlacesImportExportService::WriteSeparator(nsINavHistoryResultNode* aItem,
   rv = aItem->GetItemId(&itemId);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  // Note: we can't write the separator ID, because it makes Firefox 2.x
-  // crash/hang - see bug 381129
+  // Note: we can't write the separator ID or anything else other than NAME
+  // because it makes Firefox 2.x crash/hang - see bug 381129
 
   nsAutoString title;
   rv = mBookmarksService->GetItemTitle(itemId, title);
@@ -2018,6 +2027,18 @@ nsPlacesImportExportService::WriteContainerContents(PRInt64 aFolder, const nsACS
   rv = rootNode->SetContainerOpen(PR_TRUE);
   NS_ENSURE_SUCCESS(rv, rv);
 
+  PRInt64 placesRoot;
+  rv = mBookmarksService->GetPlacesRoot(&placesRoot);
+  NS_ENSURE_SUCCESS(rv,rv);
+
+  PRInt64 bookmarksRoot;
+  rv = mBookmarksService->GetBookmarksRoot(&bookmarksRoot);
+  NS_ENSURE_SUCCESS(rv,rv);
+
+  PRInt64 toolbarFolder;
+  rv = mBookmarksService->GetToolbarFolder(&toolbarFolder);
+  NS_ENSURE_SUCCESS(rv,rv);
+
   PRUint32 childCount = 0;
   rootNode->GetChildCount(&childCount);
   for (PRUint32 i = 0; i < childCount; ++i) {
@@ -2032,8 +2053,8 @@ nsPlacesImportExportService::WriteContainerContents(PRInt64 aFolder, const nsACS
       PRInt64 folderId;
       rv = child->GetItemId(&folderId);
       NS_ENSURE_SUCCESS(rv, rv);
-      if (aFolder == mPlacesRoot && (folderId == mToolbarFolder ||
-                               folderId == mBookmarksRoot)) {
+      if (aFolder == placesRoot && (folderId == toolbarFolder ||
+                                    folderId == bookmarksRoot)) {
         // 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
@@ -2106,7 +2127,11 @@ nsPlacesImportExportService::ImportHTMLFromFileInternal(nsILocalFile* aFile,
   mBookmarksService->BeginUpdateBatch();
 
   if (aIsImportDefaults) {
-    rv = mBookmarksService->RemoveFolderChildren(mBookmarksRoot);
+    PRInt64 bookmarksRoot;
+    rv = mBookmarksService->GetBookmarksRoot(&bookmarksRoot);
+    NS_ENSURE_SUCCESS(rv,rv);
+
+    rv = mBookmarksService->RemoveFolderChildren(bookmarksRoot);
     NS_ENSURE_SUCCESS(rv, rv);
   }
 
@@ -2219,9 +2244,13 @@ nsPlacesImportExportService::ExportHTMLToFile(nsILocalFile* aBookmarksFile)
   rv = strm->Write(kRootIntro, sizeof(kRootIntro)-1, &dummy); // <H1
   NS_ENSURE_SUCCESS(rv, rv);
 
+  PRInt64 bookmarksRoot;
+  rv = mBookmarksService->GetBookmarksRoot(&bookmarksRoot);
+  NS_ENSURE_SUCCESS(rv,rv);
+
   // bookmarks menu favicon
   nsCOMPtr<nsIURI> folderURI;
-  rv = mBookmarksService->GetFolderURI(mBookmarksRoot, getter_AddRefs(folderURI));
+  rv = mBookmarksService->GetFolderURI(bookmarksRoot, getter_AddRefs(folderURI));
   NS_ENSURE_SUCCESS(rv, rv);
   nsCAutoString folderSpec;
   rv = folderURI->GetSpec(folderSpec);
@@ -2232,7 +2261,7 @@ nsPlacesImportExportService::ExportHTMLToFile(nsILocalFile* aBookmarksFile)
   // '>Bookmarks</H1>
   rv = strm->Write(kCloseAngle, sizeof(kCloseAngle)-1, &dummy); // >
   NS_ENSURE_SUCCESS(rv, rv);
-  rv = WriteContainerTitle(mBookmarksRoot, strm);
+  rv = WriteContainerTitle(bookmarksRoot, strm);
   NS_ENSURE_SUCCESS(rv, rv);
   rv = strm->Write(kCloseRootH1, sizeof(kCloseRootH1)-1, &dummy); // </H1>
   NS_ENSURE_SUCCESS(rv, rv);
@@ -2246,7 +2275,7 @@ nsPlacesImportExportService::ExportHTMLToFile(nsILocalFile* aBookmarksFile)
   indent.Assign(kIndent);
 
   // bookmarks menu contents
-  rv = WriteContainerContents(mBookmarksRoot, EmptyCString(), strm);
+  rv = WriteContainerContents(bookmarksRoot, EmptyCString(), strm);
   NS_ENSURE_SUCCESS(rv, rv);
 
   // epilogue

browser/components/places/src/nsPlacesImportExportService.h

@@ -31,10 +31,6 @@ class nsPlacesImportExportService : public nsIPlacesImportExportService
     nsCOMPtr<nsILivemarkService> mLivemarkService;
     nsCOMPtr<nsIMicrosummaryService> mMicrosummaryService;
 
-    PRInt64 mPlacesRoot;
-    PRInt64 mBookmarksRoot;
-    PRInt64 mToolbarFolder;
-
     nsresult ImportHTMLFromFileInternal(nsILocalFile* aFile, PRBool aAllowRootChanges,
                                        PRInt64 aFolder, PRBool aIsImportDefaults);
     nsresult WriteContainer(PRInt64 aFolder, const nsACString& aIndent, nsIOutputStream* aOutput);