Bug 357316 - More add/edit bookmark UI work. Changes listed on comment 15, r=dietrich.

browser/base/content/browser-places.js

@@ -59,8 +59,23 @@ var PlacesCommandHook = {
    * @param aBrowser
    *        a <browser> element
    */
-  bookmarkPage: function PCH_bookmarkCurrentPage(aBrowser) {
+  bookmarkPage: function PCH_bookmarkPage(aBrowser) {
-    PlacesUtils.showAddBookmarkUI(aBrowser.currentURI);
+    // Copied over from addBookmarkForBrowser:
+    // Bug 52536: We obtain the URL and title from the nsIWebNavigation
+    // associated with a <browser/> rather than from a DOMWindow.
+    // This is because when a full page plugin is loaded, there is
+    // no DOMWindow (?) but information about the loaded document
+    // may still be obtained from the webNavigation.
+    var webNav = aBrowser.webNavigation;
+    var url = webNav.currentURI;
+    var title;
+    var description;
+    try {
+      title = webNav.document.title;
+      description = PlacesUtils.getDescriptionFromDocument(webNav.document);
+    }
+    catch (e) { }
+    PlacesUtils.showAddBookmarkUI(url, title, description);
   },
 
   /**
@@ -108,25 +123,6 @@ var PlacesCommandHook = {
     PlacesUtils.showAddMultiBookmarkUI(tabURIs);
   },
 
-  /**
-   * Get the description associated with a document, as specified in a <META> 
-   * element.
-   * @param   doc
-   *          A DOM Document to get a description for
-   * @returns A description string if a META element was discovered with a 
-   *          "description" or "httpequiv" attribute, empty string otherwise.
-   */
-  _getDescriptionFromDocument: function PCH_getDescriptionFromDocument(doc) {
-    var metaElements = doc.getElementsByTagName("META");
-    for (var i = 0; i < metaElements.length; ++i) {
-      if (metaElements[i].localName.toLowerCase() == "description" || 
-          metaElements[i].httpEquiv.toLowerCase() == "description") {
-        return metaElements[i].content;
-        break;
-      }
-    }
-    return "";
-  },
   
   /**
    * Adds a Live Bookmark to a feed associated with the current page. 
@@ -135,8 +131,7 @@ var PlacesCommandHook = {
    * @title     title
    *            The title of the feed. Optional.
    * @subtitle  subtitle
-   *            A short description of the feed. Optional. 
+   *            A short description of the feed. Optional.
-   *            Not yet used. TODO: implement description annotation
    */
   addLiveBookmark: function PCH_addLiveBookmark(url, feedTitle, feedSubtitle) {
     var ios = 
@@ -144,24 +139,19 @@ var PlacesCommandHook = {
         getService(Ci.nsIIOService);
     var feedURI = ios.newURI(url, null, null);
     
-    var browser = gBrowser.selectedBrowser;
+    var doc = gBrowser.contentDocument;
-   
+    var title = (arguments.length > 1) ? feedTitle : doc.title;
-    var title = (arguments.length > 1) ? feedTitle :
-                                         browser.contentDocument.title;
  
-    // TODO: implement description annotation
-#if 0
     var description;
     if (arguments.length > 2)
       description = feedSubtitle;
     else
-      description = BookmarksUtils.getDescriptionFromDocument(doc);
+      description = PlacesUtils.getDescriptionFromDocument(doc);
-#endif
 
-    var toolbarRootIP =
+    var toolbarIP =
-      new InsertionPoint(PlacesUtils.bookmarks.toolbarRoot, -1);
+      new InsertionPoint(PlacesUtils.bookmarks.toolbarFolder, -1);
-    PlacesUtils.showAddLivemarkUI(feedURI, browser.currentURI,
+    PlacesUtils.showAddLivemarkUI(feedURI, gBrowser.currentURI,
-                                  title, toolbarRootIP, true);
+                                  title, description, toolbarIP, true);
   },
 
   /**
@@ -187,14 +177,6 @@ var PlacesCommandHook = {
 
       organizer.focus();
     }
-  },
-
-  /**
-   * This method should be called when the bookmark button is clicked.
-   */
-  onBookmarkButtonClick: function PCH_onBookmarkButtonClick() {
-    var currentURI = getBrowser().selectedBrowser.webNavigation.currentURI;
-    PlacesUtils.showAddBookmarkUI(currentURI);
   }
 };
 

browser/base/content/nsContextMenu.js

@@ -1103,8 +1103,10 @@ nsContextMenu.prototype = {
   },
 
   addBookmarkForFrame: function CM_addBookmarkForFrame() {
+    var doc = this.target.ownerDocument;
     var uri = this.target.ownerDocument.documentURIObject;
-    PlacesUtils.showAddBookmarkUI(uri);
+    var description = PlacesUtils.getDescriptionFromDocument(doc);
+    PlacesUtils.showAddBookmarkUI(uri, doc.title, description);
   },
 #else
   bookmarkThisPage: function CM_bookmarkThisPage() {

browser/components/places/content/bookmarkProperties.js

@@ -54,6 +54,8 @@
  *       @ uri (nsIURI object) - optional, the default uri for the new item.
  *         The property is not used for the "folder with items" type.
  *       @ title (String) - optional, the defualt title for the new item.
+ *       @ description (String) - optional, the default description for the new
+ *         item.
  *       @ defaultInsertionPoint (InsertionPoint JS object) - optional, the
  *         default insertion point for the new item.
  *      Notes:
@@ -74,7 +76,7 @@
  *     Possible values:
  *     - "title"
  *     - "location"
- *     - "description" (XXXmano: not yet implemented)
+ *     - "description"
  *     - "keyword"
  *     - "microsummary"
  *     - "load in sidebar"
@@ -130,6 +132,7 @@ var BookmarkPropertiesPanel = {
   _bookmarkURI: null,
   _loadBookmarkInSidebar: false,
   _itemTitle: "",
+  _itemDescription: "",
   _microsummaries: null,
 
   /**
@@ -206,6 +209,7 @@ var BookmarkPropertiesPanel = {
 
           if ("loadBookmarkInSidebar" in dialogInfo)
             this._loadBookmarkInSidebar = dialogInfo.loadBookmarkInSidebar;
+
           break;
         case "folder":
           this._action = ACTION_ADD;
@@ -243,40 +247,60 @@ var BookmarkPropertiesPanel = {
               this._itemTitle = this._strings.getString("newLivemarkDefault");
           }
       }
+
+      if ("description" in dialogInfo)
+        this._itemDescription = dialogInfo.description;
     }
     else { // edit
+      const annos = PlacesUtils.annotations;
+      const bookmarks = PlacesUtils.bookmarks;
+
+      var placeURI;
       switch (dialogInfo.type) {
         case "bookmark":
           NS_ASSERT("bookmarkId" in dialogInfo);
+
           this._action = ACTION_EDIT;
           this._itemType = BOOKMARK_ITEM;
           this._bookmarkId = dialogInfo.bookmarkId;
-          this._bookmarkURI =
+          placeURI = bookmarks.getItemURI(this._bookmarkId);
-            PlacesUtils.bookmarks.getBookmarkURI(this._bookmarkId);
+
-          this._itemTitle = PlacesUtils.bookmarks
+          this._bookmarkURI = bookmarks.getBookmarkURI(this._bookmarkId);
-                                           .getItemTitle(this._bookmarkId);
+          this._itemTitle = bookmarks.getItemTitle(this._bookmarkId);
+
+          // keyword
           this._bookmarkKeyword =
-            PlacesUtils.bookmarks.getKeywordForBookmark(this._bookmarkId);
+            bookmarks.getKeywordForBookmark(this._bookmarkId);
-          var placeURI = PlacesUtils.bookmarks.getItemURI(this._bookmarkId);
+
+          // Load In Sidebar
           this._loadBookmarkInSidebar =
-            PlacesUtils.annotations.hasAnnotation(placeURI,
+            annos.hasAnnotation(placeURI, LOAD_IN_SIDEBAR_ANNO);
-                                                  LOAD_IN_SIDEBAR_ANNO);
+
           break;
         case "folder":
           NS_ASSERT("folderId" in dialogInfo);
+
           this._action = ACTION_EDIT;
           this._folderId = dialogInfo.folderId;
-          if (PlacesUtils.livemarks.isLivemark(this._folderId)) {
+          placeURI = bookmarks.getFolderURI(this._folderId);
+
+          const livemarks = PlacesUtils.livemarks;
+          if (livemarks.isLivemark(this._folderId)) {
             this._itemType = LIVEMARK_CONTAINER;
-            this._feedURI = PlacesUtils.livemarks.getFeedURI(this._folderId);
+            this._feedURI = livemarks.getFeedURI(this._folderId);
-            this._siteURI = PlacesUtils.livemarks.getSiteURI(this._folderId);
+            this._siteURI = livemarks.getSiteURI(this._folderId);
           }
           else
             this._itemType = BOOKMARK_FOLDER;
-          this._itemTitle =
+          this._itemTitle = bookmarks.getFolderTitle(this._folderId);
-            PlacesUtils.bookmarks.getFolderTitle(this._folderId);
           break;
       }
+
+      // Description
+      if (annos.hasAnnotation(placeURI, DESCRIPTION_ANNO)) {
+        this._itemDescription =
+          annos.getAnnotationString(placeURI, DESCRIPTION_ANNO);
+      }
     }
   },
 
@@ -363,12 +387,14 @@ var BookmarkPropertiesPanel = {
     if (!hiddenRows)
       return;
 
-    if (hiddenRows.indexOf("title")!= -1)
+    if (hiddenRows.indexOf("title") != -1)
       this._element("titleTextfield").hidden = true;
-    if (hiddenRows.indexOf("location")!= -1)
+    if (hiddenRows.indexOf("location") != -1)
       this._element("locationRow").hidden = true;
-    if (hiddenRows.indexOf("keyword")!= -1)
+    if (hiddenRows.indexOf("keyword") != -1)
       this._element("shortcutRow").hidden = true;
+    if (hiddenRows.indexOf("description")!= -1)
+      this._element("descriptionRow").hidden = true;
     if (hiddenRows.indexOf("folder picker") != -1)
       this._element("folderRow").hidden = true;
     if (hiddenRows.indexOf("feedURI") != -1)
@@ -387,8 +413,9 @@ var BookmarkPropertiesPanel = {
   _populateProperties: function BPP__populateProperties() {
     document.title = this._getDialogTitle();
     document.documentElement.getButton("accept").label = this._getAcceptLabel();
-    if (this._itemTitle)
+    
-      this._element("titleTextfield").value = this._itemTitle;
+    this._element("titleTextfield").value = this._itemTitle;
+    this._element("descriptionTextfield").value = this._itemDescription;
 
     if (this._itemType == BOOKMARK_ITEM) {
       if (this._bookmarkURI)
@@ -731,6 +758,15 @@ var BookmarkPropertiesPanel = {
       }
     }
 
+    // description
+    var description = this._element("descriptionTextfield").value;
+    if ((this._action != ACTION_EDIT && description) ||
+        (description != this._itemDescription)) {
+      var isFolder = this._itemType != BOOKMARK_ITEM;
+      transactions.push(new PlacesEditItemDescriptionTransaction(
+        itemId, description, this._itemType != BOOKMARK_ITEM));
+    }
+
     // If we have any changes to perform, do them via the
     // transaction manager passed by the opener so they can be undone.
     if (transactions.length > 0) {

browser/components/places/content/bookmarkProperties.xul

@@ -32,62 +32,42 @@
       <vbox id="placesInfoOptions" flex="1">
         <grid id="placesInfoGrid" flex="1">
           <columns>
-            <column flex="1" align="start"/>
+            <column/>
-            <column flex="100" align="start"/>
+            <column flex="1"/>
           </columns>
           <rows id="placesInfoRows">
-            <row id="titleRow">
+            <row id="titleRow" align="center">
-              <vbox align="end">
+              <label value="&bookmark.property.title;" control="titleTextfield"/>
-                <hbox align="center" flex="1">
-                  <label value="&bookmark.property.title;" align="middle"/>
-                </hbox>
-              </vbox>
               <textbox id="titleTextfield"/>
             </row>
-            <row id="locationRow">
+            <row id="locationRow" align="center">
-              <vbox align="end">
+              <label value="&bookmark.property.location;" control="editURLBar"/>
-                <hbox align="center" flex="1">
-                  <label value="&bookmark.property.location;" align="middle"/>
-                </hbox>
-              </vbox>
               <textbox id="editURLBar" size="10"
                        onchange="BookmarkPropertiesPanel.validateChanges();"
                        oninput="BookmarkPropertiesPanel.validateChanges();"/>
             </row>
-            <row id="livemarkFeedLocationRow">
+            <row id="livemarkFeedLocationRow" align="center">
-              <vbox align="end">
+              <label value="&livemark.property.feed_uri;" control="feedLocationTextfield"/>
-                <hbox align="center" flex="1">
-                  <label value="&livemark.property.feed_uri;" align="middle"/>
-                </hbox>
-              </vbox>
               <textbox id="feedLocationTextfield"
                        onchange="BookmarkPropertiesPanel.validateChanges();"
                        oninput="BookmarkPropertiesPanel.validateChanges();"/>
             </row>
-            <row id="livemarkSiteLocationRow">
+            <row id="livemarkSiteLocationRow" align="center">
-              <vbox align="end">
+              <label value="&livemark.property.site_uri;" control="feedSiteLocationTextfield"/>
-                <hbox align="center" flex="1">
-                  <label value="&livemark.property.site_uri;" align="middle"/>
-                </hbox>
-              </vbox>
               <textbox id="feedSiteLocationTextfield"
                        onchange="BookmarkPropertiesPanel.validateChanges();"
                        oninput="BookmarkPropertiesPanel.validateChanges();"/>
             </row>
-            <row id="shortcutRow">
+            <row id="shortcutRow" align="center">
-              <vbox align="end">
+              <label value="&bookmark.property.shortcut;" control="keywordTextfield"/>
-                <hbox align="center" flex="1">
-                  <label value="&bookmark.property.shortcut;" align="middle"/>
-                </hbox>
-              </vbox>
               <textbox id="keywordTextfield"/>
             </row>
-            <row id="microsummaryRow">
+            <row id="descriptionRow">
-              <vbox align="end">
+              <label value="&bookmark.property.description;" control="descriptionTextfield"/>
-                <hbox align="center" flex="1">
+              <textbox id="descriptionTextfield" multiline="true"/>
-                  <label value="&bookmark.property.microsummary;" align="middle"/>
+            </row>
-                </hbox>
+            <row id="microsummaryRow" align="center">
-              </vbox>
+              <label value="&bookmark.property.microsummary;" control="microsummaryMenuList"/>
               <menulist id="microsummaryMenuList">
                 <menupopup id="microsummaryMenuPopup">
                   <menuitem label="&bookmark.property.microsummary.none;"

browser/components/places/content/controller.js

@@ -2102,7 +2102,9 @@ PlacesSetLoadInSidebarTransaction.prototype = {
   },
 
   doTransaction: function PSLIST_doTransaction() {
-    this._placeURI = this.utils.bookmarks.getItemURI(this.id);
+    if (!("_placeURI" in this))
+      this._placeURI = this.utils.bookmarks.getItemURI(this.id);
+
     this._wasSet = this.utils.annotations
                        .hasAnnotation(this._placeURI, this._anno.name);
     if (this._loadInSidebar) {
@@ -2125,6 +2127,60 @@ PlacesSetLoadInSidebarTransaction.prototype = {
   }
 };
 
+/**
+ * Edit a the description of a bookmark or a folder
+ *
+ * XXXmano: aIsFolder is a temporary workaround for bug 372508
+ */
+function PlacesEditItemDescriptionTransaction(aBookmarkId, aDescription, aIsFolder) {
+  this.id = aBookmarkId;
+  this._newDescription = aDescription;
+  this._isFolder = aIsFolder;
+  this.redoTransaction = this.doTransaction;
+}
+PlacesEditItemDescriptionTransaction.prototype = {
+  __proto__: PlacesBaseTransaction.prototype,
+  _oldDescription: "",
+  DESCRIPTION_ANNO: DESCRIPTION_ANNO,
+  nsIAnnotationService: Components.interfaces.nsIAnnotationService,
+
+  doTransaction: function PSLIST_doTransaction() {
+    const annos = this.utils.annotations;
+
+    if (!("_placeURI" in this)) {
+      if (this._isFolder)
+        this._placeURI = this.utils.bookmarks.getFolderURI(this.id);
+      else
+        this._placeURI = this.utils.bookmarks.getItemURI(this.id);
+    }
+
+    if (annos.hasAnnotation(this._placeURI, this.DESCRIPTION_ANNO)) {
+      this._oldDescription =
+        annos.getAnnotationString(this._placeURI, this.DESCRIPTION_ANNO);
+    }
+
+    if (this._newDescription) {
+      annos.setAnnotationString(this._placeURI, this.DESCRIPTION_ANNO,
+                                this._newDescription, 0,
+                                this.nsIAnnotationService.EXPIRE_NEVER);
+    }
+    else if (this._oldDescription)
+      annos.removeAnnotation(this._placeURI, this.DESCRIPTION_ANNO);
+  },
+
+  undoTransaction: function PSLIST_undoTransaction() {
+    const annos = this.utils.annotations;
+
+    if (this._oldDescription) {
+      annos.setAnnotationString(this._placeURI, this.DESCRIPTION_ANNO,
+                                this._oldDescription, 0,
+                                this.nsIAnnotationService.EXPIRE_NEVER);
+    }
+    else if (this.utils.hasAnnotation(this._placeURI, this.DESCRIPTION_ANNO))
+      annos.removeAnnotation(this._placeURI, this.DESCRIPTION_ANNO);
+  }
+};
+
 /**
  * Edit a folder's title.
  */

browser/components/places/content/utils.js

@@ -46,6 +46,7 @@ const Cc = Components.classes;
 const Cr = Components.results;
 
 const LOAD_IN_SIDEBAR_ANNO = "bookmarkProperties/loadInSidebar";
+const DESCRIPTION_ANNO = "bookmarkProperties/description";
 
 function QI_node(aNode, aIID) {
   var result = null;
@@ -659,6 +660,8 @@ var PlacesUtils = {
    *        to be shown.
    * @param [optional] aTitle
    *        The default title for the new bookmark.
+   * @param [optional] aDescription
+            The default description for the new bookmark
    * @param [optional] aDefaultInsertionPoint
    *        The default insertion point for the new item. If set, the folder
    *        picker would be hidden unless aShowPicker is set to true, in which
@@ -677,7 +680,9 @@ var PlacesUtils = {
    *  - When aDefaultInsertionPoint is not set, the dialog defaults to the
    *    bookmarks root folder.
    */
-  showAddBookmarkUI: function PU_showAddBookmarkUI(aURI, aTitle,
+  showAddBookmarkUI: function PU_showAddBookmarkUI(aURI,
+                                                   aTitle,
+                                                   aDescription,
                                                    aDefaultInsertionPoint,
                                                    aShowPicker,
                                                    aLoadInSidebar) {
@@ -697,6 +702,9 @@ var PlacesUtils = {
     if (typeof(aTitle) == "string")
       info.title = aTitle;
 
+    if (aDescription)
+      info.description = aDescription;
+
     if (aDefaultInsertionPoint) {
       info.defaultInsertionPoint = aDefaultInsertionPoint;
       if (!aShowPicker)
@@ -731,8 +739,10 @@ var PlacesUtils = {
    *  - When aDefaultInsertionPoint is not set, the dialog defaults to the
    *    bookmarks root folder.
    */
-  showAddLivemarkUI: function PU_showAddLivemarkURI(aFeedURI, aSiteURI,
+  showAddLivemarkUI: function PU_showAddLivemarkURI(aFeedURI,
+                                                    aSiteURI,
                                                     aTitle,
+                                                    aDescription,
                                                     aDefaultInsertionPoint,
                                                     aShowPicker) {
     var info = {
@@ -752,6 +762,9 @@ var PlacesUtils = {
     if (typeof(aTitle) == "string")
       info.title = aTitle;
 
+    if (aDescription)
+      info.description = aDescription;
+
     if (aDefaultInsertionPoint) {
       info.defaultInsertionPoint = aDefaultInsertionPoint;
       if (!aShowPicker)
@@ -995,5 +1008,24 @@ var PlacesUtils = {
     var query = this.history.getNewQuery();
     query.setFolders([aFolderId], 1);
     return this.history.queriesToQueryString([query], 1, options);
+  },
+
+  /**
+   * Get the description associated with a document, as specified in a <META> 
+   * element.
+   * @param   doc
+   *          A DOM Document to get a description for
+   * @returns A description string if a META element was discovered with a 
+   *          "description" or "httpequiv" attribute, empty string otherwise.
+   */
+  getDescriptionFromDocument: function PU_getDescriptionFromDocument(doc) {
+    var metaElements = doc.getElementsByTagName("META");
+    for (var i = 0; i < metaElements.length; ++i) {
+      if (metaElements[i].localName.toLowerCase() == "description" || 
+          metaElements[i].httpEquiv.toLowerCase() == "description") {
+        return metaElements[i].content;
+      }
+    }
+    return "";
   }
 };

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

@@ -6,6 +6,8 @@
   "Name">
 <!ENTITY bookmark.property.shortcut
   "Shortcut">
+<!ENTITY bookmark.property.description
+  "Description">
 <!ENTITY bookmark.property.microsummary
   "Summary">
 <!ENTITY bookmark.property.microsummary.none