diff --git a/mozilla/browser/components/places/src/nsPlacesTransactionsService.js b/mozilla/browser/components/places/src/nsPlacesTransactionsService.js index 087a9711c6b..0bd1821913d 100644 --- a/mozilla/browser/components/places/src/nsPlacesTransactionsService.js +++ b/mozilla/browser/components/places/src/nsPlacesTransactionsService.js @@ -97,6 +97,19 @@ placesTransactionsService.prototype = { }, removeItem: function placesRmItem(id) { + if (id == PlacesUtils.tagsFolderId || id == PlacesUtils.placesRootId || + id == PlacesUtils.bookmarksMenuFolderId || + id == PlacesUtils.toolbarFolderId) + throw Cr.NS_ERROR_INVALID_ARG; + + // if the item lives within a tag container, use the tagging transactions + var parent = PlacesUtils.bookmarks.getFolderIdForItem(id); + var grandparent = PlacesUtils.bookmarks.getFolderIdForItem(parent); + if (grandparent == PlacesUtils.tagsFolderId) { + var uri = PlacesUtils.bookmarks.getBookmarkURI(id); + return this.untagURI(uri, [parent]); + } + return new placesRemoveItemTransaction(id); }, @@ -866,6 +879,7 @@ placesSortFolderByNameTransactions.prototype = { function placesTagURITransaction(aURI, aTags) { this._uri = aURI; this._tags = aTags; + this._unfiledItemId = -1; this.redoTransaction = this.doTransaction; } @@ -873,10 +887,22 @@ placesTagURITransaction.prototype = { __proto__: placesBaseTransaction.prototype, doTransaction: function PTU_doTransaction() { + if (PlacesUtils.getBookmarksForURI(this._uri).length == 0) { + // Force an unfiled bookmark first + this.__unfiledItemId = + PlacesUtils.bookmarks + .insertBookmark(PlacesUtils.unfiledBookmarksFolderId, + this._uri, -1, + PlacesUtils.history.getPageTitle(this._uri)); + } PlacesUtils.tagging.tagURI(this._uri, this._tags); }, undoTransaction: function PTU_undoTransaction() { + if (this.__unfiledItemId != -1) { + PlacesUtils.bookmarks.removeItem(this.__unfiledItemId); + this.__unfiledItemId = -1; + } PlacesUtils.tagging.untagURI(this._uri, this._tags); } }; diff --git a/mozilla/toolkit/components/places/public/nsITaggingService.idl b/mozilla/toolkit/components/places/public/nsITaggingService.idl index 526ee9b5fcc..e984b004bb9 100644 --- a/mozilla/toolkit/components/places/public/nsITaggingService.idl +++ b/mozilla/toolkit/components/places/public/nsITaggingService.idl @@ -52,7 +52,9 @@ interface nsITaggingService : nsISupports * @param aURI * the URL to tag. * @param aTags - * Array of tags to set for the given URL. + * Array of tags to set for the given URL. Each element within the + * array can be either a tag name or a concrete itemId of a tag + * container. */ void tagURI(in nsIURI aURI, in nsIVariant aTags); @@ -64,7 +66,8 @@ interface nsITaggingService : nsISupports * the URL to un-tag. * @param aTags * Array of tags to unset. pass null to remove all tags from the given - * url. + * url. Each element within the array can be either a tag name or a + * concrete itemId of a tag container. */ void untagURI(in nsIURI aURI, in nsIVariant aTags); diff --git a/mozilla/toolkit/components/places/src/nsTaggingService.js b/mozilla/toolkit/components/places/src/nsTaggingService.js index fb9c36810be..0426a933d6f 100644 --- a/mozilla/toolkit/components/places/src/nsTaggingService.js +++ b/mozilla/toolkit/components/places/src/nsTaggingService.js @@ -103,13 +103,20 @@ TaggingService.prototype = { /** * If there's no tag with the given name, null is returned; */ - _getTagNode: function TS__getTagIndex(aName) { - var nameLower = aName.toLowerCase(); + _getTagNode: function TS__getTagIndex(aTagNameOrId) { + if (!aTagNameOrId) + throw Cr.NS_ERROR_INVALID_ARG; + + var nameLower = null; + if (typeof(aTagNameOrId) == "string") + nameLower = aTagNameOrId.toLowerCase(); + var root = this._tagsResult.root; var cc = root.childCount; for (var i=0; i < cc; i++) { var child = root.getChild(i); - if (child.title.toLowerCase() == nameLower) + if ((nameLower && child.title.toLowerCase() == nameLower) || + child.itemId === aTagNameOrId) return child; } @@ -158,11 +165,11 @@ TaggingService.prototype = { throw Cr.NS_ERROR_INVALID_ARG; for (var i=0; i < aTags.length; i++) { - if (aTags[i].length == 0) - throw Cr.NS_ERROR_INVALID_ARG; - var tagNode = this._getTagNode(aTags[i]); if (!tagNode) { + if (typeof(aTags[i]) == "number") + throw Cr.NS_ERROR_INVALID_ARG; + var tagId = this._createTag(aTags[i]); this._bms.insertBookmark(tagId, aURI, this._bms.DEFAULT_INDEX, null); } @@ -174,7 +181,7 @@ TaggingService.prototype = { // _getTagNode ignores case sensitivity // rename the tag container so the places view would match the // user-typed values - if (tagNode.title != aTags[i]) + if (typeof(aTags[i]) == "string" && tagNode.title != aTags[i]) this._bms.setItemTitle(tagNode.itemId, aTags[i]); } } @@ -209,9 +216,6 @@ TaggingService.prototype = { } for (var i=0; i < aTags.length; i++) { - if (aTags[i].length == 0) - throw Cr.NS_ERROR_INVALID_ARG; - var tagNode = this._getTagNode(aTags[i]); if (tagNode) { var itemId = { }; @@ -220,6 +224,8 @@ TaggingService.prototype = { this._removeTagIfEmpty(tagNode.itemId); } } + else if (typeof(aTags[i]) == "number") + throw Cr.NS_ERROR_INVALID_ARG; } }, diff --git a/mozilla/toolkit/components/places/tests/unit/test_tagging.js b/mozilla/toolkit/components/places/tests/unit/test_tagging.js index 0caa148fbd9..d42418a72aa 100644 --- a/mozilla/toolkit/components/places/tests/unit/test_tagging.js +++ b/mozilla/toolkit/components/places/tests/unit/test_tagging.js @@ -84,19 +84,24 @@ function run_test() { var tag1node = tagRoot.getChild(0) .QueryInterface(Ci.nsINavHistoryContainerResultNode); + var tag1itemId = tag1node.itemId; + do_check_eq(tag1node.title, "tag 1"); tag1node.containerOpen = true; do_check_eq(tag1node.childCount, 2); - // Tagging the same url twice with the same tag should be a no-op + // Tagging the same url twice (or even trice!) with the same tag should be a + // no-op tagssvc.tagURI(uri1, ["tag 1"]); do_check_eq(tag1node.childCount, 2); - - // the former should be ignored. + tagssvc.tagURI(uri1, [tag1itemId]); + do_check_eq(tag1node.childCount, 2); do_check_eq(tagRoot.childCount, 1); + // also tests bug 407575 - tagssvc.tagURI(uri1, ["tag 1", "tag 2", "Tag 1", "Tag 2"]); + tagssvc.tagURI(uri1, [tag1itemId, "tag 1", "tag 2", "Tag 1", "Tag 2"]); do_check_eq(tagRoot.childCount, 2); + do_check_eq(tag1node.childCount, 2); // test getTagsForURI var uri1tags = tagssvc.getTagsForURI(uri1, {}); @@ -126,4 +131,5 @@ function run_test() { // removing the last uri from a tag should remove the tag-container tagssvc.untagURI(uri2, ["tag 1"]); do_check_eq(tagRoot.childCount, 1); + }