* Allows the user to access Bookmark Properties dialog from context menus.

* Adds support for editing the bookmark shortcut (formerly keyword) in the Bookmark Properties dialog.
* Improves the appearance of the Bookmark Properties dialog.
* Removes dead code from PlacesBrowserShim in browser.js

NOTE: This doesn't reflect the final UI for this functionality; rather,
this change is intended to add functionality for users of the nightlies.

bug=322988
r=annie.sullivan@gmail.com
sr=bugs@bengoodger.com

browser/base/content/browser.js

@@ -36,6 +36,7 @@
 #   Giorgio Maone <g.maone@informaction.com>
 #   Tom Germeau <tom.germeau@epigoon.com>
 #   Jesse Ruderman <jruderman@gmail.com>
+#   Joe Hughes <joe@retrovirus.com>
 #
 # 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
@@ -6263,8 +6264,6 @@ var PlacesBrowserShim = {
   
   // XXXben: these should die
   _currentURI: null, // URI of the bookmark being modified
-  _assignableFolderResult: null, // root of user-writable folders
-  MAX_INDENT_DEPTH: 6, // maximum indentation level of "tag" display
 
   init: function PBS_init() {
     this._bms =
@@ -6311,23 +6310,6 @@ var PlacesBrowserShim = {
     PlacesController.tm = PlacesTransactionManager;
   },
 
-  /**
-   * This method creates a query for the set of assignable folders.
-   * This only needs to be created once; when closed (using
-   * root.containerOpen = false) and reopened, the results will be regenerated
-   * if the data has changed since the close.
-   * XXXben - why is this done during startup?!
-   */
-  _initAssignableFolderResult: function PBS__initAssignableFolderRoot() {
-    var query = this._hist.getNewQuery();
-    query.setFolders([this._bms.placesRoot], 1);
-    var options = this._hist.getNewQueryOptions();
-    options.setGroupingMode([Ci.nsINavHistoryQueryOptions.GROUP_BY_FOLDER], 1);
-    options.excludeItems = true;
-
-    this._assignableFolderResult = this._hist.executeQuery(query, options);
-  },
-
   addBookmark: function PBS_addBookmark() {
     var selectedBrowser = getBrowser().selectedBrowser;
     this._bookmarkURI(this._bms.bookmarksRoot, selectedBrowser.currentURI, 
@@ -6378,7 +6360,7 @@ var PlacesBrowserShim = {
   /**
    * Gets the URI that the visible browser tab is rendering.
    *
-   * @returns a string containing the URI currently being shown
+   * @returns an nsIURI object representing the URI currently being shown
    */
   _getCurrentLocation: function PBS__getCurrentLocation() {
     return getBrowser().selectedBrowser.webNavigation.currentURI;
@@ -6391,10 +6373,13 @@ var PlacesBrowserShim = {
   _updateControlStates: function PBS__updateControlStates() {
     var bookmarkButton = document.getElementById("places-bookmark");
     if (bookmarkButton) {
-      if (this._bms.isBookmarked(this._getCurrentLocation()))
+      if (this._bms.isBookmarked(this._getCurrentLocation())) {
         bookmarkButton.label = this._strings.getString("locationStatusBookmarked");
-      else
+        bookmarkButton.setAttribute("bookmarked", "true");
+      } else {
         bookmarkButton.label = this._strings.getString("locationStatusNotBookmarked");
+        bookmarkButton.setAttribute("bookmarked", "false");
+      }
     }
 
     var feedButton = document.getElementById("places-subscribe");
@@ -6424,37 +6409,6 @@ var PlacesBrowserShim = {
     getBrowser().mTabBox.addEventListener("select", onTabSwitch, true);
   },
 
-  /**
-   * Prepares the bookmark properties dialog for display; should be called
-   * from the dialog's onload handler with a reference to the dialog's
-   * DOM window object.
-   */
-  prepareBookmarkDialog: function PBS_prepareBookmarkDialog(dialogWindow) {
-    this.populateProperties(dialogWindow.document);
-    this.sizeAndPositionBookmarkDialog(dialogWindow);
-  },
-
-  sizeAndPositionBookmarkDialog: function PBS_sizeAndPositionBookmarkDialog(childWindow) {
-    var urlbar = document.getElementById("urlbar");
-    var editUrlbar = childWindow.document.getElementById("edit-urlbar");
-
-    var newx = Math.max(0, urlbar.boxObject.x + window.screenX - editUrlbar.boxObject.x);
-    var newy = urlbar.boxObject.y + window.screenY - editUrlbar.boxObject.y;
-    childWindow.moveTo(newx, newy);
-
-    var childDoc = childWindow.document;
-
-    var tagbox = childDoc.getElementById("tagbox");
-    tagbox.style.overflow="auto";
-
-    var pio = childDoc.getElementById("places-info-options");
-    var pig = childDoc.getElementById("places-info-grid");
-    childDoc.documentElement.getButton("accept").hidden=true;
-
-    var newHeight = pio.boxObject.y + pio.boxObject.height + 5;
-    childWindow.resizeTo(childWindow.innerWidth, newHeight);
-  },
-
   /**
    * This method should be called when the location currently being
    * rendered by a browser changes (loading new page or forward/back).
@@ -6475,8 +6429,6 @@ var PlacesBrowserShim = {
     this._updateControlStates();
   },
 
-  ///////////////// ALL THIS NEEDS TO MOVE TO SEPARATE DIALOG SCRIPT FILE! --->
-  
   /**
    * This method should be called when the bookmark button is clicked.
    */
@@ -6490,47 +6442,6 @@ var PlacesBrowserShim = {
     }
   },
 
-  populateProperties: function PBS_populateProperties(document, location, title) {
-    if (!location) {
-      location = this._currentURI;
-      title = this._currentTitle;
-    }
-
-    var nurl = document.getElementById("edit-urlbar");
-
-    var titlebox = document.getElementById("edit-titlebox");
-
-    nurl.value = location.spec;
-    titlebox.value = title;
-
-    var tagArea = document.getElementById("tagbox");
-
-    while (tagArea.hasChildNodes()) {
-      tagArea.removeChild(tagArea.firstChild);
-    }
-
-    var elementDict = {};
-
-    var root = this._assignableFolderResult.root; //Root is always a container.
-    root.containerOpen = true;
-    this._populateTags(root, 0, tagArea, elementDict);
-    root.containerOpen = false;
-
-    var categories = this._bms.getBookmarkFolders(location, {});
-
-    this._updateFolderTextbox(document, location);
-
-    var length = 0;
-    for (key in elementDict) {
-      length++;
-    }
-
-    for (var i=0; i < categories.length; i++) {
-      var elm = elementDict[categories[i]];
-      elm.setAttribute("selected", "true");
-    }
-  },
-
   /**
    * This method shows the bookmark properties dialog.  If bookmarkURI
    * is undefined, the dialog with display properties for the URI of the
@@ -6546,229 +6457,11 @@ var PlacesBrowserShim = {
     ASSERT(this._bms.isBookmarked(this._currentURI), "showBookmarkProperties() was called on a URI that hadn't been bookmarked: " + this._currentURI.spec);
 
     this._currentTitle = this._bms.getItemTitle(this._currentURI);
-    window.openDialog("chrome://browser/content/places/bookmarkProperties.xul", "bookmarkproperties", "width=600,height=400,chrome,dependent,modal,resizable");
+    window.openDialog("chrome://browser/content/places/bookmarkProperties.xul",
+                      "bookmarkproperties",
+                      "width=600,height=400,chrome,dependent,modal,resizable",
+                      this._currentURI, PlacesController);
   },
-
-  /**
-   * This method is called to exit the Bookmark Properties panel.
-   *
-   * @param aSaveChanges boolean, should be true if changes performed while
-   *                     the panel was active should be saved
-   * @param document the document containing the fields needing to be saved
-   */
-  hideBookmarkProperties:
-  function PBS_hideBookmarkProperties(saveChanges, document) {
-    if (saveChanges) {
-      var titlebox = document.getElementById("edit-titlebox");
-      this._bms.setItemTitle(this._currentURI, titlebox.value);
-
-      var urlbox = document.getElementById("edit-urlbar");
-      if (urlbox.value != this._currentURI.spec) {
-        // TODO delete existing bookmark, create new one with same folder/locations
-      }
-    }
-
-    this._updateControlStates();
-  },
-
-
-  /**
-   * This method deletes the bookmark corresponding to the URI stored
-   * in _currentURI.  _currentURI represents the URI that the Bookmark
-   * Properties panel is currently viewing/editing.  Therefore, this method
-   * is only relevant in when the Bookmark Properties panel is active.
-   */
-  deleteBookmark: function PBS_deleteBookmark() {
-    if (!this._currentURI)
-      return;
-
-    var folders = this._bms.getBookmarkFolders(this._currentURI, {});
-    if (folders.length == 0)
-      return;
-
-    this._bms.beginUpdateBatch();
-    for (var i = 0; i < folders.length; i++) {
-      this._bms.removeItem(folders[i], this._currentURI);
-    }
-    this._bms.endUpdateBatch();
-  },
-
-  /**
-   * This method implements the "Show all bookmarks" action
-   * in the Bookmark Properties dialog.
-   */
-  dialogShowBookmarks: function PBS_dialogShowBookmarks(dialogWindow) {
-    this.hideBookmarkProperties(true, dialogWindow.document);
-    dialogWindow.close();
-    this.showBookmarks();
-  },
-
- /**
-   * This method implements the "Delete Bookmark" action
-   * in the Bookmark Properties dialog.
-   */
-  dialogDeleteBookmark: function PBS_dialogDeleteBookmark(dialogWindow) {
-    this.deleteBookmark();
-    this.hideBookmarkProperties(false, dialogWindow.document);
-    dialogWindow.close();
-  },
-
- /**
-   * This method implements the "Done" action
-   * in the Bookmark Properties dialog.
-   */
-  dialogDone: function PBS_dialogDone(dialogWindow) {
-    this.hideBookmarkProperties(true, dialogWindow.document);
-    dialogWindow.close();
-  },
-
-  /**
-   * This method sets the contents of the "Folders" textbox in the
-   * Bookmark Properties panel.
-   *
-   * @param document the document containing the textbox element
-   * @param uri an nsIURI object representing the current bookmark's URI
-   */
-  _updateFolderTextbox: function PBS__updateFolderTextbox(document, uri) {
-    var folderTextbox = document.getElementById("places-folder-list");
-    folderTextbox.value = this._getFolderNameListForURI(uri);
-  },
-
-  /**
-   * This method gets the list of folders that contain the current bookmark.
-   *
-   * @param aURI a nsIURI object representing the URI of the current bookmark
-   *
-   * @returns a comma-separated list of folder names in string form
-   */
-  _getFolderNameListForURI: function PBS__getFolderNameListForURI(uri) {
-    var folders = this._bms.getBookmarkFolders(uri, {});
-    var results = [];
-    for (var i = 0; i < folders.length; i++) {
-      results.push(this._bms.getFolderTitle(folders[i]));
-    }
-    return results.join(", ");
-  },
-
-  /**
-   * Recursively populates the tag-like set of clickable folders.
-   *
-   * @param aContainer a reference to an nsINavHistoryContainerResultNode
-   *        (whose) containerOpen property is set to true) representing
-   *        the roote of the bookmark folder tree
-   * @param aDepth the current iteration depth -- pass this 0 at the top level.
-   *        This only affects the visual indentation level of the tag display.
-   * @param aParentElement a vbox element into which the tags will be populated
-   * @param aElementDict a dictionary mapping folder IDs to element references
-   *        to be populated in this method
-   *
-   * @returns none
-   */
-  _populateTags:
-  function PBS__populateTags (container, depth, parentElement, elementDict) {
-    ASSERT(container.containerOpen, "The containerOpen property of the container parameter should be set to true before calling populateTags(), and then set to false again afterwards.");
-
-    var row = null;
-    for (var i = 0; i < container.childCount; i++) {
-      var childNode = container.getChild(i);
-
-      if (childNode.type != childNode.RESULT_TYPE_FOLDER)
-        continue;
-
-      var childFolder =
-        childNode.QueryInterface(Ci.nsINavHistoryFolderResultNode);
-      childFolder.containerOpen = true;
-
-      // If we can't alter it, no use showing it as an option.
-
-      // childFolder.childrenReadOnly currently returns wrong answer for
-      // livemarks (joe@retrovirus.com 2006-02-14)
-        //      if (childFolder.childrenReadOnly) {
-      if (this._bms.getFolderReadonly(childFolder.folderId)) {
-        childFolder.containerOpen = false;
-        continue;
-      }
-
-      if (childFolder.hasChildren) {
-        row = document.createElement("hbox");
-        row.setAttribute("class", "l" + depth);
-        var tag = this._createTagElement(childFolder, false);
-        elementDict[childFolder.folderId] = tag;
-        tag.setAttribute("isparent", "true");
-        row.appendChild(tag);
-        parentElement.appendChild(row);
-        row = null;
-        var nextDepth = depth + 1;
-        // We're limiting max indentation level here.
-        if (nextDepth > this.MAX_INDENT_DEPTH)
-          nextDepth = this.MAX_INDENT_DEPTH;
-        this._populateTags(childFolder, nextDepth, parentElement, elementDict);
-      } else {
-        if (row == null) {
-          row = document.createElement("description");
-          row.setAttribute("class", "l" + depth);
-          parentElement.appendChild(row);
-        } else {
-          // we now know that there must"ve been a tag before us on the same row
-          var separator = document.createElement("label");
-          separator.setAttribute("value", eval("\"\\u2022\"")); // bullet
-          separator.setAttribute("class", "tag-separator");
-          row.appendChild(separator);
-        }
-        var tag = this._createTagElement(childFolder, false);
-        elementDict[childFolder.folderId] = tag;
-        row.appendChild(tag);
-      }
-      childFolder.containerOpen = false;
-    }
-  },
-
-  /**
-   * This method creates a XUL element to represent a given Bookmark
-   * folder node.
-   *
-   * @param aNode an nsINavHistoryFolderResultNode object
-   * @param aIsSelected boolean, true if the given folder is currently selected
-   *
-   * @return a new XUL element corresponding to aNode
-   */
-  _createTagElement: function PBS_createTagElement(node, isSelected) {
-    var tag = document.createElement("label");
-    tag.setAttribute("value", node.title);
-    tag.setAttribute("folderid", node.folderId);
-    tag.setAttribute("selected", "" + isSelected);
-    var self = this;
-    function onClick(e) {
-      self.tagClicked(e);
-    }
-    tag.addEventListener("command", onClick, false);
-    // We need the click event handler until we change the element from labels
-    // to something like checkboxes.
-    tag.addEventListener("click", onClick, false);
-    tag.setAttribute("class", "tag");
-    return tag;
-  },
-
-  /**
-   * This method should be called when a tag element generated by
-   * _createTagElement is clicked by the user.
-   */
-  tagClicked: function PBS_tagClicked(event) {
-    var tagElement = event.target;
-
-    var folderId = parseInt(tagElement.getAttribute("folderid"));
-
-    if (tagElement.getAttribute("selected") == "true") {
-      this._bms.removeItem(folderId, this._currentURI);
-      tagElement.setAttribute("selected", "false");
-    } else {
-      this._bms.insertItem(folderId, this._currentURI, -1);
-      tagElement.setAttribute("selected", "true");
-    }
-
-    this._updateFolderTextbox(tagElement.ownerDocument, this._currentURI);
-  },
-
 };
 
 

browser/components/places/content/bookmarkProperties.js

@@ -0,0 +1,418 @@
+//* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* ***** 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 the Places Bookmark Properties.
+ *
+ * The Initial Developer of the Original Code is Google Inc.
+ * Portions created by the Initial Developer are Copyright (C) 2006
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Joe Hughes <jhughes@google.com>
+ *
+ * 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 ***** */
+
+#include controller.js
+
+var BookmarkPropertiesPanel = {
+
+  /**
+   * The Bookmarks Service.
+   */
+  __bms: null,
+  get _bms() {
+    if (!this.__bms) {
+      this.__bms =
+        Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
+        getService(Ci.nsINavBookmarksService);
+    }
+    return this.__bms;
+  },
+
+  /**
+   * The Nav History Service.
+   */
+  __hist: null,
+  get _hist() {
+    if (!this.__hist) {
+      this.__hist =
+        Cc["@mozilla.org/browser/nav-history-service;1"].
+        getService(Ci.nsINavHistoryService);
+    }
+    return this.__hist;
+  },
+
+  /**
+   * The I/O Service, useful for creating nsIURIs from strings.
+   */
+  __ios: null,
+  get _ios() {
+    if (!this.__ios) {
+      this.__ios =
+        Cc["@mozilla.org/network/io-service;1"].
+        getService(Ci.nsIIOService);
+    }
+    return this.__ios;
+  },
+
+
+  _bookmarkURI: null,
+  _bookmarkTitle: "",
+  _dialogWindow: null,
+  _parentWindow: null,
+  _controller: null,
+  MAX_INDENT_DEPTH: 6, // maximum indentation level of "tag" display
+
+  /**
+   * This method can be run on a URI parameter to ensure that it didn't
+   * receive a string instead of an nsIURI object.
+   */
+  _assertURINotString: function PC__assertURINotString(value) {
+    ASSERT((typeof(value) == "object") && !(value instanceof String),
+    "This method should be passed a URI as a nsIURI object, not as a string.");
+  },
+
+  /**
+   * This method should be called by the onload of the Bookmark Properties
+   * dialog to initialize the state of the panel.
+   *
+   * @param bookmarkURI  a nsIURI object representing the bookmarked URI that
+   *                     we want to view the properties of
+   * @param dialogWindow the window object of the Bookmark Properties dialog
+   * @param controller   a PlacesController object for interacting with the
+   *                     Places system
+   */
+  init: function BPP_init(dialogWindow, bookmarkURI, controller) {
+    this._assertURINotString(bookmarkURI);
+
+    this._bookmarkURI = bookmarkURI;
+    this._bookmarkTitle = this._bms.getItemTitle(this._bookmarkURI);
+    this._dialogWindow = dialogWindow;
+    this._controller = controller;
+
+    this._initAssignableFolderResult();
+    this._populateProperties();
+    this._updateSize();
+  },
+
+  /**
+   * This method creates a query for the set of assignable folders.
+   * This only needs to be created once; when closed (using
+   * root.containerOpen = false) and reopened, the results will be regenerated
+   * if the data has changed since the close.
+   */
+  _initAssignableFolderResult: function BPP__initAssignableFolderRoot() {
+    var query = this._hist.getNewQuery();
+    query.setFolders([this._bms.placesRoot], 1);
+    var options = this._hist.getNewQueryOptions();
+    options.setGroupingMode([Ci.nsINavHistoryQueryOptions.GROUP_BY_FOLDER], 1);
+    options.excludeItems = true;
+
+    this._assignableFolderResult = this._hist.executeQuery(query, options);
+  },
+
+  /**
+   * This method fills in the data values for the fields in the dialog.
+   */
+  _populateProperties: function BPP__populateProperties() {
+    var location = this._bookmarkURI;
+    var title = this._bookmarkTitle;
+    var document = this._dialogWindow.document;
+
+    // hide standard dialog button, lest it throw off size calculation later
+    this._dialogWindow.document.documentElement.getButton("accept").hidden=true;
+
+    var nurl = document.getElementById("edit-urlbar");
+
+    var titlebox = document.getElementById("edit-titlebox");
+
+    nurl.value = location.spec;
+    titlebox.value = title;
+
+    var shortcutbox =
+      this._dialogWindow.document.getElementById("edit-shortcutbox");
+    shortcutbox.value = this._bms.getKeywordForURI(this._bookmarkURI);
+
+    var tagArea = document.getElementById("tagbox");
+
+    while (tagArea.hasChildNodes()) {
+      tagArea.removeChild(tagArea.firstChild);
+    }
+
+    var elementDict = {};
+
+    var root = this._assignableFolderResult.root; //Root is always a container.
+    root.containerOpen = true;
+    this._populateTags(root, 0, tagArea, elementDict);
+    root.containerOpen = false;
+
+    var categories = this._bms.getBookmarkFolders(location, {});
+
+    this._updateFolderTextbox(location);
+
+    var length = 0;
+    for (key in elementDict) {
+      length++;
+    }
+
+    for (var i=0; i < categories.length; i++) {
+      var elm = elementDict[categories[i]];
+      elm.setAttribute("selected", "true");
+    }
+  },
+
+  /**
+   * Recursively populates the tag-like set of clickable folders.
+   *
+   * @param aContainer a reference to an nsINavHistoryContainerResultNode
+   *        (whose) containerOpen property is set to true) representing
+   *        the roote of the bookmark folder tree
+   * @param aDepth the current iteration depth -- pass this 0 at the top level.
+   *        This only affects the visual indentation level of the tag display.
+   * @param aParentElement a vbox element into which the tags will be populated
+   * @param aElementDict a dictionary mapping folder IDs to element references
+   *        to be populated in this method
+   *
+   * @returns none
+   */
+  _populateTags:
+  function BPP__populateTags (container, depth, parentElement, elementDict) {
+    ASSERT(container.containerOpen, "The containerOpen property of the container parameter should be set to true before calling populateTags(), and then set to false again afterwards.");
+
+    var row = null;
+    for (var i = 0; i < container.childCount; i++) {
+      var childNode = container.getChild(i);
+
+      if (childNode.type != childNode.RESULT_TYPE_FOLDER)
+        continue;
+
+      var childFolder =
+        childNode.QueryInterface(Ci.nsINavHistoryFolderResultNode);
+      childFolder.containerOpen = true;
+
+      // If we can't alter it, no use showing it as an option.
+
+      if (childFolder.childrenReadOnly) {
+        childFolder.containerOpen = false;
+        continue;
+      }
+
+      if (childFolder.hasChildren) {
+        row = document.createElement("hbox");
+        row.setAttribute("class", "l" + depth);
+        var tag = this._createTagElement(childFolder, false);
+        elementDict[childFolder.folderId] = tag;
+        tag.setAttribute("isparent", "true");
+        row.appendChild(tag);
+        parentElement.appendChild(row);
+        row = null;
+        var nextDepth = depth + 1;
+        // We're limiting max indentation level here.
+        if (nextDepth > this.MAX_INDENT_DEPTH)
+          nextDepth = this.MAX_INDENT_DEPTH;
+        this._populateTags(childFolder, nextDepth, parentElement, elementDict);
+      } else {
+        if (row == null) {
+          row = document.createElement("description");
+          row.setAttribute("class", "l" + depth);
+          parentElement.appendChild(row);
+        } else {
+          // we now know that there must"ve been a tag before us on the same row
+          var separator = document.createElement("label");
+          separator.setAttribute("value", eval("\"\\u2022\"")); // bullet
+          separator.setAttribute("class", "tag-separator");
+          row.appendChild(separator);
+        }
+        var tag = this._createTagElement(childFolder, false);
+        elementDict[childFolder.folderId] = tag;
+        row.appendChild(tag);
+      }
+      childFolder.containerOpen = false;
+    }
+  },
+
+  /**
+   * This method creates a XUL element to represent a given Bookmark
+   * folder node.
+   *
+   * @param aNode an nsINavHistoryFolderResultNode object
+   * @param aIsSelected boolean, true if the given folder is currently selected
+   *
+   * @return a new XUL element corresponding to aNode
+   */
+  _createTagElement: function BPP_createTagElement(node, isSelected) {
+    var tag = this._dialogWindow.document.createElement("label");
+    tag.setAttribute("value", node.title);
+    tag.setAttribute("folderid", node.folderId);
+    tag.setAttribute("selected", "" + isSelected);
+    var self = this;
+    function onClick(e) {
+      self.tagClicked(e);
+    }
+    tag.addEventListener("command", onClick, false);
+    // We need the click event handler until we change the element from labels
+    // to something like checkboxes.
+    tag.addEventListener("click", onClick, false);
+    tag.setAttribute("class", "tag");
+    return tag;
+  },
+
+  /**
+   * This method should be called when a tag element generated by
+   * _createTagElement is clicked by the user.
+   */
+  tagClicked: function BPP_tagClicked(event) {
+    var tagElement = event.target;
+
+    var folderId = parseInt(tagElement.getAttribute("folderid"));
+
+    if (tagElement.getAttribute("selected") == "true") {
+      this._bms.removeItem(folderId, this._bookmarkURI);
+      tagElement.setAttribute("selected", "false");
+    } else {
+      this._bms.insertItem(folderId, this._bookmarkURI, -1);
+      tagElement.setAttribute("selected", "true");
+    }
+
+    this._updateFolderTextbox(this._bookmarkURI);
+  },
+
+  /**
+   * This method sets the contents of the "Folders" textbox in the
+   * Bookmark Properties panel.
+   *
+   * @param uri an nsIURI object representing the current bookmark's URI
+   */
+  _updateFolderTextbox: function BPP__updateFolderTextbox(uri) {
+    var folderTextbox = document.getElementById("places-folder-list");
+    folderTextbox.value = this._getFolderNameListForURI(uri);
+  },
+
+  /**
+   * This method gets the list of folders that contain the current bookmark.
+   *
+   * @param aURI a nsIURI object representing the URI of the current bookmark
+   *
+   * @returns a comma-separated list of folder names in string form
+   */
+  _getFolderNameListForURI: function BPP__getFolderNameListForURI(uri) {
+    var folders = this._bms.getBookmarkFolders(uri, {});
+    var results = [];
+    for (var i = 0; i < folders.length; i++) {
+      results.push(this._bms.getFolderTitle(folders[i]));
+    }
+    return results.join(", ");
+  },
+
+  /**
+   * Makes a URI from a spec.
+   * @param   spec
+   *          The string spec of the URI
+   * @returns A URI object for the spec. 
+   */
+  _uri: function PC__uri(spec) {
+    return this._ios.newURI(spec, null, null);
+  },
+
+  /**
+   * Size the dialog to fit its contents.
+   */
+  _updateSize: function BPP__updateSize() {
+    var childDoc = this._dialogWindow.document;
+
+    var tagbox = childDoc.getElementById("tagbox");
+    tagbox.style.overflow="auto";
+
+    var pio = childDoc.getElementById("places-info-options");
+    var pig = childDoc.getElementById("places-info-grid");
+
+    var newHeight = pio.boxObject.y + pio.boxObject.height + 5;
+    this._dialogWindow.resizeTo(this._dialogWindow.innerWidth, newHeight);
+  },
+
+ /**
+  * This method implements the "Delete Bookmark" action
+  * in the Bookmark Properties dialog.
+  */
+  dialogDeleteBookmark: function BPP_dialogDeleteBookmark() {
+    this.deleteBookmark(this._bookmarkURI);
+    this._hideBookmarkProperties();
+  },
+
+ /**
+  * This method implements the "Done" action
+  * in the Bookmark Properties dialog.
+  */
+  dialogDone: function BPP_dialogDone() {
+    this._saveChanges();
+    this._hideBookmarkProperties();
+  },
+
+  /**
+   * This method deletes the bookmark corresponding to the URI stored
+   * in bookmarkURI.
+   */
+  deleteBookmark: function BPP_deleteBookmark(bookmarkURI) {
+    this._assertURINotString(bookmarkURI);
+
+    var folders = this._bms.getBookmarkFolders(bookmarkURI, {});
+    if (folders.length == 0)
+      return;
+
+    this._bms.beginUpdateBatch();
+    for (var i = 0; i < folders.length; i++) {
+      this._bms.removeItem(folders[i], bookmarkURI);
+    }
+    this._bms.endUpdateBatch();
+  },
+
+  /**
+   * Save any changes that might have been made while the properties dialog
+   * was open.
+   */
+  _saveChanges: function PBD_saveChanges() {
+    var titlebox = this._dialogWindow.document.getElementById("edit-titlebox");
+    this._bms.setItemTitle(this._bookmarkURI, titlebox.value);
+
+    var shortcutbox =
+      this._dialogWindow.document.getElementById("edit-shortcutbox");
+    this._bms.setKeywordForURI(this._bookmarkURI, shortcutbox.value);
+
+    var urlbox = this._dialogWindow.document.getElementById("edit-urlbar");
+    if (urlbox.value != this._bookmarkURI.spec) {
+      /*  this._controller.changeBookmarkURI(this._bookmarkURI,
+          this._uri(urlbox.value));*/
+      LOG("TODO: delete existing bookmark, create new one with same folder & location.");
+    }
+  },
+
+  /**
+   * This method is called to exit the Bookmark Properties panel.
+   */
+  _hideBookmarkProperties: function BPP__hideBookmarkProperties() {
+    this._dialogWindow.close();
+  },
+}

browser/components/places/content/bookmarkProperties.xul

@@ -1,15 +1,19 @@
 <?xml version="1.0"?>
 
+<?xml-stylesheet href="chrome://global/skin/"?>
 <?xml-stylesheet href="chrome://browser/skin/places/bookmarkProperties.css"?>
 
-<!DOCTYPE window SYSTEM "chrome://browser/locale/places/places.dtd">
+<!DOCTYPE window 
+  SYSTEM "chrome://browser/locale/places/bookmarkProperties.dtd">
 
 <dialog id="bookmarkproperties" title="&bookmark.property.panel.title;"
-	buttons="accept"
+        buttons="accept"
-        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" onload="window.PlacesBrowserShim = window.opener.PlacesBrowserShim; window.PlacesBrowserShim.prepareBookmarkDialog(window);">
+        ondialogaccept="BookmarkPropertiesPanel.dialogDone();"
-      <hbox id="edit-urlbox">
+        xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
-	<textbox id="edit-urlbar" flex="1" size="10" tooltiptext="&bookmark.property.location;"/>
+        onload="BookmarkPropertiesPanel.init(window, window.arguments[0], window.arguments[1]);">
-      </hbox>
+
+  <script type="application/x-javascript"
+          src="chrome://browser/content/places/bookmarkProperties.js"/>
 
       <vbox id="places-info-options">
 	<grid id="places-info-grid" flex="1">
@@ -26,13 +30,29 @@
 	      </vbox>
 	      <textbox id="edit-titlebox" value=""/>
 	    </row>
+	    <row>
+	      <vbox align="end">
+		<hbox align="center" flex="1">
+		  <label value="&bookmark.property.location;" align="middle"/>
+		</hbox>
+	      </vbox>
+	      <textbox id="edit-urlbar" size="10"/>
+	    </row>
+	    <row>
+	      <vbox align="end">
+		<hbox align="center" flex="1">
+		  <label value="&bookmark.property.shortcut;" align="middle"/>
+		</hbox>
+	      </vbox>
+	      <textbox id="edit-shortcutbox" value=""/>
+	    </row>
 	    <row>
 	      <vbox align="end">
 		<hbox align="center" flex="1">
 		  <label value="&bookmark.property.folders;"/>
 		</hbox>
 	      </vbox>
-	      <textbox id="places-folder-list" value="Chronic-what" disabled="true"/>
+	      <textbox id="places-folder-list" value="" disabled="true"/>
 	    </row>
 	    <row>
 	      <vbox align="end">
@@ -46,10 +66,12 @@
 	    <row>
 	      <vbox></vbox>
 	      <vbox><hbox align="end">
-		<button id="places-link" label="&cmd.show_bookmarks.label;" oncommand="PlacesBrowserShim.dialogShowBookmarks(window);"/>		
+		<button label="&cmd.delete_bookmark.label;"
-		<button label="&cmd.delete_bookmark.label;" oncommand="PlacesBrowserShim.dialogDeleteBookmark(window);" align="end"/>
+                        oncommand="BookmarkPropertiesPanel.dialogDeleteBookmark();"
+                        align="end"/>
 		<spacer flex="1"/>
-		<button label="&cmd.hide_bookmark_properties.label;" oncommand="PlacesBrowserShim.dialogDone(window);"/>
+		<button label="&cmd.hide_bookmark_properties.label;" 
+                        oncommand="BookmarkPropertiesPanel.dialogDone();"/>
 	      </hbox>
 	      </vbox>
 	    </row>

browser/components/places/content/commands.inc

@@ -16,10 +16,13 @@
                label="&cmd.edit_copy.label;" accesskey="&cmd.edit_copy.accesskey;"
                oncommand="PlacesController.copy();"/>
       <command id="placesCmd_show:info" 
+               oncommand="PlacesController.showBookmarkPropertiesForSelection();"
 #ifdef XP_WIN
-               label="&cmd.show_infoWin.label;" accesskey="&cmd.show_infoWin.accesskey;"/>
+               label="&cmd.show_infoWin.label;" 
+	       accesskey="&cmd.show_infoWin.accesskey;"/>
 #else
-               label="&cmd.show_infoMac.label;" accesskey="&cmd.show_infoMac.accesskey;"/>
+               label="&cmd.show_infoMac.label;" 
+               accesskey="&cmd.show_infoMac.accesskey;"/>
 #endif
     </commandset>
     <commandset>

browser/components/places/content/controller.js

@@ -299,6 +299,16 @@ var PlacesController = {
     return this.__hist;
   },
 
+  /** UI Text Strings */
+
+  __strings: null,
+  get _strings() {
+    if (!this.__strings) {
+      this.__strings = document.getElementById("placeBundle");
+    }
+    return this.__strings;
+  },
+
   /**
    * Generates a HistoryResultNode for the contents of a folder. 
    * @param   folderId
@@ -319,6 +329,7 @@ var PlacesController = {
     options.setGroupingMode([Ci.nsINavHistoryQueryOptions.GROUP_BY_FOLDER], 1);
     options.excludeItems = excludeItems;
     options.expandQueries = expandQueries;
+
     var result = this._hist.executeQuery(query, options);
     result.root.containerOpen = true;
     return asContainer(result.root);
@@ -766,6 +777,73 @@ var PlacesController = {
       this._activeView.browserWindow.openUILink(node.uri, event, false, false);
   },
 
+  /**
+   * Opens the bookmark properties for the selected URI Node.
+   */
+
+  showBookmarkPropertiesForSelection: 
+  function PC_showBookmarkPropertiesForSelection() {
+    var node = this._activeView.selectedURINode;
+    if (!node || !node.uri) return;
+
+    this.showBookmarkProperties(this._uri(node.uri));
+  },
+
+  /**
+   * This method can be run on a URI parameter to ensure that it didn't
+   * receive a string instead of an nsIURI object.
+   */
+
+  _assertURINotString: function PC__assertURINotString(value) {
+    ASSERT((typeof(value) == "object") && !(value instanceof String), "This method should be passed a URI as a nsIURI object, not as a string.");
+  },
+
+  /**
+   * Opens the bookmark properties panel for a given bookmarked URI.
+   *
+   * @param bookmarkURI  an nsIURI object representing a bookmarked URI
+   */
+
+  showBookmarkProperties: function PC_showBookmarkProperties(bookmarkURI) {
+    this._assertURINotString(bookmarkURI);
+    ASSERT(this._bms.isBookmarked(bookmarkURI), "showBookmarkProperties() was called on a URI that hadn't been bookmarked: " + bookmarkURI.spec);
+
+    if (!this._bms.isBookmarked(bookmarkURI)) return;
+
+    var view = this._activeView.browserWindow;
+    view.openDialog("chrome://browser/content/places/bookmarkProperties.xul",
+                    "bookmarkproperties",
+                    "width=600,height=400,chrome,dependent,modal,resizable",
+                    bookmarkURI, this);
+  },
+
+  /**
+   * This method changes the URI of a bookmark.  Because of the URI-based
+   * identity model, it accomplishes this by replacing instances of the old
+   * URI with the new URI in each applicable folder, then copies the
+   * metadata from the old URI to the new URI.
+   */
+  /* NOTE(jhughes): don't use yet; UI doesn't update correctly 2006-03-04
+                    see bug 329524 */
+  changeBookmarkURI: function PC_changeBookmarkProperties(oldURI, newURI) {
+    this._assertURINotString(oldURI);
+    this._assertURINotString(newURI);
+    ASSERT(this._bms.isBookmarked(oldURI));
+
+    if (oldURI.spec == newURI.spec) return;
+
+    var folders = this._bms.getBookmarkFolders(oldURI, {});
+    this._bms.beginUpdateBatch();
+    for (var i = 0; i < folders.length; i++) {
+      this._bms.replaceItem(folders[i], oldURI, newURI);
+    }
+    this._bms.setItemTitle(newURI,
+                           this._bms.getItemTitle(oldURI));
+    this._bms.setKeywordForURI(newURI, 
+                               this._bms.getKeywordForURI(oldURI));
+    this._bms.endUpdateBatch();
+  },
+
   /**
    * Loads the selected URL in a new tab. 
    */

browser/components/places/jar.mn

@@ -4,6 +4,7 @@ browser.jar:
     content/browser/places/places.xml                    (content/places.xml)
     content/browser/places/places.css                    (content/places.css)
     content/browser/places/bookmarkProperties.xul        (content/bookmarkProperties.xul)
+*    content/browser/places/bookmarkProperties.js         (content/bookmarkProperties.js)
 *   content/browser/places/toolbar.xml                   (content/toolbar.xml)
 *   content/browser/places/menu.xml                      (content/menu.xml)
     content/browser/places/tree.xml                      (content/tree.xml)

browser/locales/en-US/chrome/browser/places/bookmarkProperties.dtd

@@ -0,0 +1,16 @@
+<!ENTITY bookmark.property.panel.title
+  "Bookmark Properties">
+<!ENTITY bookmark.property.location
+  "Location">
+<!ENTITY bookmark.property.title
+  "Name">
+<!ENTITY bookmark.property.shortcut
+  "Shortcut">
+<!ENTITY bookmark.property.folders
+  "Folders">
+<!ENTITY bookmark.property.folder_list
+  "Folder List">
+<!ENTITY cmd.delete_bookmark.label
+  "Delete Bookmark">
+<!ENTITY cmd.hide_bookmark_properties.label
+  "Done">

browser/locales/en-US/chrome/browser/places/places.dtd

@@ -204,19 +204,3 @@
   "Star (On)">
 <!ENTITY location.status.bookmark.tooltip
   "Add a Bookmark to this page">
-<!ENTITY bookmark.property.panel.title
-  "Bookmark Properties">
-<!ENTITY bookmark.property.title
-  "Title">
-<!ENTITY bookmark.property.folders
-  "Folders">
-<!ENTITY bookmark.property.folder_list
-  "Folder List">
-<!ENTITY bookmark.property.location
-  "Bookmark Location">
-<!ENTITY cmd.show_bookmarks.label
-  "See all bookmarks...">
-<!ENTITY cmd.delete_bookmark.label
-  "Delete Bookmark">
-<!ENTITY cmd.hide_bookmark_properties.label
-  "Done">

browser/locales/jar.mn

@@ -24,6 +24,7 @@
     locale/browser/shellservice.properties         (%chrome/browser/shellservice.properties)
 #ifdef MOZ_PLACES
     locale/browser/places/places.dtd               (%chrome/browser/places/places.dtd)
+    locale/browser/places/bookmarkProperties.dtd   (%chrome/browser/places/bookmarkProperties.dtd)
     locale/browser/places/places.properties        (%chrome/browser/places/places.properties)
     locale/browser/places/default_places.html      (%chrome/browser/places/default_places.html)
 #else

browser/themes/pinstripe/browser/places/bookmarkProperties.css

@@ -1,7 +1,3 @@
-#bookmarkproperties {
-  background-color: yellow;
-}
-
 .tag{
   padding: 2px;
   margin: 0px;

browser/themes/winstripe/browser/places/bookmarkProperties.css

@@ -1,7 +1,3 @@
-#bookmarkproperties {
-  background-color: yellow;
-}
-
 .tag{
   padding: 2px;
   margin: 0px;