Bug 325342, implement places sort bookmark folder by name command, r=dietrich,mano

browser/components/places/content/controller.js

@@ -205,6 +205,11 @@ PlacesController.prototype = {
 #endif
       }
       return false;
+    case "placesCmd_sortBy:name":
+      var selectedNode = this._view.selectedNode;
+      return selectedNode &&
+             PlacesUtils.nodeIsFolder(selectedNode) &&
+             !PlacesUtils.nodeIsReadOnly(selectedNode);
     case "placesCmd_setAsBookmarksToolbarFolder":
       if (this._view.hasSingleSelection) {
         var selectedNode = this._view.selectedNode;
@@ -297,6 +302,9 @@ PlacesController.prototype = {
     case "placesCmd_reload":
       this.reloadSelectedLivemarks();
       break;
+    case "placesCmd_sortBy:name":
+      this.sortFolderByName();
+      break;
     case "placesCmd_setAsBookmarksToolbarFolder":
       this.setBookmarksToolbarFolder();
       break;
@@ -397,71 +405,6 @@ PlacesController.prototype = {
     return false;
   },
 
-#ifdef BROKEN_SORT_CODE
-  /**
-   * Updates commands for persistent sorting
-   * @param   inSysArea
-   *          true if the selection intersects the read only "system" area.
-   * @param   hasSingleSelection
-   *          true if only one item is selected in the view
-   * @param   selectedNode
-   *          The selected nsINavHistoryResultNode
-   * @param   canInsert
-   *          true if the item is a writable container that can be inserted 
-   *          into
-   */
-  _updateSortCommands: 
-  function PC__updateSortCommands(inSysArea, hasSingleSelection, selectedNode, 
-                                  canInsert) {
-    // Some views, like menupopups, destroy their result as they hide, but they
-    // are still the "last-active" view. Don't barf. 
-    var result = this._view.getResult();
-    var viewIsFolder = result ? PlacesUtils.nodeIsFolder(result.root) : false;
-
-    // Depending on the selection, the persistent sort command sorts the 
-    // contents of the current folder (when the selection is mixed or leaf 
-    // items like individual bookmarks are selected) or the contents of the
-    // selected folder (if a single folder is selected).     
-    var sortingChildren = false;
-    var name = result.root.title;
-    var sortFolder = result.root;
-    if (selectedNode && selectedNode.parent) {
-      name = selectedNode.parent.title;
-      sortFolder = selectedNode.parent;
-    }
-    if (hasSingleSelection && PlacesUtils.nodeIsFolder(selectedNode)) {
-      name = selectedNode.title;
-      sortFolder = selectedNode;
-      sortingChildren = true;
-    }
-
-    // Count the children of the container. If there aren't at least two, we 
-    // don't want to enable the command since there's nothing to be sorted.
-    // We need to get the unfiltered contents of the container to make this
-    // determination, which means a new query, since the existing query may
-    // be filtered (e.g. left list). 
-    var enoughChildrenToSort = false;
-    if (PlacesUtils.nodeIsFolder(sortFolder)) {
-      var folder = asFolder(sortFolder);
-      var contents = this.getFolderContents(folder.folderId, false, false);
-      enoughChildrenToSort = contents.childCount > 1;
-    }
-    var metadata = this._buildSelectionMetadata();
-    this._setEnabled("placesCmd_sortby:name", 
-      (sortingChildren || !inSysArea) && canInsert && viewIsFolder && 
-      !("mixed" in metadata) && enoughChildrenToSort);
-
-    var command = document.getElementById("placesCmd_sortby:name");
-    
-    if (name) {
-      command.setAttribute("label", 
-        PlacesUtils.getFormattedString("sortByName", [name]));
-    }
-    else
-      command.setAttribute("label", PlacesUtils.getString("sortByNameGeneric"));
-  },
-#endif
-
   /**
    * Looks at the data on the clipboard to see if it is paste-able. 
    * Paste-able data is:
@@ -1042,6 +985,15 @@ PlacesController.prototype = {
                       this._view.getSelectionNodes(), PlacesUtils.tm);
   },
 
+  /**
+   * Sort the selected folder by name
+   */
+  sortFolderByName: function PC_sortFolderByName() {
+    var selectedNode = this._view.selectedNode;
+    var txn = new PlacesSortFolderByNameTransaction(selectedNode.folderId, selectedNode.bookmarkIndex);
+    PlacesUtils.tm.doTransaction(txn);
+  },
+
   /**
    * Makes the selected node the bookmarks toolbar folder.
    */
@@ -2225,6 +2177,47 @@ PlacesEditBookmarkMicrosummaryTransaction.prototype = {
   }
 };
 
+/**
+ * Sort a folder by name
+ */
+function PlacesSortFolderByNameTransaction(aFolderId, aFolderIndex) {
+  this._folderId = aFolderId;
+  this._folderIndex = aFolderIndex;
+  this._oldOrder = null,
+  this.redoTransaction = this.doTransaction;
+}
+PlacesSortFolderByNameTransaction.prototype = {
+  __proto__: PlacesBaseTransaction.prototype,
+
+  doTransaction: function PSSFBN_doTransaction() {
+    this._oldOrder = [];
+
+    var items = [];
+    var contents = this.utils.getFolderContents(this._folderId, false, false);
+    var count = contents.childCount;
+    for (var i = 0; i < count; ++i) {
+      var item = contents.getChild(i);
+      this._oldOrder[item.itemId] = i;
+      items.push(item);
+    }
+
+    function sortItems(a, b) {
+      var atitle = a.title;
+      var btitle = b.title;
+      return (atitle == btitle) ? 0 : ((atitle < btitle) ? -1 : 1);
+    }
+    items.sort(sortItems);
+
+    for (var i = 0; i < count; ++i)
+      this.bookmarks.setItemIndex(items[i].itemId, i);
+  },
+
+  undoTransaction: function PSSFBN_undoTransaction() {
+    for (item in this._oldOrder)
+      this.bookmarks.setItemIndex(item, this._oldOrder[item]);
+  }
+};
+
 /**
  * Set the bookmarks toolbar folder.
  */
@@ -2259,6 +2252,6 @@ function goUpdatePlacesCommands() {
   goUpdateCommand("placesCmd_moveBookmarks");
   goUpdateCommand("placesCmd_setAsBookmarksToolbarFolder");
   goUpdateCommand("placesCmd_reload");
-  // XXXmano todo: sort commands handling
+  goUpdateCommand("placesCmd_sortBy:name");
 #endif
 }

browser/components/places/content/history-panel.js

@@ -80,7 +80,7 @@ function initContextMenu() {
                             "placesContext_new:separator",
                             "placesContext_cut",
                             "placesContext_paste",
-                            "placesContext_sortby:name"];
+                            "placesContext_sortBy:name"];
   for (var i=0; i < alwaysHideElements.length; i++) {
     var elt = document.getElementById(alwaysHideElements[i]);
     elt.removeAttribute("selection");

browser/components/places/content/placesOverlay.xul

@@ -89,10 +89,8 @@
              observes="placesCmd_show:info"/>
     <command id="placesCmd_reload"
              oncommand="goDoCommand('placesCmd_reload');"/>
-
-    <!-- Bug 325342 -->
-    <command id="placesCmd_sortby:name" disabled="true"/>
-
+    <command id="placesCmd_sortBy:name"
+             oncommand="goDoCommand('placesCmd_sortBy:name');"/>
     <command id="placesCmd_moveBookmarks"
              oncommand="goDoCommand('placesCmd_moveBookmarks');"/>
     <command id="placesCmd_setAsBookmarksToolbarFolder"
@@ -181,8 +179,8 @@
               label="&cmd.reloadLivebookmark.label;"
               accesskey="&cmd.reloadLivebookmark.accesskey;"
               selection="livemark/feedURI|allLivemarks"/>
-    <menuitem id="placesContext_sortby:name"
-              command="placesCmd_sortby:name"
+    <menuitem id="placesContext_sortBy:name"
+              command="placesCmd_sortBy:name"
               label="&cmd.sortby_name.label;"
               accesskey="&cmd.context_sortby_name.accesskey;"
               selection="mutable"/>

toolkit/components/places/public/nsINavBookmarksService.idl

@@ -54,7 +54,7 @@ interface nsITransaction;
  * Observer for bookmark changes.
  */
 
-[scriptable, uuid(b3576cd9-d6f0-4faa-9b5d-cf7a9204dce9)]
+[scriptable, uuid(7A553BA3-5381-4A66-95D9-C7BA3895603C)]
 interface nsINavBookmarkObserver : nsISupports
 {
   /**
@@ -364,6 +364,17 @@ interface nsINavBookmarksService : nsISupports
    */
   long getItemIndex(in long long aItemId);
 
+  /**
+   * Changes the index for a item. This method does not change the indices of
+   * any other items in the same folder, so ensure that the new index does not
+   * already exist, or change the index of other items accordingly, otherwise
+   * the indices will become corrupted.
+   *
+   *  @param aItemId    The id of the item to modify
+   *  @param aNewIndex  The new index
+   */
+  void setItemIndex(in long long aItemId, in long aNewIndex);
+
   /**
    * Get the place: url for a bookmark folder. You can use this value to
    * get/set the icon for a folder. Its important that you use this function

toolkit/components/places/src/nsNavBookmarks.cpp

@@ -2028,6 +2028,49 @@ nsNavBookmarks::GetItemIndex(PRInt64 aItemId, PRInt32 *aIndex)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsNavBookmarks::SetItemIndex(PRInt64 aItemId, PRInt32 aNewIndex)
+{
+  nsresult rv;
+  PRInt32 oldIndex = 0;
+  PRInt64 parent = 0;
+
+  {
+    mozStorageStatementScoper scopeGet(mDBGetItemProperties);
+    rv = mDBGetItemProperties->BindInt64Parameter(0, aItemId);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    PRBool results;
+    rv = mDBGetItemProperties->ExecuteStep(&results);
+    NS_ENSURE_SUCCESS(rv, rv);
+    if (!results)
+      return NS_OK;
+
+    oldIndex = mDBGetItemProperties->AsInt32(kGetItemPropertiesIndex_Position);
+    parent = mDBGetItemProperties->AsInt64(kGetItemPropertiesIndex_Parent);
+  }
+
+  nsCOMPtr<mozIStorageStatement> statement;
+  rv = DBConn()->CreateStatement(NS_LITERAL_CSTRING("UPDATE moz_bookmarks SET position = ?2 WHERE id = ?1"),
+                               getter_AddRefs(statement));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = statement->BindInt64Parameter(0, aItemId);
+  NS_ENSURE_SUCCESS(rv, rv);
+  rv = statement->BindInt32Parameter(1, aNewIndex);
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  rv = statement->Execute();
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver,
+                      OnItemRemoved(aItemId, parent, oldIndex))
+  ENUMERATE_WEAKARRAY(mObservers, nsINavBookmarkObserver,
+                      OnItemAdded(aItemId, parent, aNewIndex))
+
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsNavBookmarks::SetKeywordForBookmark(PRInt64 aBookmarkId, const nsAString& aKeyword)
 {