From 9a163d79316a901fa365973835abe72bf6a2cff1 Mon Sep 17 00:00:00 2001
From: "reed%reedloden.com"
 <reed%reedloden.com@18797224-902f-48f8-a5cc-f745e15eee43>
Date: Tue, 8 Apr 2008 18:43:40 +0000
Subject: [PATCH] Bug 421483 - "Reorganize pre-populated smart bookmarks (add
 versioning)" [p=mak77@supereva.it (Marco Bonardo [mak77]) ui-r=beltzner
 r=dietrich a=blocking-firefox3+]

git-svn-id: svn://10.0.0.236/trunk@249823 18797224-902f-48f8-a5cc-f745e15eee43
---
 .../browser/base/content/browser-menubar.inc  |   8 +-
 mozilla/browser/components/nsBrowserGlue.js   | 157 ++++++++++++------
 mozilla/browser/components/nsIBrowserGlue.idl |   2 +-
 .../places/tests/unit/test_384370.js          |   2 +-
 .../places/tests/unit/test_421483.js          | 122 ++++++++++++++
 .../places/tests/unit/test_bookmarks_html.js  |   2 +-
 .../components/places/src/nsNavHistory.cpp    |   4 +-
 7 files changed, 235 insertions(+), 62 deletions(-)
 create mode 100644 mozilla/browser/components/places/tests/unit/test_421483.js

diff --git a/mozilla/browser/base/content/browser-menubar.inc b/mozilla/browser/base/content/browser-menubar.inc
index 24d11f285fc..fe68132547c 100644
--- a/mozilla/browser/base/content/browser-menubar.inc
+++ b/mozilla/browser/base/content/browser-menubar.inc
@@ -407,7 +407,7 @@
                oncommand="BookmarksEventHandler.onCommand(event);"
                onclick="BookmarksEventHandler.onClick(event);"
                onpopupshowing="BookmarksEventHandler.onPopupShowing(event);">
-      <menuitem label="&bookmarkThisPageCmd.label;" 
+      <menuitem label="&bookmarkThisPageCmd.label;"
                 command="Browser:AddBookmarkAs" key="addBookmarkAsKb"/>
       <menuitem id="subscribeToPageMenuitem"
                 label="&subscribeToPageMenuitem.label;"
@@ -418,17 +418,17 @@
             label="&subscribeToPageMenupopup.label;"
             hidden="true">
         <menupopup id="subscribeToPageSubmenuMenupopup" 
-                   onpopupshowing="return FeedHandler.buildFeedList(event.target);" 
+                   onpopupshowing="return FeedHandler.buildFeedList(event.target);"
                    oncommand="return FeedHandler.subscribeToFeed(null, event);"
                    onclick="checkForMiddleClick(this, event);"/>
       </menu>
-      <menuitem label="&addCurPagesCmd.label;" 
+      <menuitem label="&addCurPagesCmd.label;"
                 command="Browser:BookmarkAllTabs" key="bookmarkAllTabsKb"/>
-      <menuseparator id="organizeBookmarksSeparator"/>
       <menuitem id="bookmarksShowAll"
                 label="&organizeBookmarks.label;"
                 command="Browser:ShowAllBookmarks"
                 key="manBookmarkKb"/>
+      <menuseparator id="organizeBookmarksSeparator"/>
       <menu id="bookmarksToolbarFolderMenu"
             class="menu-iconic bookmark-item"
             container="true">
diff --git a/mozilla/browser/components/nsBrowserGlue.js b/mozilla/browser/components/nsBrowserGlue.js
index c79333f6f41..87cec3653ee 100644
--- a/mozilla/browser/components/nsBrowserGlue.js
+++ b/mozilla/browser/components/nsBrowserGlue.js
@@ -388,8 +388,10 @@ BrowserGlue.prototype = {
    * of the Places db:
    * - browser.places.importBookmarksHTML
    *   Set to false by the history service to indicate we need to re-import.
-   * - browser.places.createdSmartBookmarks
-   *   Set during HTML import to indicate that the queries were created.
+   * - browser.places.smartBookmarksVersion
+   *   Set during HTML import to indicate that Smart Bookmarks were created.
+   *   Set to -1 to disable Smart Bookmarks creation.
+   *   Set to 0 to restore current Smart Bookmarks.
    *
    * These prefs are set up by the frontend:
    * - browser.bookmarks.restore_default_bookmarks
@@ -444,7 +446,7 @@ BrowserGlue.prototype = {
         // if there's no JSON backup or we are restoring default bookmarks
 
         // ensurePlacesDefaultQueriesInitialized() is called by import.
-        prefBranch.setBoolPref("browser.places.createdSmartBookmarks", false);
+        prefBranch.setIntPref("browser.places.smartBookmarksVersion", 0);
 
         var dirService = Cc["@mozilla.org/file/directory_service;1"].
                          getService(Ci.nsIProperties);
@@ -597,19 +599,31 @@ BrowserGlue.prototype = {
   },
 
   ensurePlacesDefaultQueriesInitialized: function() {
-    // bail out if the folder is already created
+    const SMART_BOOKMARKS_VERSION = 1;
+    const SMART_BOOKMARKS_ANNO = "Places/SmartBookmark";
+    const SMART_BOOKMARKS_PREF = "browser.places.smartBookmarksVersion";
+
+    // XXX should this be a pref?  see bug #399268
+    const MAX_RESULTS = 10;
+
     var prefBranch = Cc["@mozilla.org/preferences-service;1"].
                      getService(Ci.nsIPrefBranch);
-    var createdSmartBookmarks = false;
-    try {
-      createdSmartBookmarks = prefBranch.getBoolPref("browser.places.createdSmartBookmarks");
-    } catch(ex) { }
 
-    if (createdSmartBookmarks)
+    // get current smart bookmarks version
+    var smartBookmarksCurrentVersion = -1;
+    try {
+      smartBookmarksCurrentVersion = prefBranch.getIntPref(SMART_BOOKMARKS_PREF);
+    } catch(ex) {}
+
+    // bail out if we don't have to create or update Smart Bookmarks
+    if (smartBookmarksCurrentVersion == -1 ||
+        smartBookmarksCurrentVersion >= SMART_BOOKMARKS_VERSION)
       return;
 
     var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
                 getService(Ci.nsINavBookmarksService);
+    var annosvc = Cc["@mozilla.org/browser/annotation-service;1"].
+                  getService(Ci.nsIAnnotationService);
 
     var callback = {
       _placesBundle: Cc["@mozilla.org/intl/stringbundle;1"].
@@ -623,54 +637,91 @@ BrowserGlue.prototype = {
       },
 
       runBatched: function() {
-        var smartBookmarksFolderTitle =
-          this._placesBundle.GetStringFromName("smartBookmarksFolderTitle");
-        var mostVisitedTitle =
-          this._placesBundle.GetStringFromName("mostVisitedTitle");
-        var recentlyBookmarkedTitle =
-          this._placesBundle.GetStringFromName("recentlyBookmarkedTitle");
-        var recentTagsTitle =
-          this._placesBundle.GetStringFromName("recentTagsTitle");
+        var smartBookmarks = [];
+        var bookmarksMenuIndex = 0;
+        var bookmarksToolbarIndex = 0;
 
-        var defaultIndex = bmsvc.DEFAULT_INDEX;
+        // MOST VISITED
+        var smart = {queryId: "MostVisited", // don't change this
+                     itemId: null,
+                     title: this._placesBundle.GetStringFromName("mostVisitedTitle"),
+                     uri: this._uri("place:queryType=" +
+                                    Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY +
+                                    "&sort=" +
+                                    Ci.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING +
+                                    "&maxResults=" + MAX_RESULTS),
+                     parent: bmsvc.toolbarFolder,
+                     position: bookmarksToolbarIndex++};
+        smartBookmarks.push(smart);
 
-        // index = 0, make it the first folder
-        var placesFolder = bmsvc.createFolder(bmsvc.toolbarFolder, smartBookmarksFolderTitle,
-                                              0);
+        // RECENTLY BOOKMARKED
+        smart = {queryId: "RecentlyBookmarked", // don't change this
+                 itemId: null,
+                 title: this._placesBundle.GetStringFromName("recentlyBookmarkedTitle"),
+                 uri: this._uri("place:folder=BOOKMARKS_MENU" +
+                                "&folder=UNFILED_BOOKMARKS" +
+                                "&folder=TOOLBAR" +
+                                "&queryType=" +
+                                Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS +
+                                "&sort=" +
+                                Ci.nsINavHistoryQueryOptions.SORT_BY_DATEADDED_DESCENDING +
+                                "&excludeItemIfParentHasAnnotation=livemark%2FfeedURI" +
+                                "&maxResults=" + MAX_RESULTS +
+                                "&excludeQueries=1"),
+                 parent: bmsvc.bookmarksMenuFolder,
+                 position: bookmarksMenuIndex++};
+        smartBookmarks.push(smart);
 
-        // XXX should this be a pref?  see bug #399268
-        var maxResults = 10;
+        // RECENT TAGS
+        smart = {queryId: "RecentTags", // don't change this
+                 itemId: null,
+                 title: this._placesBundle.GetStringFromName("recentTagsTitle"),
+                 uri: this._uri("place:"+
+                    "type=" +
+                    Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY +
+                    "&sort=" +
+                    Ci.nsINavHistoryQueryOptions.SORT_BY_LASTMODIFIED_DESCENDING +
+                    "&maxResults=" + MAX_RESULTS),
+                 parent: bmsvc.bookmarksMenuFolder,
+                 position: bookmarksMenuIndex++};
+        smartBookmarks.push(smart);
 
-        var mostVisitedItem = bmsvc.insertBookmark(placesFolder,
-          this._uri("place:queryType=" +
-              Ci.nsINavHistoryQueryOptions.QUERY_TYPE_HISTORY +
-              "&sort=" +
-              Ci.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING +
-              "&maxResults=" + maxResults),
-              defaultIndex, mostVisitedTitle);
+        var smartBookmarkItemIds = annosvc.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO, {});
+        // set current itemId, parent and position if Smart Bookmark exists
+        for each(var itemId in smartBookmarkItemIds) {
+          var queryId = annosvc.getItemAnnotation(itemId, SMART_BOOKMARKS_ANNO);
+          for (var i = 0; i < smartBookmarks.length; i++){
+            if (smartBookmarks[i].queryId == queryId) {
+              smartBookmarks[i].itemId = itemId;
+              smartBookmarks[i].parent = bmsvc.getFolderIdForItem(itemId);
+              smartBookmarks[i].position = bmsvc.getItemIndex(itemId);
+              // remove current item, since it will be replaced
+              bmsvc.removeItem(itemId);
+              break;
+            }
+            // We don't remove old Smart Bookmarks because user could still
+            // find them useful, or could have personalized them.
+            // Instead we remove the Smart Bookmark annotation.
+            if (i == smartBookmarks.length - 1)
+              annosvc.removeItemAnnotation(itemId, SMART_BOOKMARKS_ANNO);
+          }
+        }
 
-        // excludeQueries=1 so that user created "saved searches" 
-        // and these queries (added automatically) are excluded
-        var recentlyBookmarkedItem = bmsvc.insertBookmark(placesFolder,
-          this._uri("place:folder=BOOKMARKS_MENU" + 
-              "&folder=UNFILED_BOOKMARKS" +
-              "&folder=TOOLBAR" +
-              "&queryType=" + Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS +
-              "&sort=" +
-              Ci.nsINavHistoryQueryOptions.SORT_BY_DATEADDED_DESCENDING +
-              "&excludeItemIfParentHasAnnotation=livemark%2FfeedURI" +
-              "&maxResults=" + maxResults +
-              "&excludeQueries=1"),
-              defaultIndex, recentlyBookmarkedTitle);
-
-        var sep =  bmsvc.insertSeparator(placesFolder, defaultIndex);
-
-        var recentTagsItem = bmsvc.insertBookmark(placesFolder,
-          this._uri("place:"+
-              "type=" + Ci.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY +
-              "&sort=" + Ci.nsINavHistoryQueryOptions.SORT_BY_LASTMODIFIED_DESCENDING +
-              "&maxResults=" + maxResults),
-          defaultIndex, recentTagsTitle);
+        // create smart bookmarks
+        for each(var smartBookmark in smartBookmarks) {
+          smartBookmark.itemId = bmsvc.insertBookmark(smartBookmark.parent,
+                                                      smartBookmark.uri,
+                                                      smartBookmark.position,
+                                                      smartBookmark.title);
+          annosvc.setItemAnnotation(smartBookmark.itemId,
+                                    SMART_BOOKMARKS_ANNO, smartBookmark.queryId,
+                                    0, annosvc.EXPIRE_NEVER);
+        }
+        
+        // If we are creating all Smart Bookmarks from ground up, add a
+        // separator below them in the bookmarks menu.
+        if (smartBookmarkItemIds.length == 0)
+          bmsvc.insertSeparator(bmsvc.bookmarksMenuFolder, bookmarksMenuIndex);
       }
     };
 
@@ -681,7 +732,7 @@ BrowserGlue.prototype = {
       Components.utils.reportError(ex);
     }
     finally {
-      prefBranch.setBoolPref("browser.places.createdSmartBookmarks", true);
+      prefBranch.setIntPref(SMART_BOOKMARKS_PREF, SMART_BOOKMARKS_VERSION);
       prefBranch.QueryInterface(Ci.nsIPrefService).savePrefFile(null);
     }
   },
diff --git a/mozilla/browser/components/nsIBrowserGlue.idl b/mozilla/browser/components/nsIBrowserGlue.idl
index 3a8fa6e45fc..3413d979156 100644
--- a/mozilla/browser/components/nsIBrowserGlue.idl
+++ b/mozilla/browser/components/nsIBrowserGlue.idl
@@ -70,7 +70,7 @@ interface nsIBrowserGlue : nsISupports
   void sanitize(in nsIDOMWindow aParentWindow);
 
   /**
-   * Add the special "Places" folder (with some special queries) to the personal toolbar folder.
+   * Add Smart Bookmarks special queries to bookmarks menu and toolbar folder.
    */
   void ensurePlacesDefaultQueriesInitialized();
 };
diff --git a/mozilla/browser/components/places/tests/unit/test_384370.js b/mozilla/browser/components/places/tests/unit/test_384370.js
index efa8adc26a6..1fdb983059e 100644
--- a/mozilla/browser/components/places/tests/unit/test_384370.js
+++ b/mozilla/browser/components/places/tests/unit/test_384370.js
@@ -62,7 +62,7 @@ function run_test() {
 
   // avoid creating the places smart folder during tests
   Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch).
-  setBoolPref("browser.places.createdSmartBookmarks", true);
+  setIntPref("browser.places.smartBookmarksVersion", -1);
 
   // file pointer to legacy bookmarks file
   //var bookmarksFileOld = do_get_file("browser/components/places/tests/unit/bookmarks.large.html");
diff --git a/mozilla/browser/components/places/tests/unit/test_421483.js b/mozilla/browser/components/places/tests/unit/test_421483.js
new file mode 100644
index 00000000000..0e6e6762d5f
--- /dev/null
+++ b/mozilla/browser/components/places/tests/unit/test_421483.js
@@ -0,0 +1,122 @@
+/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is Places Unit Test code.
+ *
+ * The Initial Developer of the Original Code is Mozilla Corp.
+ * Portions created by the Initial Developer are Copyright (C) 2008
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *  Marco Bonardo <mak77@supereva.it>
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+// Get bookmarks service
+try {
+  var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
+              getService(Ci.nsINavBookmarksService);
+} catch(ex) {
+  do_throw("Could not get Bookmarks service\n");
+}
+
+// Get annotation service
+try {
+  var annosvc = Cc["@mozilla.org/browser/annotation-service;1"].
+                getService(Ci.nsIAnnotationService);
+} catch(ex) {
+  do_throw("Could not get Annotation service\n");
+}
+
+// Get browser glue
+try {
+  var gluesvc = Cc["@mozilla.org/browser/browserglue;1"].
+                getService(Ci.nsIBrowserGlue);
+} catch(ex) {
+  do_throw("Could not get BrowserGlue service\n");
+}
+
+// Get pref service
+try {
+  var pref =  Cc["@mozilla.org/preferences-service;1"].
+              getService(Ci.nsIPrefBranch);
+} catch(ex) {
+  do_throw("Could not get Preferences service\n");
+}
+
+const SMART_BOOKMARKS_ANNO = "Places/SmartBookmark";
+const SMART_BOOKMARKS_PREF = "browser.places.smartBookmarksVersion";
+
+// main
+function run_test() {
+  // TEST 1: smart bookmarks disabled
+  pref.setIntPref("browser.places.smartBookmarksVersion", -1);
+  gluesvc.ensurePlacesDefaultQueriesInitialized();
+  var smartBookmarkItemIds = annosvc.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO, {});
+  do_check_eq(smartBookmarkItemIds.length, 0);
+  // check that pref has not been bumped up
+  do_check_eq(pref.getIntPref("browser.places.smartBookmarksVersion"), -1);
+
+  // TEST 2: create smart bookmarks
+  pref.setIntPref("browser.places.smartBookmarksVersion", 0);
+  gluesvc.ensurePlacesDefaultQueriesInitialized();
+  smartBookmarkItemIds = annosvc.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO, {});
+  do_check_neq(smartBookmarkItemIds.length, 0);
+  // check that pref has been bumped up
+  do_check_true(pref.getIntPref("browser.places.smartBookmarksVersion") > 0);
+
+  var smartBookmarksCount = smartBookmarkItemIds.length;
+
+  // TEST 3: smart bookmarks restore
+  // remove one smart bookmark and restore
+  bmsvc.removeItem(smartBookmarkItemIds[0]);
+  pref.setIntPref("browser.places.smartBookmarksVersion", 0);
+  gluesvc.ensurePlacesDefaultQueriesInitialized();
+  smartBookmarkItemIds = annosvc.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO, {});
+  do_check_eq(smartBookmarkItemIds.length, smartBookmarksCount);
+  // check that pref has been bumped up
+  do_check_true(pref.getIntPref("browser.places.smartBookmarksVersion") > 0);
+
+  // TEST 4: move a smart bookmark, change its title, then restore
+  // smart bookmark should be restored in place
+  var parent = bmsvc.getFolderIdForItem(smartBookmarkItemIds[0]);
+  var oldTitle = bmsvc.getItemTitle(smartBookmarkItemIds[0]);
+  // create a subfolder and move inside it
+  var newParent = bmsvc.createFolder(parent, "test", bmsvc.DEFAULT_INDEX);
+  bmsvc.moveItem(smartBookmarkItemIds[0], newParent, bmsvc.DEFAULT_INDEX);
+  // change title
+  bmsvc.setItemTitle(smartBookmarkItemIds[0], "new title");
+  // restore
+  pref.setIntPref("browser.places.smartBookmarksVersion", 0);
+  gluesvc.ensurePlacesDefaultQueriesInitialized();
+  smartBookmarkItemIds = annosvc.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO, {});
+  do_check_eq(smartBookmarkItemIds.length, smartBookmarksCount);
+  do_check_eq(bmsvc.getFolderIdForItem(smartBookmarkItemIds[0]), newParent);
+  do_check_eq(bmsvc.getItemTitle(smartBookmarkItemIds[0]), oldTitle);
+  // check that pref has been bumped up
+  do_check_true(pref.getIntPref("browser.places.smartBookmarksVersion") > 0);
+}
diff --git a/mozilla/browser/components/places/tests/unit/test_bookmarks_html.js b/mozilla/browser/components/places/tests/unit/test_bookmarks_html.js
index 7a341c6c292..81903d00f82 100644
--- a/mozilla/browser/components/places/tests/unit/test_bookmarks_html.js
+++ b/mozilla/browser/components/places/tests/unit/test_bookmarks_html.js
@@ -89,7 +89,7 @@ function run_test() {
 
   // avoid creating the places smart folder during tests
   Cc["@mozilla.org/preferences-service;1"].getService(Ci.nsIPrefBranch).
-  setBoolPref("browser.places.createdSmartBookmarks", true);
+  setIntPref("browser.places.smartBookmarksVersion", -1);
 
   // file pointer to legacy bookmarks file
   var bookmarksFileOld = do_get_file("browser/components/places/tests/unit/bookmarks.preplaces.html");
diff --git a/mozilla/toolkit/components/places/src/nsNavHistory.cpp b/mozilla/toolkit/components/places/src/nsNavHistory.cpp
index 25e5dd83413..31e3b12b810 100644
--- a/mozilla/toolkit/components/places/src/nsNavHistory.cpp
+++ b/mozilla/toolkit/components/places/src/nsNavHistory.cpp
@@ -145,7 +145,7 @@
 #define PREF_FRECENCY_UNVISITED_TYPED_BONUS     "places.frecency.unvisitedTypedBonus"
 #define PREF_BROWSER_IMPORT_BOOKMARKS           "browser.places.importBookmarksHTML"
 #define PREF_BROWSER_IMPORT_DEFAULTS            "browser.places.importDefaults"
-#define PREF_BROWSER_CREATEDSMARTBOOKMARKS      "browser.places.createdSmartBookmarks"
+#define PREF_BROWSER_SMARTBOOKMARKSVERSION      "browser.places.smartBookmarksVersion"
 #define PREF_BROWSER_LEFTPANEFOLDERID           "browser.places.leftPaneFolderId"
 
 // Default (integer) value of PREF_DB_CACHE_PERCENTAGE from 0-100
@@ -600,7 +600,7 @@ nsNavHistory::InitDBFile(PRBool aForceInit)
       NS_ENSURE_SUCCESS(rv, rv);  
 
       // if the places.sqlite gets deleted/corrupted the queries should be created again
-      rv = prefs->SetBoolPref(PREF_BROWSER_CREATEDSMARTBOOKMARKS, PR_FALSE);
+      rv = prefs->SetIntPref(PREF_BROWSER_SMARTBOOKMARKSVERSION, 0);
       NS_ENSURE_SUCCESS(rv, rv);  
       
       // we must create a new Organizer left pane folder root, the old will not be valid anymore