Bug 374986 - Inappropriate commands are enabled for 'special' folders. r=sspitzer.

browser/components/places/content/controller.js

@@ -113,8 +113,7 @@ PlacesController.prototype = {
     case "cmd_cut":
     case "cmd_delete":
     case "placesCmd_moveBookmarks":
-      return !this.rootNodeIsSelected() && 
-             this._hasRemovableSelection();
+      return this._hasRemovableSelection();
     case "cmd_copy":
       return this._view.hasSelection;
     case "cmd_paste":
@@ -322,10 +321,19 @@ PlacesController.prototype = {
       return false;
 
     var nodes = this._view.getSelectionNodes();
-    var root = this._view.getResult().root;
+    var root = this._view.getResultNode();
 
+    var btFolderId = PlacesUtils.toolbarFolderId;
     for (var i = 0; i < nodes.length; ++i) {
-      var parent = nodes[i].parent || root;
+      // Disallow removing the view's root node 
+      if (nodes[i] == root)
+        return false;
+
+      // Disallow removing the toolbar folder
+      if (PlacesUtils.nodeIsFolder(nodes[i]) &&
+          asFolder(nodes[i]).folderId == btFolderId)
+        return false;
+
       // We don't call nodeIsReadOnly here, because nodeIsReadOnly means that
       // a node has children that cannot be edited, reordered or removed. Here,
       // we don't care if a node's children can't be reordered or edited, just
@@ -334,7 +342,8 @@ PlacesController.prototype = {
       // removable), but some special bookmark folders may have non-removable
       // children, e.g. live bookmark folder children. It doesn't make sense
       // to delete a child of a live bookmark folder, since when the folder
-      // refreshes, the child will return. 
+      // refreshes, the child will return.
+      var parent = nodes[i].parent || root;
       if (PlacesUtils.isReadonlyFolder(parent))
         return false;
     }
@@ -370,16 +379,7 @@ PlacesController.prototype = {
    * Determines whether or not nodes can be inserted relative to the selection.
    */
   _canInsert: function PC__canInsert() {
-    var nodes = this._view.getSelectionNodes();
-    var root = this._view.getResult().root;
-    for (var i = 0; i < nodes.length; ++i) {
-      var parent = nodes[i].parent || root;
-      if (PlacesUtils.nodeIsReadOnly(parent))
-        return false;
-    }
-    // Even if there's no selection, we need to check the root. Otherwise 
-    // commands may be enabled for history views when nothing is selected. 
-    return !PlacesUtils.nodeIsReadOnly(root);
+    return this._view.insertionPoint != null;
   },
 
   /**

browser/components/places/content/tree.xml

@@ -432,19 +432,23 @@
           //          then use getIndexOfNode to find your absolute index in
           //          the parent container instead. 
           //
-          var selection = this.view.selection;
+          var resultView = this.getResultView();
+          var selection = resultView.selection;
           var rc = selection.getRangeCount();
           var min = { }, max = { };
           selection.getRangeAt(rc - 1, min, max);
           
-          var resultView = this.getResultView();
-          
           // If the sole selection is an open container, insert into it rather 
           // than adjacent to it. Note that this only applies to _single_ 
           // selections - if the last element within a multi-selection is an
-          // open folder, insert _adajacent_ to the selection. 
-          if (this.hasSingleSelection && this.view.isContainer(max.value) && 
-              this.view.isContainerOpen(max.value))
+          // open folder, insert _adajacent_ to the selection.
+          //
+          // If the sole selection is the bookmarks toolbar folder, we nsert
+          // into it even if it is not opened
+          if (this.hasSingleSelection && resultView.isContainer(max.value) &&
+              (resultView.isContainerOpen(max.value) ||
+               resultView.nodeForTreeIndex(max.value)
+                         .folderId == PlacesUtils.bookmarksRootId))
             orientation = NHRVO.DROP_ON;
 
           return this._getInsertionPoint(max.value, orientation);
@@ -478,7 +482,12 @@
               index = orientation == NHRVO.DROP_BEFORE ? lsi : lsi + 1;
             }
           }
-          if (PlacesUtils.nodeIsFolder(container)) 
+
+          // * Disallow insertion of items under readonly folders
+          // * Disallow insertion of items under the places root
+          if (PlacesUtils.nodeIsFolder(container) &&
+              !PlacesUtils.nodeIsReadOnly(container) &&
+              container.folderId != PlacesUtils.placesRootId) 
             return new InsertionPoint(asFolder(container).folderId, index, orientation);
           return null;
         ]]></body>

browser/components/places/content/utils.js

@@ -1070,6 +1070,28 @@ var PlacesUtils = {
       }
     }
     return "";
+  },
+
+  // identifier getters for special folders
+  get placesRootId() {
+    if (!("_placesRootId" in this))
+      this._placesRootId = this.bookmarks.placesRoot;
+
+    return this._placesRootId;
+  },
+
+  get bookmarksRootId() {
+    if (!("_bookmarksRootId" in this))
+      this._bookmarksRootId = this.bookmarks.bookmarksRoot;
+
+    return this._bookmarksRootId;
+  },
+
+  get toolbarFolderId() {
+    if (!("_toolbarFolderId" in this))
+      this._toolbarFolderId = this.bookmarks.toolbarFolder;
+
+    return this._toolbarFolderId;
   }
 };