Bug 743692. (Ev1a) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 3 (Logic changes). r=neil.
authorSerge Gautherie <sgautherie.bz@free.fr>
Thu, 19 Apr 2012 17:57:15 +0200
changeset 11672 d04fc59c787f9646b7048b392ff6020e241c6fce
parent 11671 7fe3e7bebed6e27b889afd070975bfd75e51e55f
child 11673 96209faea907e76ffe3dfec5914d39c2ecc6fe3a
push idunknown
push userunknown
push dateunknown
reviewersneil
bugs743692, 493557
Bug 743692. (Ev1a) Port |Bug 493557 - "Recent Tags" and "Recently Bookmarked" are flipped when smart bookmarks are updated| to SeaMonkey, part 3 (Logic changes). r=neil.
suite/common/places/tests/unit/test_browserGlue_smartBookmarks.js
suite/common/src/nsSuiteGlue.js
--- a/suite/common/places/tests/unit/test_browserGlue_smartBookmarks.js
+++ b/suite/common/places/tests/unit/test_browserGlue_smartBookmarks.js
@@ -141,16 +141,121 @@ tests.push({
 
     next_test();
   }
 });
 
 //------------------------------------------------------------------------------
 
 tests.push({
+  description: "bookmarks position is retained when version changes.",
+  exec: function() {
+    // Sanity check items.
+    do_check_eq(countFolderChildren(PlacesUtils.toolbarFolderId),
+                SMART_BOOKMARKS_ON_TOOLBAR + DEFAULT_BOOKMARKS_ON_TOOLBAR);
+    do_check_eq(countFolderChildren(PlacesUtils.bookmarksMenuFolderId),
+                SMART_BOOKMARKS_ON_MENU + DEFAULT_BOOKMARKS_ON_MENU);
+
+    let itemId = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId, 0);
+    do_check_true(PlacesUtils.annotations.itemHasAnnotation(itemId, SMART_BOOKMARKS_ANNO));
+    let firstItemTitle = PlacesUtils.bookmarks.getItemTitle(itemId);
+
+    itemId = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId, 1);
+    do_check_true(PlacesUtils.annotations.itemHasAnnotation(itemId, SMART_BOOKMARKS_ANNO));
+    let secondItemTitle = PlacesUtils.bookmarks.getItemTitle(itemId);
+
+    // Set preferences.
+    Services.prefs.setIntPref(PREF_SMART_BOOKMARKS_VERSION, 1);
+
+    rebuildSmartBookmarks();
+
+    // Count items.
+    do_check_eq(countFolderChildren(PlacesUtils.toolbarFolderId),
+                SMART_BOOKMARKS_ON_TOOLBAR + DEFAULT_BOOKMARKS_ON_TOOLBAR);
+    do_check_eq(countFolderChildren(PlacesUtils.bookmarksMenuFolderId),
+                SMART_BOOKMARKS_ON_MENU + DEFAULT_BOOKMARKS_ON_MENU);
+
+    // Check smart bookmarks are still in correct position.
+    itemId = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId, 0);
+    do_check_true(PlacesUtils.annotations.itemHasAnnotation(itemId, SMART_BOOKMARKS_ANNO));
+    do_check_eq(PlacesUtils.bookmarks.getItemTitle(itemId), firstItemTitle);
+
+    itemId = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId, 1);
+    do_check_true(PlacesUtils.annotations.itemHasAnnotation(itemId, SMART_BOOKMARKS_ANNO));
+    do_check_eq(PlacesUtils.bookmarks.getItemTitle(itemId), secondItemTitle);
+
+    // Check version has been updated.
+    do_check_eq(Services.prefs.getIntPref(PREF_SMART_BOOKMARKS_VERSION),
+                SMART_BOOKMARKS_VERSION);
+
+    next_test();
+  }
+});
+
+//------------------------------------------------------------------------------
+
+tests.push({
+  description: "moved bookmarks position is retained when version changes.",
+  exec: function() {
+    // Sanity check items.
+    do_check_eq(countFolderChildren(PlacesUtils.toolbarFolderId),
+                SMART_BOOKMARKS_ON_TOOLBAR + DEFAULT_BOOKMARKS_ON_TOOLBAR);
+    do_check_eq(countFolderChildren(PlacesUtils.bookmarksMenuFolderId),
+                SMART_BOOKMARKS_ON_MENU + DEFAULT_BOOKMARKS_ON_MENU);
+
+    let itemId1 = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId, 0);
+    do_check_true(PlacesUtils.annotations.itemHasAnnotation(itemId1, SMART_BOOKMARKS_ANNO));
+    let firstItemTitle = PlacesUtils.bookmarks.getItemTitle(itemId1);
+
+    let itemId2 = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId, 1);
+    do_check_true(PlacesUtils.annotations.itemHasAnnotation(itemId2, SMART_BOOKMARKS_ANNO));
+    let secondItemTitle = PlacesUtils.bookmarks.getItemTitle(itemId2);
+
+    // Move the first smart bookmark to the end of the menu.
+    PlacesUtils.bookmarks.moveItem(itemId1, PlacesUtils.bookmarksMenuFolderId,
+                                   PlacesUtils.bookmarks.DEFAULT_INDEX);
+
+    do_check_eq(itemId1, PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId,
+                                                   PlacesUtils.bookmarks.DEFAULT_INDEX));
+
+    // Set preferences.
+    Services.prefs.setIntPref(PREF_SMART_BOOKMARKS_VERSION, 1);
+
+    rebuildSmartBookmarks();
+
+    // Count items.
+    do_check_eq(countFolderChildren(PlacesUtils.toolbarFolderId),
+                SMART_BOOKMARKS_ON_TOOLBAR + DEFAULT_BOOKMARKS_ON_TOOLBAR);
+    do_check_eq(countFolderChildren(PlacesUtils.bookmarksMenuFolderId),
+                SMART_BOOKMARKS_ON_MENU + DEFAULT_BOOKMARKS_ON_MENU);
+
+    // Check smart bookmarks are still in correct position.
+    itemId2 = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId, 0);
+    do_check_true(PlacesUtils.annotations.itemHasAnnotation(itemId2, SMART_BOOKMARKS_ANNO));
+    do_check_eq(PlacesUtils.bookmarks.getItemTitle(itemId2), secondItemTitle);
+
+    itemId1 = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId,
+                                                   PlacesUtils.bookmarks.DEFAULT_INDEX);
+    do_check_true(PlacesUtils.annotations.itemHasAnnotation(itemId1, SMART_BOOKMARKS_ANNO));
+    do_check_eq(PlacesUtils.bookmarks.getItemTitle(itemId1), firstItemTitle);
+
+    // Move back the smart bookmark to the original position.
+    PlacesUtils.bookmarks.moveItem(itemId1, PlacesUtils.bookmarksMenuFolderId, 1);
+
+    // Check version has been updated.
+    do_check_eq(Services.prefs.getIntPref(PREF_SMART_BOOKMARKS_VERSION),
+                SMART_BOOKMARKS_VERSION);
+
+    next_test();
+  }
+});
+
+//------------------------------------------------------------------------------
+
+tests.push({
   description: "An explicitly removed smart bookmark should not be recreated.",
   exec: function() {   
     // Remove toolbar's smart bookmarks
     PlacesUtils.bookmarks.removeItem(PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.toolbarFolderId, 0));
 
     // Sanity check items.
     do_check_eq(countFolderChildren(PlacesUtils.toolbarFolderId),
                 DEFAULT_BOOKMARKS_ON_TOOLBAR);
@@ -232,21 +337,16 @@ function next_test() {
   else {
     // Clean up database from all bookmarks.
     remove_all_bookmarks();
     do_test_finished();
   }
 }
 
 function run_test() {
-  // Bug 510219.
-  // Disabled on Windows due to almost permanent failure.
-  if ("@mozilla.org/windows-registry-key;1" in Components.classes)
-    return;
-
   do_test_pending();
 
   remove_bookmarks_html();
   remove_all_JSON_backups();
 
   // Initialize SuiteGlue, but remove its listener to places-init-complete.
   let bg = Cc["@mozilla.org/suite/suiteglue;1"].getService(Ci.nsIObserver);
   // Initialize Places.
--- a/suite/common/src/nsSuiteGlue.js
+++ b/suite/common/src/nsSuiteGlue.js
@@ -898,132 +898,128 @@ SuiteGlue.prototype = {
     // Get current smart bookmarks version.  If not set, create them.
     let smartBookmarksCurrentVersion = 0;
     try {
       smartBookmarksCurrentVersion = Services.prefs.getIntPref(SMART_BOOKMARKS_PREF);
     } catch(ex) {}
 
     // If version is current or smart bookmarks are disabled, just bail out.
     if (smartBookmarksCurrentVersion == -1 ||
-        smartBookmarksCurrentVersion >= SMART_BOOKMARKS_VERSION)
+        smartBookmarksCurrentVersion >= SMART_BOOKMARKS_VERSION) {
       return;
+    }
 
     let batch = {
       runBatched: function BG_EPDQI_runBatched() {
-        var smartBookmarks = [];
         let menuIndex = 0;
         let toolbarIndex = 0;
         let bundle = Services.strings.createBundle("chrome://communicator/locale/places/places.properties");
 
-        // MOST VISITED
-        var smart = {queryId: "MostVisited", // don't change this
-                     itemId: null,
-                     title: bundle.GetStringFromName("mostVisitedTitle"),
-                     uri: NetUtil.newURI("place:redirectsMode=" +
-                                    Components.interfaces.nsINavHistoryQueryOptions.REDIRECTS_MODE_TARGET +
-                                    "&sort=" +
-                                    Components.interfaces.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING +
-                                    "&maxResults=" + MAX_RESULTS),
-                     parent: PlacesUtils.toolbarFolderId,
-                     position: toolbarIndex++,
-                     newInVersion: 1 };
-        smartBookmarks.push(smart);
-
-        // RECENTLY BOOKMARKED
-        smart = {queryId: "RecentlyBookmarked", // don't change this
-                 itemId: null,
-                 title: bundle.GetStringFromName("recentlyBookmarkedTitle"),
-                 uri: NetUtil.newURI("place:folder=BOOKMARKS_MENU" +
+        let smartBookmarks = {
+          MostVisited: {
+            title: bundle.GetStringFromName("mostVisitedTitle"),
+            uri: NetUtil.newURI("place:redirectsMode=" +
+                                Components.interfaces.nsINavHistoryQueryOptions.REDIRECTS_MODE_TARGET +
+                                "&sort=" +
+                                Components.interfaces.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING +
+                                "&maxResults=" + MAX_RESULTS),
+            parent: PlacesUtils.toolbarFolderId,
+            position: toolbarIndex++,
+            newInVersion: 1
+          },
+          RecentlyBookmarked: {
+            title: bundle.GetStringFromName("recentlyBookmarkedTitle"),
+            uri: NetUtil.newURI("place:folder=BOOKMARKS_MENU" +
                                 "&folder=UNFILED_BOOKMARKS" +
                                 "&folder=TOOLBAR" +
                                 "&queryType=" +
                                 Components.interfaces.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS +
                                 "&sort=" +
                                 Components.interfaces.nsINavHistoryQueryOptions.SORT_BY_DATEADDED_DESCENDING +
                                 "&maxResults=" + MAX_RESULTS +
                                 "&excludeQueries=1"),
-                 parent: PlacesUtils.bookmarksMenuFolderId,
-                 position: menuIndex++,
-                 newInVersion: 1 };
-        smartBookmarks.push(smart);
-
-        // RECENT TAGS
-        smart = {queryId: "RecentTags", // don't change this
-                 itemId: null,
-                 title: bundle.GetStringFromName("recentTagsTitle"),
-                 uri: NetUtil.newURI("place:"+
-                    "type=" +
-                    Components.interfaces.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY +
-                    "&sort=" +
-                    Components.interfaces.nsINavHistoryQueryOptions.SORT_BY_LASTMODIFIED_DESCENDING +
-                    "&maxResults=" + MAX_RESULTS),
-                 parent: PlacesUtils.bookmarksMenuFolderId,
-                 position: menuIndex++,
-                 newInVersion: 1 };
-        smartBookmarks.push(smart);
+            parent: PlacesUtils.bookmarksMenuFolderId,
+            position: menuIndex++,
+            newInVersion: 1
+          },
+          RecentTags: {
+            title: bundle.GetStringFromName("recentTagsTitle"),
+            uri: NetUtil.newURI("place:"+
+                                "type=" +
+                                Components.interfaces.nsINavHistoryQueryOptions.RESULTS_AS_TAG_QUERY +
+                                "&sort=" +
+                                Components.interfaces.nsINavHistoryQueryOptions.SORT_BY_LASTMODIFIED_DESCENDING +
+                                "&maxResults=" + MAX_RESULTS),
+            parent: PlacesUtils.bookmarksMenuFolderId,
+            position: menuIndex++,
+            newInVersion: 1
+          }
+        };
 
         // Set current itemId, parent and position if Smart Bookmark exists,
         // we will use these informations to create the new version at the same
         // position.
         let smartBookmarkItemIds = PlacesUtils.annotations.getItemsWithAnnotation(SMART_BOOKMARKS_ANNO);
-        for each(var itemId in smartBookmarkItemIds) {
+        smartBookmarkItemIds.forEach(function (itemId) {
           let queryId = PlacesUtils.annotations.getItemAnnotation(itemId, SMART_BOOKMARKS_ANNO);
-          for (var i = 0; i < smartBookmarks.length; i++){
-            if (smartBookmarks[i].queryId == queryId) {
-              smartBookmarks[i].found = true;
-              smartBookmarks[i].itemId = itemId;
-              smartBookmarks[i].parent = PlacesUtils.bookmarks.getFolderIdForItem(itemId);
-              smartBookmarks[i].position = PlacesUtils.bookmarks.getItemIndex(itemId);
-              // Remove current item, since it will be replaced.
-              PlacesUtils.bookmarks.removeItem(itemId);
-              break;
-            }
+          if (queryId in smartBookmarks) {
+            let smartBookmark = smartBookmarks[queryId];
+            smartBookmark.itemId = itemId;
+            smartBookmark.parent = PlacesUtils.bookmarks.getFolderIdForItem(itemId);
+            smartBookmark.position = PlacesUtils.bookmarks.getItemIndex(itemId);
+          } else {
             // 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)
-              PlacesUtils.annotations.removeItemAnnotation(itemId, SMART_BOOKMARKS_ANNO);
+            PlacesUtils.annotations.removeItemAnnotation(itemId, SMART_BOOKMARKS_ANNO);
           }
-        }
+        });
 
-        // Create smart bookmarks.
-        for each(var smartBookmark in smartBookmarks) {
+        for (let queryId in smartBookmarks) {
+          let smartBookmark = smartBookmarks[queryId];
+
           // We update or create only changed or new smart bookmarks.
           // Also we respect user choices, so we won't try to create a smart
           // bookmark if it has been removed.
           if (smartBookmarksCurrentVersion > 0 &&
               smartBookmark.newInVersion <= smartBookmarksCurrentVersion &&
-              !smartBookmark.found)
+              !smartBookmark.itemId)
             continue;
 
+          // Remove old version of the smart bookmark if it exists, since it
+          // will be replaced in place.
+          if (smartBookmark.itemId) {
+            PlacesUtils.bookmarks.removeItem(smartBookmark.itemId);
+          }
+
           // Create the new smart bookmark and store its updated itemId.
           smartBookmark.itemId =
             PlacesUtils.bookmarks.insertBookmark(smartBookmark.parent,
                                                  smartBookmark.uri,
                                                  smartBookmark.position,
                                                  smartBookmark.title);
           PlacesUtils.annotations.setItemAnnotation(smartBookmark.itemId,
                                                     SMART_BOOKMARKS_ANNO,
-                                                    smartBookmark.queryId, 0,
+                                                    queryId, 0,
                                                     PlacesUtils.annotations.EXPIRE_NEVER);
         }
 
         // If we are creating all Smart Bookmarks from ground up, add a
         // separator below them in the bookmarks menu.
         if (smartBookmarksCurrentVersion == 0 &&
             smartBookmarkItemIds.length == 0) {
           let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.bookmarksMenuFolderId,
                                                         menuIndex);
           // Don't add a separator if the menu was empty or there is one already.
           if (id != -1 &&
               PlacesUtils.bookmarks.getItemType(id) != PlacesUtils.bookmarks.TYPE_SEPARATOR) {
             PlacesUtils.bookmarks.insertSeparator(PlacesUtils.bookmarksMenuFolderId,
                                                   menuIndex);
           }
-       }
+        }
       }
     };
 
     try {
       PlacesUtils.bookmarks.runInBatchMode(batch, null);
     }
     catch(ex) {
       Components.utils.reportError(ex);