Bug 394252 - Unable to create a bookmark folder with Star menu. r=dietrich.
authormozilla.mano@sent.com
Thu, 13 Mar 2008 17:52:21 -0700
changeset 13041 ce720feb26ddf59156fef762fa4fa12060aed2e2
parent 13040 e407423119636dce03247470e71263bf32786e5b
child 13042 9833c895105893300eb87cf64e9acb3c4f78005c
push idunknown
push userunknown
push dateunknown
reviewersdietrich
bugs394252
milestone1.9b5pre
Bug 394252 - Unable to create a bookmark folder with Star menu. r=dietrich.
browser/base/content/browser-places.js
browser/components/places/content/editBookmarkOverlay.js
browser/components/places/content/editBookmarkOverlay.xul
browser/components/places/content/tree.xml
browser/components/places/content/treeView.js
--- a/browser/base/content/browser-places.js
+++ b/browser/base/content/browser-places.js
@@ -95,36 +95,42 @@ var StarUI = {
   },
 
   // nsIDOMEventListener
   handleEvent: function SU_handleEvent(aEvent) {
     switch (aEvent.type) {
       case "popuphidden":
         if (aEvent.originalTarget == this.panel) {
           if (!this._element("editBookmarkPanelContent").hidden)
-            gEditItemOverlay.uninitPanel(true);
+            this.quitEditMode();
           this._restoreCommandsState();
           this._itemId = -1;
           this._uri = null;
           if (this._batching) {
             PlacesUIUtils.ptm.endBatch();
             this._batching = false;
           }
         }
         break;
       case "keypress":
         if (aEvent.keyCode == KeyEvent.DOM_VK_ESCAPE) {
-          // In edit mode, the ESC key is mapped to the cancel button
-          if (!this._element("editBookmarkPanelContent").hidden)
-            this.cancelButtonOnCommand();
-          else // otherwise we just hide the panel
+          // In edit mode, if we're not editing a folder, the ESC key is mapped
+          // to the cancel button
+          if (!this._element("editBookmarkPanelContent").hidden) {
+            var elt = aEvent.target;
+            if (elt.localName != "tree" ||
+                (elt.localName == "tree" && !elt.hasAttribute("editing")))
+              this.cancelButtonOnCommand();
+          }
+        }
+        else if (aEvent.keyCode == KeyEvent.DOM_VK_RETURN) {
+          // hide the panel unless the folder tree is focused
+          if (aEvent.target.localName != "tree")
             this.panel.hidePopup();
         }
-        else if (aEvent.keyCode == KeyEvent.DOM_VK_RETURN)
-          this.panel.hidePopup(); // hide the panel
         break;
     }
   },
 
   _overlayLoaded: false,
   _overlayLoading: false,
   showEditBookmarkPopup:
   function SU_showEditBookmarkPopup(aItemId, aAnchorElement, aPosition) {
@@ -236,20 +242,16 @@ var StarUI = {
     this._element("editBookmarkPanelTitle").value =
       bundle.getString("editBookmarkPanel.pageBookmarkedTitle");
 
     // description
     this._element("editBookmarkPanelDescription").textContent =
       bundle.getFormattedString("editBookmarkPanel.pageBookmarkedDescription",
                                 [brandShortName]);
 
-    // hide the edit panel and the buttons below to it (Cancel, Done)
-    this._element("editBookmarkPanelContent").hidden = true;
-    this._element("editBookmarkPanelBottomButtons").hidden = true;
-
     // show the "Edit.." button and the Remove Bookmark button, hide the
     // undo-remove-bookmark button.
     this._element("editBookmarkPanelEditButton").hidden = false;
     this._element("editBookmarkPanelRemoveButton").hidden = false;
     this._element("editBookmarkPanelUndoRemoveButton").hidden = true;
 
     // unset the unstarred state, if set
     this._element("editBookmarkPanelStarIcon").removeAttribute("unstarred");
@@ -260,24 +262,33 @@ var StarUI = {
       this.panel.popupBoxObject
           .setConsumeRollupEvent(Ci.nsIPopupBoxObject.ROLLUP_CONSUME);
       this.panel.openPopup(aAnchorElement, aPosition, -1, -1);
     }
     else
       this.panel.focus();
   },
 
+  quitEditMode: function SU_quitEditMode() {
+    this._element("editBookmarkPanelContent").hidden = true;
+    this._element("editBookmarkPanelBottomButtons").hidden = true;
+    gEditItemOverlay.uninitPanel(true);
+  },
+
   editButtonCommand: function SU_editButtonCommand() {
     this.showEditBookmarkPopup();
   },
 
   cancelButtonOnCommand: function SU_cancelButtonOnCommand() {
+    // The order here is important! We have to hide the panel first, otherwise
+    // changes done as part of Undo may change the panel contents and by
+    // that force it to commit more transactions
+    this.panel.hidePopup();
     this.endBatch();
     PlacesUIUtils.ptm.undoTransaction();
-    this.panel.hidePopup();
   },
 
   removeBookmarkButtonCommand: function SU_removeBookmarkButtonCommand() {
 #ifdef ADVANCED_STARRING_UI
     // In minimal mode ("page bookmarked" notification), the bookmark
     // is removed and the panel is hidden immediately. In full edit mode,
     // a "Bookmark Removed" notification along with an Undo button is
     // shown
@@ -285,20 +296,22 @@ var StarUI = {
       PlacesUIUtils.ptm.endBatch();
       PlacesUIUtils.ptm.beginBatch(); // allow undo from within the notification
       var bundle = this._element("bundle_browser");
 
       // "Bookmark Removed" title (the description field is already empty in
       // this mode)
       this._element("editBookmarkPanelTitle").value =
         bundle.getString("editBookmarkPanel.bookmarkedRemovedTitle");
-      // hide the edit fields, the buttons below to and the remove bookmark
-      // button. Show the undo-remove-bookmark button.
-      this._element("editBookmarkPanelContent").hidden = true;
-      this._element("editBookmarkPanelBottomButtons").hidden = true;
+
+      // hide the edit panel
+      this.quitEditMode();
+
+      // Hide the remove bookmark button, show the undo-remove-bookmark
+      // button.
       this._element("editBookmarkPanelUndoRemoveButton").hidden = false;
       this._element("editBookmarkPanelRemoveButton").hidden = true;
       this._element("editBookmarkPanelStarIcon").setAttribute("unstarred", "true");
       this.panel.focus();
     }
 #endif
 
     // cache its uri so we can get the new itemId in the case of undo
--- a/browser/components/places/content/editBookmarkOverlay.js
+++ b/browser/components/places/content/editBookmarkOverlay.js
@@ -586,31 +586,35 @@ var gEditItemOverlay = {
   },
 
   toggleFolderTreeVisibility: function EIO_toggleFolderTreeVisibility() {
     var expander = this._element("foldersExpander");
     if (!this._folderTree.collapsed) {
       expander.className = "expander-down";
       expander.setAttribute("tooltiptext",
                             expander.getAttribute("tooltiptextdown"));
-      this._folderTree.collapsed = true;
+      this._folderTree.collapsed =
+        this._element("newFolderBox").collapsed = true;
       this._element("chooseFolderSeparator").hidden =
         this._element("chooseFolderMenuItem").hidden = false;
     }
     else {
       expander.className = "expander-up"
       expander.setAttribute("tooltiptext",
                             expander.getAttribute("tooltiptextup"));
-      this._folderTree.collapsed = false;
-      if (!this._folderTree.place) {
-        const FOLDER_TREE_PLACE_URI =
-          "place:excludeItems=1&excludeQueries=1&excludeReadOnlyFolders=1&folder=" +
-          window.top.PlacesUIUtils.allBookmarksFolderId;
-        this._folderTree.place = FOLDER_TREE_PLACE_URI;
-      }
+      this._folderTree.collapsed =
+        this._element("newFolderBox").collapsed = false;
+
+      // XXXmano: Ideally we would only do this once, but for some odd reason,
+      // the editable mode set on this tree, together with its collapsed state
+      // breaks the view.
+      const FOLDER_TREE_PLACE_URI =
+        "place:excludeItems=1&excludeQueries=1&excludeReadOnlyFolders=1&folder=" +
+        window.top.PlacesUIUtils.allBookmarksFolderId;
+      this._folderTree.place = FOLDER_TREE_PLACE_URI;
 
       this._element("chooseFolderSeparator").hidden =
         this._element("chooseFolderMenuItem").hidden = true;
       var currentFolder = this._getFolderIdFromMenuList();
       this._folderTree.selectItems([currentFolder]);
       this._folderTree.focus();
     }
   },
@@ -761,16 +765,36 @@ var gEditItemOverlay = {
       if (tags[i] == "") {
         tags.splice(i, 1);
         i--;
       }
     }
     return tags;
   },
 
+  newFolder: function EIO_newFolder() {
+    var ip = this._folderTree.insertionPoint;
+
+    // default to the bookmarks menu folder
+    if (ip.itemId == PlacesUIUtils.allBookmarksFolderId ||
+        ip.itemId == PlacesUIUtils.unfiledBookmarksFolderId) {
+      ip.itemId = PlacesUtils.bookmarksMenuFolderId;
+      ip.index = -1;
+    }
+
+    // XXXmano: add a separate "New Folder" string at some point...
+    var defaultLabel = this._element("newFolderButton").label;
+    var txn = PlacesUIUtils.ptm.createFolder(defaultLabel, ip.itemId, ip.index);
+    PlacesUtils.ptm.doTransaction(txn);
+    this._folderTree.focus();
+    this._folderTree.selectItems([this._lastNewItem]);
+    this._folderTree.startEditing(this._folderTree.view.selection.currentIndex,
+                                  this._folderTree.columns.getFirstColumn());
+  },
+
   // nsIDOMEventListener
   handleEvent: function EIO_nsIDOMEventListener(aEvent) {
     switch (aEvent.type) {
     case "CheckboxStateChange":
       // Update the tags field when items are checked/unchecked in the listbox
       var tags = this._getTagsArrayFromTagField();
 
       if (aEvent.target.checked)
@@ -864,14 +888,17 @@ var gEditItemOverlay = {
 
     var folderItem = this._getFolderMenuItem(aNewParent);
 
     // just setting selectItem _does not_ trigger oncommand, so we don't
     // recurse
     this._folderMenuList.selectedItem = folderItem;
   },
 
+  onItemAdded: function EIO_onItemAdded(aItemId, aFolder, aIndex) {
+    this._lastNewItem = aItemId;
+  },
+
   onBeginUpdateBatch: function() { },
   onEndUpdateBatch: function() { },
-  onItemAdded: function() { },
   onItemRemoved: function() { },
   onItemVisited: function() { },
 };
--- a/browser/components/places/content/editBookmarkOverlay.xul
+++ b/browser/components/places/content/editBookmarkOverlay.xul
@@ -30,19 +30,22 @@
 # 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 *****
 
+<!-- XXXmano: temporary also use moveBookmarks for the "New Folder" button -->
 <!DOCTYPE overlay [
-<!ENTITY % placesDTD SYSTEM "chrome://browser/locale/places/editBookmarkOverlay.dtd">
-%placesDTD;
+<!ENTITY % editBookmarkOverlayDTD SYSTEM "chrome://browser/locale/places/editBookmarkOverlay.dtd">
+%editBookmarkOverlayDTD;
+<!ENTITY % moveBookmarksDTD SYSTEM "chrome://browser/locale/places/moveBookmarks.dtd">
+%moveBookmarksDTD;
 ]>
 
 <?xml-stylesheet href="chrome://browser/skin/places/editBookmarkOverlay.css"?>
 <?xml-stylesheet href="chrome://browser/skin/places/places.css"?>
 
 <overlay id="editBookmarkOverlay"
          xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul">
 
@@ -119,46 +122,55 @@
             <menupopup>
               <!-- Static item for special folders -->
               <menuitem id="editBMPanel_unfiledRootItem"
                         class="menuitem-iconic folder-icon"/>
               <menuitem id="editBMPanel_bmRootItem"
                         class="menuitem-iconic folder-icon"/>
               <menuitem id="editBMPanel_toolbarFolderItem"
                         class="menuitem-iconic folder-icon"/>
-              <menuseparator id="editBMPanel_foldersSeparator" hidden="true"/>
               <menuseparator id="editBMPanel_chooseFolderSeparator"/>
               <menuitem id="editBMPanel_chooseFolderMenuItem"
 	        label="&editBookmarkOverlay.choose.label;"
                         class="menuitem-iconic folder-icon"/>
+              <menuseparator id="editBMPanel_foldersSeparator" hidden="true"/>
             </menupopup>
           </menulist>
           <button id="editBMPanel_foldersExpander"
                   class="expander-down"
                   tooltiptext="&editBookmarkOverlay.foldersExpanderDown.tooltip;"
                   tooltiptextdown="&editBookmarkOverlay.foldersExpanderDown.tooltip;"
                   tooltiptextup="&editBookmarkOverlay.expanderUp.tooltip;"
                   oncommand="gEditItemOverlay.toggleFolderTreeVisibility();"
                   observes="paneElementsBroadcaster"/>
         </row>
 
         <tree id="editBMPanel_folderTree"
               class="placesTree"
               type="places"
               height="150"
               collapsed="true"
+              editable="true"
               onselect="gEditItemOverlay.onFolderTreeSelect();"
               hidecolumnpicker="true"
               observes="paneElementsBroadcaster">
           <treecols>
             <treecol anonid="title" flex="1" primary="true" hideheader="true"/>
           </treecols>
           <treechildren flex="1"/>
         </tree>
 
+        <hbox id="editBMPanel_newFolderBox" collapsed="true">
+          <button label="&newFolderButton.label;"
+                  id="editBMPanel_newFolderButton"
+                  accesskey="&newFolderButton.accesskey;"
+                  oncommand="gEditItemOverlay.newFolder();"/>
+          <spacer flex="1"/>
+        </hbox>
+
         <row align="center" id="editBMPanel_tagsRow">
           <label value="&editBookmarkOverlay.tags.label;"
                  accesskey="&editBookmarkOverlay.tags.accesskey;"
                  control="editBMPanel_tagsField"
                  observes="paneElementsBroadcaster"/>
           <textbox id="editBMPanel_tagsField"
                    onblur="gEditItemOverlay.onTagsFieldBlur();"
                    observes="paneElementsBroadcaster"
--- a/browser/components/places/content/tree.xml
+++ b/browser/components/places/content/tree.xml
@@ -510,20 +510,16 @@
           if (index != -1) {
             var lastSelected = resultview.nodeForTreeIndex(index);
             if (resultview.isContainer(index) && orientation == Ci.nsITreeView.DROP_ON) {
               // If the last selected item is an open container, append _into_
               // it, rather than insert adjacent to it. 
               container = lastSelected;
               index = -1;
             }
-            else if (resultview.isSorted()) {
-              // If we are into a sorted view we should append at the end
-              index = -1;
-            }
             else if (!this._disallowInsertion(lastSelected) &&
                      lastSelected.containerOpen &&
                      orientation == Ci.nsITreeView.DROP_AFTER) {
              // If the last selected item is an open container and the user is
              // trying to drag into it as a first item, really insert into it.
              container = lastSelected;
              orientation = Ci.nsITreeView.DROP_BEFORE;
              index = 0;
@@ -534,18 +530,28 @@
               // insertion point.
               container = lastSelected.parent || container;
 
               // avoid the potentially expensive call to getIndexOfNode() 
               // if we know this container doesn't allow insertion
               if (this._disallowInsertion(container))
                 return null;
 
-              var lsi = PlacesUtils.getIndexOfNode(lastSelected);
-              index = orientation == Ci.nsITreeView.DROP_BEFORE ? lsi : lsi + 1;
+              var queryOptions = asQuery(result.root).queryOptions;
+              if (queryOptions.excludeItems || queryOptions.excludeQueries ||
+                  queryOptions.excludeReadOnlyFolders ||
+                  queryOptions.sortingMode != Ci.nsINavHistoryQueryOptions.SORT_BY_NONE) {
+                // If we are within either a sorted view or a view in which
+                // some items may be invisible, insert at the end
+                index = -1;   
+              }
+              else {
+                var lsi = PlacesUtils.getIndexOfNode(lastSelected);
+                index = orientation == Ci.nsITreeView.DROP_BEFORE ? lsi : lsi + 1;
+              }
             }
           }
 
           if (this._disallowInsertion(container))
             return null;
 
           return new InsertionPoint(PlacesUtils.getConcreteItemId(container),
                                     index, orientation);
--- a/browser/components/places/content/treeView.js
+++ b/browser/components/places/content/treeView.js
@@ -1337,21 +1337,43 @@ PlacesTreeView.prototype = {
         break;
       default:
         throw Cr.NS_ERROR_INVALID_ARG;
     }
     this._result.sortingAnnotation = newSortingAnnotation;
     this._result.sortingMode = newSort;
   },
 
+  isEditable: function PTV_isEditable(aRow, aColumn) {
+    // At this point we only support editing the title field.
+    if (aColumn.index != 0)
+      return false;
+
+    var node = this.nodeForTreeIndex(aRow);
+    if (!PlacesUtils.nodeIsReadOnly(node) &&
+        (PlacesUtils.nodeIsFolder(node) ||
+         (PlacesUtils.nodeIsBookmark(node) &&
+          !PlacesUtils.nodeIsLivemarkItem(node))))
+      return true;
+
+    return false;
+  },
+
+  setCellText: function PTV_setCellText(aRow, aColumn, aText) {
+    // we may only get here if the cell is editable
+    var node = this.nodeForTreeIndex(aRow);
+    if (node.title != aText) {
+      var txn = PlacesUIUtils.ptm.editItemTitle(node.itemId, aText);
+      PlacesUtils.ptm.doTransaction(txn);
+    }
+  },
+
   selectionChanged: function() { },
   cycleCell: function PTV_cycleCell(aRow, aColumn) { },
-  isEditable: function(aRow, aColumn) { return false; },
   isSelectable: function(aRow, aColumn) { return false; },
-  setCellText: function(aRow, aColumn) { },
   performAction: function(aAction) { },
   performActionOnRow: function(aAction, aRow) { },
   performActionOnCell: function(aAction, aRow, aColumn) { }
 };
 
 function PlacesTreeView(aShowRoot, aFlatList, aOnOpenFlatContainer) {
   if (aShowRoot && aFlatList)
     throw("Flat-list mode is not supported when show-root is set");