diff --git a/toolkit/components/places/src/nsNavHistoryResult.cpp b/toolkit/components/places/src/nsNavHistoryResult.cpp index 0d29910d58c2..0ce7fccbe3a8 100644 --- a/toolkit/components/places/src/nsNavHistoryResult.cpp +++ b/toolkit/components/places/src/nsNavHistoryResult.cpp @@ -186,7 +186,7 @@ nsNavHistoryResultNode::GetTags(nsAString& aTags) { // then build it the first time this method called is called (and by that, // implicitly unset the void flag). Result observers may re-set the void flag // in order to force rebuilding of the tags string. - if (!mTags.IsVoid()) { + if (!mTags.IsVoid() && mParent) { aTags.Assign(mTags); return NS_OK; } @@ -2964,6 +2964,7 @@ nsNavHistoryFolderResultNode::nsNavHistoryFolderResultNode( mContentsValid(PR_FALSE), mQueryItemId(-1), mIsRegisteredFolderObserver(PR_FALSE) +, mBatchInProgress(PR_FALSE) { mItemId = aFolderId; } @@ -3356,6 +3357,7 @@ nsNavHistoryFolderResultNode::FindChildById(PRInt64 aItemId, NS_IMETHODIMP nsNavHistoryFolderResultNode::OnBeginUpdateBatch() { + mBatchInProgress = PR_TRUE; return NS_OK; } @@ -3365,6 +3367,13 @@ nsNavHistoryFolderResultNode::OnBeginUpdateBatch() NS_IMETHODIMP nsNavHistoryFolderResultNode::OnEndUpdateBatch() { + if (mBatchInProgress) { + mBatchInProgress = PR_FALSE; + nsresult rv = Refresh(); + NS_ENSURE_SUCCESS(rv, rv); + } + else + NS_WARNING("EndUpdateBatch without a begin"); return NS_OK; } @@ -3378,6 +3387,9 @@ nsNavHistoryFolderResultNode::OnItemAdded(PRInt64 aItemId, { NS_ASSERTION(aParentFolder == mItemId, "Got wrong bookmark update"); + if (mBatchInProgress) + return NS_OK; + // here, try to do something reasonable if the bookmark service gives us // a bogus index. if (aIndex < 0) { @@ -3457,6 +3469,9 @@ NS_IMETHODIMP nsNavHistoryFolderResultNode::OnItemRemoved(PRInt64 aItemId, PRInt64 aParentFolder, PRInt32 aIndex) { + if (mBatchInProgress) + return NS_OK; + // We only care about notifications when a child changes. When the deleted // item is us, our parent should also be registered and will remove us from // its list. @@ -3572,6 +3587,9 @@ nsNavHistoryFolderResultNode::OnItemChanged(PRInt64 aItemId, const nsACString& aProperty, PRBool aIsAnnotationProperty, const nsACString& aValue) { + if (mBatchInProgress) + return NS_OK; + // The query-item's title is used for simple-query nodes if (mQueryItemId != -1) { PRBool isTitleChange = aProperty.EqualsLiteral("title"); @@ -3594,6 +3612,9 @@ NS_IMETHODIMP nsNavHistoryFolderResultNode::OnItemVisited(PRInt64 aItemId, PRInt64 aVisitId, PRTime aTime) { + if (mBatchInProgress) + return NS_OK; + if (mOptions->ExcludeItems()) return NS_OK; // don't update items when we aren't displaying them if (! StartIncrementalUpdate()) @@ -3645,6 +3666,10 @@ nsNavHistoryFolderResultNode::OnItemMoved(PRInt64 aItemId, PRInt64 aOldParent, { NS_ASSERTION(aOldParent == mItemId || aNewParent == mItemId, "Got a bookmark message that doesn't belong to us"); + + if (mBatchInProgress) + return NS_OK; + if (! StartIncrementalUpdate()) return NS_OK; // entire container was refreshed for us @@ -4062,6 +4087,18 @@ nsNavHistoryResult::GetRoot(nsINavHistoryContainerResultNode** aRoot) // nsINavBookmarkObserver implementation +PR_STATIC_CALLBACK(PLDHashOperator) +FolderObserverEnumerator(nsTrimInt64HashKey::KeyType, + nsNavHistoryResult::FolderObserverList *aList, + void *aData) +{ + nsNavHistoryResult::FolderObserverList *list = + static_cast(aData); + (void)list->AppendElements(*aList); + + return PL_DHASH_NEXT; +} + // Here, it is important that we create a COPY of the observer array. Some // observers will requery themselves, which may cause the observer array to // be modified or added to. @@ -4076,6 +4113,15 @@ nsNavHistoryResult::GetRoot(nsINavHistoryContainerResultNode** aRoot) } \ } \ } +#define ENUMERATE_ALL_BOOKMARK_FOLDER_OBSERVERS(_functionCall) \ + { \ + FolderObserverList _folders; \ + mBookmarkFolderObservers.EnumerateRead(FolderObserverEnumerator, &_folders); \ + for (PRUint32 _fol_i = 0; _fol_i < _folders.Length(); _fol_i++) { \ + if (_folders[_fol_i]) \ + _folders[_fol_i]->_functionCall; \ + } \ + } #define ENUMERATE_ALL_BOOKMARKS_OBSERVERS(_functionCall) \ { \ nsTArray observerCopy(mAllBookmarksObservers); \ @@ -4101,6 +4147,7 @@ nsNavHistoryResult::OnBeginUpdateBatch() mBatchInProgress = PR_TRUE; ENUMERATE_HISTORY_OBSERVERS(OnBeginUpdateBatch()); ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnBeginUpdateBatch()); + ENUMERATE_ALL_BOOKMARK_FOLDER_OBSERVERS(OnBeginUpdateBatch()); return NS_OK; } @@ -4114,6 +4161,7 @@ nsNavHistoryResult::OnEndUpdateBatch() mBatchInProgress = PR_FALSE; ENUMERATE_HISTORY_OBSERVERS(OnEndUpdateBatch()); ENUMERATE_ALL_BOOKMARKS_OBSERVERS(OnEndUpdateBatch()); + ENUMERATE_ALL_BOOKMARK_FOLDER_OBSERVERS(OnEndUpdateBatch()); } else NS_WARNING("EndUpdateBatch without a begin"); diff --git a/toolkit/components/places/src/nsNavHistoryResult.h b/toolkit/components/places/src/nsNavHistoryResult.h index bb352285ea53..3fffd1149e16 100644 --- a/toolkit/components/places/src/nsNavHistoryResult.h +++ b/toolkit/components/places/src/nsNavHistoryResult.h @@ -808,6 +808,7 @@ public: private: PRBool mIsRegisteredFolderObserver; + PRBool mBatchInProgress; }; // nsNavHistorySeparatorResultNode diff --git a/toolkit/components/places/tests/bookmarks/test_432706_in_batch.js b/toolkit/components/places/tests/bookmarks/test_432706_in_batch.js new file mode 100644 index 000000000000..996b6b45e2c2 --- /dev/null +++ b/toolkit/components/places/tests/bookmarks/test_432706_in_batch.js @@ -0,0 +1,75 @@ +/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim:set ts=2 sw=2 sts=2 et: */ +/* ***** 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 Places test code. + * + * The Initial Developer of the Original Code is Mozilla Corp. + * Portions created by the Initial Developer are Copyright (C) 2008 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * Dietrich Ayala + * + * 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 ***** */ + +var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"]. + getService(Ci.nsINavBookmarksService); +var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"]. + getService(Ci.nsINavHistoryService); + +// main +function run_test() { + + // create a folder + var testFolder = bmsvc.createFolder(bmsvc.placesRoot, "test Folder", + bmsvc.DEFAULT_INDEX); + + // get a node for the folder + var query = histsvc.getNewQuery(); + query.setFolders([testFolder], 1); + var result = histsvc.executeQuery(query, histsvc.getNewQueryOptions()); + var rootNode = result.root; + rootNode.containerOpen = true; + + // start a batch + bmsvc.runInBatchMode({ + runBatched: function(aUserData) { + // add a bookmark + var bookmarkId = bmsvc.insertBookmark(testFolder, uri("http://google.com/"), + bmsvc.DEFAULT_INDEX, ""); + + // confirm the node child count didn't change inside the batch + do_check_eq(rootNode.childCount, 0); + } + }, null); + + // confirm that after the batch ended, the node and + // observer have been updated + do_check_eq(rootNode.childCount, 1); + + rootNode.containerOpen = false; +} diff --git a/toolkit/components/places/tests/bookmarks/test_disconnected_nodes.js b/toolkit/components/places/tests/bookmarks/test_disconnected_nodes.js new file mode 100644 index 000000000000..3602ef706fcc --- /dev/null +++ b/toolkit/components/places/tests/bookmarks/test_disconnected_nodes.js @@ -0,0 +1,93 @@ +/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim:set ts=2 sw=2 sts=2 et: */ +/* ***** 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 bug 471563 test code. + * + * The Initial Developer of the Original Code is + * Mozilla Corporation. + * Portions created by the Initial Developer are Copyright (C) 2008 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * Dietrich Ayala (Original Author) + * + * 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 ***** */ + +// get services +var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"]. + getService(Ci.nsINavHistoryService); +var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"]. + getService(Ci.nsINavBookmarksService); +var tagssvc = Cc["@mozilla.org/browser/tagging-service;1"]. + getService(Ci.nsITaggingService); + +function run_test() { + // get toolbar node + var options = histsvc.getNewQueryOptions(); + var query = histsvc.getNewQuery(); + query.setFolders([bmsvc.toolbarFolder], 1); + var result = histsvc.executeQuery(query, options); + var toolbarNode = result.root; + toolbarNode.containerOpen = true; + + // add a bookmark + var bookmarkURI = uri("http://foo.com"); + var bookmarkId = bmsvc.insertBookmark(bmsvc.toolbarFolder, bookmarkURI, + bmsvc.DEFAULT_INDEX, ""); + + // get the node for the new bookmark + var node = toolbarNode.getChild(toolbarNode.childCount-1); + do_check_eq(node.itemId, bookmarkId); + + // confirm there's no tags via the .tags property + do_check_eq(node.tags, null); + + // add a tag + // NOTE: this executes in a batch, which causes |toolbarNode| + // to be refreshed, which disconnects the uri node |node|. + tagssvc.tagURI(bookmarkURI, ["foo"]); + do_check_eq(node.tags, "foo"); + + // add another tag, and check that the node updates itself. + tagssvc.tagURI(bookmarkURI, ["bar"]); + do_check_eq(node.tags, "bar, foo"); + + // change the bookmark title, + // confirm that the node updates itself. + // XXX disabled until bug 471563 is fixed. + //bmsvc.setItemTitle(node.itemId, "newtitle"); + //do_check_eq(node.title, "newtitle"); + + // change the bookmark uri, + // confirm that the node updates itself. + // XXX disabled until bug 471563 is fixed. + //var newURI = uri("http://newuri"); + //bmsvc.changeBookmarkURI(node.itemId, newURI); + //do_check_eq(node.uri, newURI.spec); + + toolbarNode.containerOpen = false; +}