mirror of
https://github.com/mozilla/gecko-dev.git
synced 2024-11-29 07:42:04 +00:00
Bug 394353 (Part 2) - Tag list is not updated when manually adding, renaming, or deleting tags from within the tags field.
r+a=dietrich
This commit is contained in:
Marco Bonardo
parent
e8af7e9d58
commit
c0a325a9ff
@ -138,7 +138,7 @@ var gEditItemOverlay = {
|
||||
this.uninitPanel(false);
|
||||
|
||||
var aItemIdList;
|
||||
if (aFor.length) {
|
||||
if (Array.isArray(aFor)) {
|
||||
aItemIdList = aFor;
|
||||
aFor = aItemIdList[0];
|
||||
}
|
||||
@ -211,7 +211,6 @@ var gEditItemOverlay = {
|
||||
this._multiEdit = true;
|
||||
this._allTags = [];
|
||||
this._itemIds = aItemIdList;
|
||||
var nodeToCheck = 0;
|
||||
for (var i = 0; i < aItemIdList.length; i++) {
|
||||
if (aItemIdList[i] instanceof Ci.nsIURI) {
|
||||
this._uris[i] = aItemIdList[i];
|
||||
@ -220,10 +219,8 @@ var gEditItemOverlay = {
|
||||
else
|
||||
this._uris[i] = PlacesUtils.bookmarks.getBookmarkURI(this._itemIds[i]);
|
||||
this._tags[i] = PlacesUtils.tagging.getTagsForURI(this._uris[i]);
|
||||
if (this._tags[i].length < this._tags[nodeToCheck].length)
|
||||
nodeToCheck = i;
|
||||
}
|
||||
this._getCommonTags(nodeToCheck);
|
||||
this._allTags = this._getCommonTags();
|
||||
this._initTextField("tagsField", this._allTags.join(", "), false);
|
||||
this._element("itemsCountText").value =
|
||||
PlacesUIUtils.getFormattedString("detailsPane.multipleItems",
|
||||
@ -241,7 +238,9 @@ var gEditItemOverlay = {
|
||||
|
||||
// observe changes
|
||||
if (!this._observersAdded) {
|
||||
if (this._itemId != -1)
|
||||
// Single bookmarks observe any change. History entries and multiEdit
|
||||
// observe only tags changes, through bookmarks.
|
||||
if (this._itemId != -1 || this._uri || this._multiEdit)
|
||||
PlacesUtils.bookmarks.addObserver(this, false);
|
||||
window.addEventListener("unload", this, false);
|
||||
this._observersAdded = true;
|
||||
@ -250,22 +249,19 @@ var gEditItemOverlay = {
|
||||
this._initialized = true;
|
||||
},
|
||||
|
||||
_getCommonTags: function(aArrIndex) {
|
||||
var tempArray = this._tags[aArrIndex];
|
||||
var isAllTag;
|
||||
for (var k = 0; k < tempArray.length; k++) {
|
||||
isAllTag = true;
|
||||
for (var j = 0; j < this._tags.length; j++) {
|
||||
if (j == aArrIndex)
|
||||
continue;
|
||||
if (this._tags[j].indexOf(tempArray[k]) == -1) {
|
||||
isAllTag = false;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (isAllTag)
|
||||
this._allTags.push(tempArray[k]);
|
||||
}
|
||||
/**
|
||||
* Finds tags that are in common among this._tags entries that track tags
|
||||
* for each selected uri.
|
||||
* The tags arrays should be kept up-to-date for this to work properly.
|
||||
*
|
||||
* @return array of common tags for the selected uris.
|
||||
*/
|
||||
_getCommonTags: function() {
|
||||
return this._tags[0].filter(
|
||||
function (aTag) this._tags.every(
|
||||
function (aTags) aTags.indexOf(aTag) != -1
|
||||
), this
|
||||
);
|
||||
},
|
||||
|
||||
_initTextField: function(aTextFieldId, aValue, aReadOnly) {
|
||||
@ -542,7 +538,7 @@ var gEditItemOverlay = {
|
||||
}
|
||||
|
||||
if (this._observersAdded) {
|
||||
if (this._itemId != -1)
|
||||
if (this._itemId != -1 || this._uri || this._multiEdit)
|
||||
PlacesUtils.bookmarks.removeObserver(this);
|
||||
|
||||
this._observersAdded = false;
|
||||
@ -1069,6 +1065,42 @@ var gEditItemOverlay = {
|
||||
onItemChanged: function EIO_onItemChanged(aItemId, aProperty,
|
||||
aIsAnnotationProperty, aValue,
|
||||
aLastModified, aItemType) {
|
||||
if (aProperty == "tags") {
|
||||
// Tags case is special, since they should be updated if either:
|
||||
// - the notification is for the edited bookmark
|
||||
// - the notification is for the edited history entry
|
||||
// - the notification is for one of edited uris
|
||||
let shouldUpdateTagsField = this._itemId == aItemId;
|
||||
if (this._itemId == -1 || this._multiEdit) {
|
||||
// Check if the changed uri is part of the modified ones.
|
||||
let changedURI = PlacesUtils.bookmarks.getBookmarkURI(aItemId);
|
||||
let uris = this._multiEdit ? this._uris : [this._uri];
|
||||
uris.forEach(function (aURI, aIndex) {
|
||||
if (aURI.equals(changedURI)) {
|
||||
shouldUpdateTagsField = true;
|
||||
if (this._multiEdit) {
|
||||
this._tags[aIndex] = PlacesUtils.tagging.getTagsForURI(this._uris[aIndex]);
|
||||
}
|
||||
}
|
||||
}, this);
|
||||
}
|
||||
|
||||
if (shouldUpdateTagsField) {
|
||||
if (this._multiEdit) {
|
||||
this._allTags = this._getCommonTags();
|
||||
this._initTextField("tagsField", this._allTags.join(", "), false);
|
||||
}
|
||||
else {
|
||||
let tags = PlacesUtils.tagging.getTagsForURI(this._uri).join(", ");
|
||||
this._initTextField("tagsField", tags, false);
|
||||
}
|
||||
}
|
||||
|
||||
// Any tags change should be reflected in the tags selector.
|
||||
this._rebuildTagsSelectorList();
|
||||
return;
|
||||
}
|
||||
|
||||
if (this._itemId != aItemId) {
|
||||
if (aProperty == "title") {
|
||||
// If the title of a folder which is listed within the folders
|
||||
@ -1164,25 +1196,9 @@ var gEditItemOverlay = {
|
||||
onItemAdded: function EIO_onItemAdded(aItemId, aParentId, aIndex, aItemType,
|
||||
aURI) {
|
||||
this._lastNewItem = aItemId;
|
||||
|
||||
if (this._uri && aItemType == PlacesUtils.bookmarks.TYPE_BOOKMARK &&
|
||||
PlacesUtils.bookmarks.getFolderIdForItem(aParentId) ==
|
||||
PlacesUtils.tagsFolderId) {
|
||||
// Ensure the tagsField is in sync.
|
||||
let tags = PlacesUtils.tagging.getTagsForURI(this._uri).join(", ");
|
||||
this._initTextField("tagsField", tags, false);
|
||||
}
|
||||
},
|
||||
onItemRemoved: function(aItemId, aParentId, aIndex, aItemType) {
|
||||
if (this._uri && aItemType == PlacesUtils.bookmarks.TYPE_BOOKMARK &&
|
||||
PlacesUtils.bookmarks.getFolderIdForItem(aParentId) ==
|
||||
PlacesUtils.tagsFolderId) {
|
||||
// Ensure the tagsField is in sync.
|
||||
let tags = PlacesUtils.tagging.getTagsForURI(this._uri).join(", ");
|
||||
this._initTextField("tagsField", tags, false);
|
||||
}
|
||||
},
|
||||
|
||||
onItemRemoved: function() { },
|
||||
onBeginUpdateBatch: function() { },
|
||||
onEndUpdateBatch: function() { },
|
||||
onBeforeItemRemoved: function() { },
|
||||
|
||||
@ -36,43 +36,163 @@
|
||||
|
||||
<script type="application/javascript">
|
||||
<![CDATA[
|
||||
function runTest() {
|
||||
function runTest()
|
||||
{
|
||||
Components.utils.import("resource://gre/modules/NetUtil.jsm");
|
||||
|
||||
const TEST_URI = NetUtil.newURI("http://www.test.me");
|
||||
const TEST_URI = NetUtil.newURI("http://www.test.me/");
|
||||
const TEST_URI2 = NetUtil.newURI("http://www.test.again.me/");
|
||||
const TEST_TAG = "test-tag";
|
||||
|
||||
// Add a bookmark, init the panel then add a tag to the bookmark
|
||||
// through the Places API. The editBookmarkPanel should live update.
|
||||
ok(gEditItemOverlay, "Sanity check: gEditItemOverlay is in context");
|
||||
|
||||
// Open the tags selector.
|
||||
document.getElementById("editBMPanel_tagsSelectorRow").collapsed = false;
|
||||
|
||||
function checkTagsSelector(aAvailableTags, aCheckedTags)
|
||||
{
|
||||
is(PlacesUtils.tagging.allTags.length, aAvailableTags.length,
|
||||
"tagging service is in sync.");
|
||||
let tagsSelector = document.getElementById("editBMPanel_tagsSelector");
|
||||
let children = tagsSelector.childNodes;
|
||||
is(children.length, aAvailableTags.length,
|
||||
"Found expected number of tags in the tags selector");
|
||||
Array.forEach(children, function (aChild) {
|
||||
let tag = aChild.getAttribute("label");
|
||||
ok(true, "Found tag '" + tag + "' in the selector");
|
||||
ok(aAvailableTags.indexOf(tag) != -1, "Found expected tag");
|
||||
let checked = aChild.getAttribute("checked") == "true";
|
||||
is(checked, aCheckedTags.indexOf(tag) != -1,
|
||||
"Tag is correctly marked");
|
||||
});
|
||||
}
|
||||
|
||||
// Add a bookmark.
|
||||
let itemId = PlacesUtils.bookmarks.insertBookmark(
|
||||
PlacesUtils.toolbarFolderId, TEST_URI,
|
||||
PlacesUtils.unfiledBookmarksFolderId, TEST_URI,
|
||||
PlacesUtils.bookmarks.DEFAULT_INDEX, "test.me"
|
||||
);
|
||||
|
||||
// Init panel.
|
||||
ok(gEditItemOverlay, "gEditItemOverlay is in context");
|
||||
gEditItemOverlay.initPanel(itemId);
|
||||
|
||||
// Add a tag.
|
||||
PlacesUtils.tagging.tagURI(TEST_URI, [TEST_TAG]);
|
||||
// Test that the tag has been added.
|
||||
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI)[0], TEST_TAG,
|
||||
"Tag correctly added.");
|
||||
// Check the panel.
|
||||
is (document.getElementById("editBMPanel_tagsField").value, TEST_TAG,
|
||||
"Tags match.");
|
||||
"Correctly added tag to a single bookmark");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, TEST_TAG,
|
||||
"Editing a single bookmark shows the added tag");
|
||||
checkTagsSelector([TEST_TAG], [TEST_TAG]);
|
||||
|
||||
// Remove tag.
|
||||
PlacesUtils.tagging.untagURI(TEST_URI, [TEST_TAG]);
|
||||
// Test that the tag has been removed.
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI)[0], undefined,
|
||||
"Tag correctly removed.");
|
||||
// Check the panel.
|
||||
is (document.getElementById("editBMPanel_tagsField").value, "",
|
||||
"Tags match.");
|
||||
"The tag has been removed");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, "",
|
||||
"Editing a single bookmark should not show any tag");
|
||||
checkTagsSelector([], []);
|
||||
|
||||
// Add a second bookmark.
|
||||
let itemId2 = PlacesUtils.bookmarks.insertBookmark(
|
||||
PlacesUtils.unfiledBookmarksFolderId, TEST_URI2,
|
||||
PlacesUtils.bookmarks.DEFAULT_INDEX, "test.again.me"
|
||||
);
|
||||
|
||||
// Init panel with multiple bookmarks.
|
||||
gEditItemOverlay.initPanel([itemId, itemId2]);
|
||||
|
||||
// Add a tag to the first bookmark.
|
||||
PlacesUtils.tagging.tagURI(TEST_URI, [TEST_TAG]);
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI)[0], TEST_TAG,
|
||||
"Correctly added a tag to the first bookmark.");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, "",
|
||||
"Editing multiple bookmarks without matching tags should not show any tag.");
|
||||
checkTagsSelector([TEST_TAG], []);
|
||||
|
||||
// Add a tag to the second bookmark.
|
||||
PlacesUtils.tagging.tagURI(TEST_URI2, [TEST_TAG]);
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI2)[0], TEST_TAG,
|
||||
"Correctly added a tag to the second bookmark.");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, TEST_TAG,
|
||||
"Editing multiple bookmarks should show matching tags.");
|
||||
checkTagsSelector([TEST_TAG], [TEST_TAG]);
|
||||
|
||||
// Remove tag from the first bookmark.
|
||||
PlacesUtils.tagging.untagURI(TEST_URI, [TEST_TAG]);
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI)[0], undefined,
|
||||
"Correctly removed tag from the first bookmark.");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, "",
|
||||
"Editing multiple bookmarks without matching tags should not show any tag.");
|
||||
checkTagsSelector([TEST_TAG], []);
|
||||
|
||||
// Remove tag from the second bookmark.
|
||||
PlacesUtils.tagging.untagURI(TEST_URI2, [TEST_TAG]);
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI2)[0], undefined,
|
||||
"Correctly removed tag from the second bookmark.");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, "",
|
||||
"Editing multiple bookmarks without matching tags should not show any tag.");
|
||||
checkTagsSelector([], []);
|
||||
|
||||
// Init panel with a nsIURI entry.
|
||||
gEditItemOverlay.initPanel(TEST_URI);
|
||||
|
||||
// Add a tag.
|
||||
PlacesUtils.tagging.tagURI(TEST_URI, [TEST_TAG]);
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI)[0], TEST_TAG,
|
||||
"Correctly added tag to the first entry.");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, TEST_TAG,
|
||||
"Editing a single nsIURI entry shows the added tag");
|
||||
checkTagsSelector([TEST_TAG], [TEST_TAG]);
|
||||
|
||||
// Remove tag.
|
||||
PlacesUtils.tagging.untagURI(TEST_URI, [TEST_TAG]);
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI)[0], undefined,
|
||||
"Correctly removed tag from the nsIURI entry.");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, "",
|
||||
"Editing a single nsIURI entry should not show any tag");
|
||||
checkTagsSelector([], []);
|
||||
|
||||
// Init panel with multiple nsIURI entries.
|
||||
gEditItemOverlay.initPanel([TEST_URI, TEST_URI2]);
|
||||
|
||||
// Add a tag to the first entry.
|
||||
PlacesUtils.tagging.tagURI(TEST_URI, [TEST_TAG]);
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI)[0], TEST_TAG,
|
||||
"Tag correctly added.");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, "",
|
||||
"Editing multiple nsIURIs without matching tags should not show any tag.");
|
||||
checkTagsSelector([TEST_TAG], []);
|
||||
|
||||
// Add a tag to the second entry.
|
||||
PlacesUtils.tagging.tagURI(TEST_URI2, [TEST_TAG]);
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI2)[0], TEST_TAG,
|
||||
"Tag correctly added.");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, TEST_TAG,
|
||||
"Editing multiple nsIURIs should show matching tags");
|
||||
checkTagsSelector([TEST_TAG], [TEST_TAG]);
|
||||
|
||||
// Remove tag from the first entry.
|
||||
PlacesUtils.tagging.untagURI(TEST_URI, [TEST_TAG]);
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI)[0], undefined,
|
||||
"Correctly removed tag from the first entry.");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, "",
|
||||
"Editing multiple nsIURIs without matching tags should not show any tag.");
|
||||
checkTagsSelector([TEST_TAG], []);
|
||||
|
||||
// Remove tag from the second entry.
|
||||
PlacesUtils.tagging.untagURI(TEST_URI2, [TEST_TAG]);
|
||||
is(PlacesUtils.tagging.getTagsForURI(TEST_URI2)[0], undefined,
|
||||
"Correctly removed tag from the second entry.");
|
||||
is(document.getElementById("editBMPanel_tagsField").value, "",
|
||||
"Editing multiple nsIURIs without matching tags should not show any tag.");
|
||||
checkTagsSelector([], []);
|
||||
|
||||
// Cleanup.
|
||||
PlacesUtils.bookmarks.removeItem(itemId);
|
||||
PlacesUtils.bookmarks.removeFolderChildren(
|
||||
PlacesUtils.unfiledBookmarksFolderId
|
||||
);
|
||||
|
||||
}
|
||||
]]>
|
||||
</script>
|
||||
|
||||
@ -941,6 +941,11 @@ nsNavBookmarks::InsertBookmark(PRInt64 aFolder,
|
||||
|
||||
if (bookmarks.Length()) {
|
||||
for (PRUint32 i = 0; i < bookmarks.Length(); i++) {
|
||||
// Don't notify to the same tag entry we just added.
|
||||
if (bookmarks[i] == *aNewBookmarkId) {
|
||||
continue;
|
||||
}
|
||||
|
||||
NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
|
||||
nsINavBookmarkObserver,
|
||||
OnItemChanged(bookmarks[i], NS_LITERAL_CSTRING("tags"),
|
||||
@ -1042,37 +1047,40 @@ nsNavBookmarks::RemoveItem(PRInt64 aItemId)
|
||||
NS_ENSURE_SUCCESS(rv, rv);
|
||||
}
|
||||
|
||||
bool isTagEntry = false;
|
||||
if (itemType == TYPE_BOOKMARK) {
|
||||
// Check if the removed bookmark was child of a tag container.
|
||||
// This is done before notifying since during the notification the parent
|
||||
// could be removed as well.
|
||||
PRInt64 grandParentId;
|
||||
rv = GetFolderIdForItem(folderId, &grandParentId);
|
||||
NS_ENSURE_SUCCESS(rv, rv);
|
||||
isTagEntry = grandParentId == mTagsRoot;
|
||||
}
|
||||
|
||||
NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
|
||||
nsINavBookmarkObserver,
|
||||
OnItemRemoved(aItemId, folderId, childIndex, itemType));
|
||||
|
||||
if (itemType == TYPE_BOOKMARK) {
|
||||
// If the removed bookmark was a child of a tag container, notify all
|
||||
// bookmark-folder result nodes which contain a bookmark for the removed
|
||||
// bookmark's url.
|
||||
PRInt64 grandParentId;
|
||||
rv = GetFolderIdForItem(folderId, &grandParentId);
|
||||
if (isTagEntry) {
|
||||
// Get all bookmarks pointing to the same uri as this tag entry and
|
||||
// notify them that tags changed.
|
||||
nsCOMPtr<nsIURI> uri;
|
||||
rv = NS_NewURI(getter_AddRefs(uri), spec);
|
||||
NS_ENSURE_SUCCESS(rv, rv);
|
||||
nsTArray<PRInt64> bookmarks;
|
||||
rv = GetBookmarkIdsForURITArray(uri, bookmarks);
|
||||
NS_ENSURE_SUCCESS(rv, rv);
|
||||
if (grandParentId == mTagsRoot) {
|
||||
nsCOMPtr<nsIURI> uri;
|
||||
rv = NS_NewURI(getter_AddRefs(uri), spec);
|
||||
NS_ENSURE_SUCCESS(rv, rv);
|
||||
nsTArray<PRInt64> bookmarks;
|
||||
|
||||
rv = GetBookmarkIdsForURITArray(uri, bookmarks);
|
||||
NS_ENSURE_SUCCESS(rv, rv);
|
||||
|
||||
if (bookmarks.Length()) {
|
||||
for (PRUint32 i = 0; i < bookmarks.Length(); i++) {
|
||||
NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
|
||||
nsINavBookmarkObserver,
|
||||
OnItemChanged(bookmarks[i],
|
||||
NS_LITERAL_CSTRING("tags"), PR_FALSE,
|
||||
EmptyCString(), 0, TYPE_BOOKMARK));
|
||||
}
|
||||
}
|
||||
for (PRUint32 i = 0; i < bookmarks.Length(); i++) {
|
||||
NOTIFY_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
|
||||
nsINavBookmarkObserver,
|
||||
OnItemChanged(bookmarks[i],
|
||||
NS_LITERAL_CSTRING("tags"), PR_FALSE,
|
||||
EmptyCString(), 0, TYPE_BOOKMARK));
|
||||
}
|
||||
}
|
||||
|
||||
return NS_OK;
|
||||
}
|
||||
|
||||
|
||||
@ -226,13 +226,13 @@ TaggingService.prototype = {
|
||||
var result = this._getTagResult(aTagId);
|
||||
if (!result)
|
||||
return;
|
||||
var node = result.root;
|
||||
node.QueryInterface(Ci.nsINavHistoryContainerResultNode);
|
||||
var node = PlacesUtils.asContainer(result.root);
|
||||
node.containerOpen = true;
|
||||
var cc = node.childCount;
|
||||
node.containerOpen = false;
|
||||
if (cc == 0)
|
||||
PlacesUtils.bookmarks.removeItem(node.itemId);
|
||||
if (cc == 0) {
|
||||
PlacesUtils.bookmarks.removeItem(aTagId);
|
||||
}
|
||||
},
|
||||
|
||||
// nsITaggingService
|
||||
@ -263,7 +263,6 @@ TaggingService.prototype = {
|
||||
if (itemId != -1) {
|
||||
// There is a tagged item.
|
||||
PlacesUtils.bookmarks.removeItem(itemId);
|
||||
this._removeTagIfEmpty(tag.id);
|
||||
}
|
||||
}
|
||||
}, taggingService);
|
||||
@ -432,6 +431,11 @@ TaggingService.prototype = {
|
||||
if (tagIds)
|
||||
this.untagURI(itemURI, tagIds);
|
||||
}
|
||||
|
||||
// Item is a tag entry. If this was the last entry for this tag, remove it.
|
||||
else if (itemURI && this._tagFolders[aFolderId]) {
|
||||
this._removeTagIfEmpty(aFolderId);
|
||||
}
|
||||
},
|
||||
|
||||
onItemChanged: function(aItemId, aProperty, aIsAnnotationProperty, aNewValue,
|
||||
|
||||
Loading…
Reference in New Issue
Block a user