From 90877ef48b6599e80edfbef45bfd5d48757d717e Mon Sep 17 00:00:00 2001 From: "mozilla.mano%sent.com" Date: Tue, 13 Mar 2007 22:35:52 +0000 Subject: [PATCH] Bug 370215 - update microsummaries to use unique place: uris for Places bookmarks. r=myk/dietrich. --- .../src/nsMicrosummaryService.js | 59 +++++++++++-------- .../places/content/bookmarkProperties.js | 43 ++++++++++---- .../components/places/content/controller.js | 15 +++-- browser/components/places/content/toolbar.xml | 22 +++---- 4 files changed, 83 insertions(+), 56 deletions(-) diff --git a/browser/components/microsummaries/src/nsMicrosummaryService.js b/browser/components/microsummaries/src/nsMicrosummaryService.js index 822167d85ea5..798a42dd2775 100644 --- a/browser/components/microsummaries/src/nsMicrosummaryService.js +++ b/browser/components/microsummaries/src/nsMicrosummaryService.js @@ -632,7 +632,7 @@ MicrosummaryService.prototype = { * via XPCOM but is more performant; inside this component, use this version. * Outside the component, use getBookmarks (no underscore prefix) instead. * - * @returns an array of bookmark IDs + * @returns an array of place: uris representing bookmarks items * */ _getBookmarks: function MSS__getBookmarks() { @@ -649,33 +649,33 @@ MicrosummaryService.prototype = { return bookmarks; }, - _getField: function MSS__getField(bookmarkID, fieldName) { - var pageURI = bookmarkID.QueryInterface(Ci.nsIURI); + _getField: function MSS__getField(aPlaceURI, aFieldName) { + aPlaceURI.QueryInterface(Ci.nsIURI); var fieldValue; - switch(fieldName) { + switch(aFieldName) { case FIELD_MICSUM_EXPIRATION: - fieldValue = this._ans.getAnnotationInt64(pageURI, fieldName); + fieldValue = this._ans.getAnnotationInt64(aPlaceURI, aFieldName); break; case FIELD_MICSUM_GEN_URI: case FIELD_GENERATED_TITLE: case FIELD_CONTENT_TYPE: default: - fieldValue = this._ans.getAnnotationString(pageURI, fieldName); + fieldValue = this._ans.getAnnotationString(aPlaceURI, aFieldName); break; } return fieldValue; }, - _setField: function MSS__setField(bookmarkID, fieldName, fieldValue) { - var pageURI = bookmarkID.QueryInterface(Ci.nsIURI); + _setField: function MSS__setField(aPlaceURI, aFieldName, aFieldValue) { + aPlaceURI.QueryInterface(Ci.nsIURI); - switch(fieldName) { + switch(aFieldName) { case FIELD_MICSUM_EXPIRATION: - this._ans.setAnnotationInt64(pageURI, - fieldName, - fieldValue, + this._ans.setAnnotationInt64(aPlaceURI, + aFieldName, + aFieldValue, 0, this._ans.EXPIRE_NEVER); break; @@ -683,30 +683,37 @@ MicrosummaryService.prototype = { case FIELD_GENERATED_TITLE: case FIELD_CONTENT_TYPE: default: - this._ans.setAnnotationString(pageURI, - fieldName, - fieldValue, + this._ans.setAnnotationString(aPlaceURI, + aFieldName, + aFieldValue, 0, this._ans.EXPIRE_NEVER); break; } }, - _clearField: function MSS__clearField(bookmarkID, fieldName) { - var pageURI = bookmarkID.QueryInterface(Ci.nsIURI); - - this._ans.removeAnnotation(pageURI, fieldName); + _clearField: function MSS__clearField(aPlaceURI, aFieldName) { + aPlaceURI.QueryInterface(Ci.nsIURI); + this._ans.removeAnnotation(aPlaceURI, aFieldName); }, - _hasField: function MSS__hasField(bookmarkID, fieldName) { - var pageURI = bookmarkID.QueryInterface(Ci.nsIURI); - return this._ans.hasAnnotation(pageURI, fieldName); + _hasField: function MSS__hasField(aPlaceURI, fieldName) { + aPlaceURI.QueryInterface(Ci.nsIURI); + return this._ans.hasAnnotation(aPlaceURI, fieldName); }, - _getPageForBookmark: function MSS__getPageForBookmark(bookmarkID) { - var pageURI = bookmarkID.QueryInterface(Ci.nsIURI); + _getPageForBookmark: function MSS__getPageForBookmark(aPlaceURI) { + aPlaceURI.QueryInterface(Ci.nsIURI); - return pageURI; + // Manually get the bookmark identifier out of the place uri until + // the query system supports parsing this sort of place: uris + var matches = /moz_bookmarks.id=([0-9]+)/.exec(aPlaceURI.spec); + if (matches) { + var bookmarkId = parseInt(matches[1]); + return this._bms.getBookmarkURI(bookmarkId); + } + + return null; }, #else @@ -914,7 +921,7 @@ MicrosummaryService.prototype = { } else { // Display a static title initially (unless there's already one set) - if (!this._getField(bookmarkID, FIELD_GENERATED_TITLE)) + if (!this._hasField(bookmarkID, FIELD_GENERATED_TITLE)) this._setField(bookmarkID, FIELD_GENERATED_TITLE, microsummary.generator.name || microsummary.generator.uri.spec); diff --git a/browser/components/places/content/bookmarkProperties.js b/browser/components/places/content/bookmarkProperties.js index 823a69d2a7e8..f84db618ad8a 100755 --- a/browser/components/places/content/bookmarkProperties.js +++ b/browser/components/places/content/bookmarkProperties.js @@ -354,9 +354,14 @@ var BookmarkPropertiesPanel = { }, _initMicrosummaryPicker: function BPP__initMicrosummaryPicker() { + var placeURI = null; + if (this._action == ACTION_EDIT) { + NS_ASSERT(this._bookmarkId, "No bookmark identifier"); + placeURI = PlacesUtils.bookmarks.getItemURI(this._bookmarkId); + } try { this._microsummaries = this._mss.getMicrosummaries(this._bookmarkURI, - this._bookmarkURI); + placeURI); } catch(ex) { // There was a problem retrieving microsummaries; disable the picker. @@ -496,9 +501,13 @@ var BookmarkPropertiesPanel = { microsummaryMenuPopup.appendChild(menuItem); - // Select the item if this is the current microsummary for the bookmark. - if (this._mss.isMicrosummary(this._bookmarkURI, microsummary)) - microsummaryMenuList.selectedItem = menuItem; + if (this._action == ACTION_EDIT) { + NS_ASSERT(this._bookmarkId, "No bookmark identifier"); + var placeURI = PlacesUtils.bookmarks.getItemURI(this._bookmarkId); + // Select the item if this is the current microsummary for the bookmark. + if (this._mss.isMicrosummary(placeURI, microsummary)) + microsummaryMenuList.selectedItem = menuItem; + } } }, @@ -734,18 +743,28 @@ var BookmarkPropertiesPanel = { // the "don't display a microsummary" item. var newMicrosummary = menuList.selectedItem.microsummary; - // Only add a microsummary update to the transaction if the microsummary - // has actually changed, i.e. the user selected no microsummary, - // but the bookmark previously had one, or the user selected a microsummary - // which is not the one the bookmark previously had. - if ((newMicrosummary == null && - this._mss.hasMicrosummary(this._bookmarkURI)) || - (newMicrosummary != null && - !this._mss.isMicrosummary(this._bookmarkURI, newMicrosummary))) { + if (this._action == ACTION_ADD && newMicrosummary) { transactions.push( new PlacesEditBookmarkMicrosummaryTransaction(itemId, newMicrosummary)); } + else { + NS_ASSERT(itemId != -1, "should have had a real bookmark id"); + + // Only add a microsummary update to the transaction if the + // microsummary has actually changed, i.e. the user selected no + // microsummary, but the bookmark previously had one, or the user + // selected a microsummary which is not the one the bookmark previously + // had. + var placeURI = PlacesUtils.bookmarks.getItemURI(itemId); + if ((newMicrosummary == null && this._mss.hasMicrosummary(placeURI)) || + (newMicrosummary != null && + !this._mss.isMicrosummary(placeURI, newMicrosummary))) { + transactions.push( + new PlacesEditBookmarkMicrosummaryTransaction(itemId, + newMicrosummary)); + } + } } // load in sidebar diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js index 83bcf66b3458..207a593cb108 100755 --- a/browser/components/places/content/controller.js +++ b/browser/components/places/content/controller.js @@ -2274,7 +2274,6 @@ PlacesEditLivemarkFeedURITransaction.prototype = { /** * Edit a bookmark's microsummary. */ -// XXXDietrich - bug 370215 - update to use bookmark id once 360133 is fixed. function PlacesEditBookmarkMicrosummaryTransaction(aID, newMicrosummary) { this.id = aID; this._newMicrosummary = newMicrosummary; @@ -2288,20 +2287,20 @@ PlacesEditBookmarkMicrosummaryTransaction.prototype = { getService(Ci.nsIMicrosummaryService), doTransaction: function PEBMT_doTransaction() { - var uri = this.bookmarks.getBookmarkURI(this.id); - this._oldMicrosummary = this.mss.getMicrosummary(uri); + var placeURI = this.bookmarks.getItemURI(this.id); + this._oldMicrosummary = this.mss.getMicrosummary(placeURI); if (this._newMicrosummary) - this.mss.setMicrosummary(uri, this._newMicrosummary); + this.mss.setMicrosummary(placeURI, this._newMicrosummary); else - this.mss.removeMicrosummary(uri); + this.mss.removeMicrosummary(placeURI); }, undoTransaction: function PEBMT_undoTransaction() { - var uri = this.bookmarks.getBookmarkURI(this.id); + var placeURI = this.bookmarks.getItemURI(this.id); if (this._oldMicrosummary) - this.mss.setMicrosummary(uri, this._oldMicrosummary); + this.mss.setMicrosummary(placeURI, this._oldMicrosummary); else - this.mss.removeMicrosummary(uri); + this.mss.removeMicrosummary(placeURI); } }; diff --git a/browser/components/places/content/toolbar.xml b/browser/components/places/content/toolbar.xml index 157c3295918f..9e50dd710b0e 100755 --- a/browser/components/places/content/toolbar.xml +++ b/browser/components/places/content/toolbar.xml @@ -163,20 +163,20 @@ var annotations = PlacesUtils.annotations; // This try/catch block is a temporary workaround for bug 336194. - var pages; + var placeURIs; try { - pages = annotations.getPagesWithAnnotation("bookmarks/generatedTitle", {}); + placeURIs = annotations.getPagesWithAnnotation("bookmarks/generatedTitle", {}); } catch(e) { - pages = []; + placeURIs = []; } // XXX It'd be faster to grab the annotations in a single query // instead of querying separately for each one, but the annotation // service provides no mechanism for doing so. - for ( var i = 0; i < pages.length; i++) - this.__generatedTitles[pages[i].spec] = - annotations.getAnnotationString(pages[i], "bookmarks/generatedTitle"); + for ( var i = 0; i < placeURIs.length; i++) + this.__generatedTitles[placeURIs[i].spec] = + annotations.getAnnotationString(placeURIs[i], "bookmarks/generatedTitle"); } return this.__generatedTitles; @@ -221,10 +221,12 @@ // Add the URI to the list of URIs currently in the view. this._currentURIs[child.uri] = true; - // If the URI has a generated title, use that instead. - if (this._generatedTitles[child.uri]) - title = this._generatedTitles[child.uri]; - + if (PlacesUtils.nodeIsBookmark(child)) { + // If the item has a generated title, use that instead. + var placeURI = PlacesUtils.bookmarks.getItemURI(child.bookmarkId); + if (this._generatedTitles[placeURI.spec]) + title = this._generatedTitles[placeURI.spec]; + } } else if (PlacesUtils.nodeIsSeparator(child)) { button = document.createElementNS(XULNS, "toolbarseparator"); } else if (PlacesUtils.nodeIsContainer(child)) {