From fd784c5852e1450f61f7740abe56698a6157ac1d Mon Sep 17 00:00:00 2001 From: "benjamin%smedbergs.us" Date: Tue, 18 Jul 2006 18:37:29 +0000 Subject: [PATCH] Bug 333450 r=beng Add bookmark indices to result nodes. Original committer: brettw%gmail.com Original revision: 1.73 Original date: 2006/04/17 18:57:52 --- .../places/src/nsNavHistoryResult.cpp | 320 +++++++++++------- 1 file changed, 199 insertions(+), 121 deletions(-) diff --git a/toolkit/components/places/src/nsNavHistoryResult.cpp b/toolkit/components/places/src/nsNavHistoryResult.cpp index da94fbb8ab7..68ca278a2d5 100644 --- a/toolkit/components/places/src/nsNavHistoryResult.cpp +++ b/toolkit/components/places/src/nsNavHistoryResult.cpp @@ -111,6 +111,7 @@ nsNavHistoryResultNode::nsNavHistoryResultNode( mAccessCount(aAccessCount), mTime(aTime), mFaviconURI(aIconURI), + mBookmarkIndex(-1), mIndentLevel(-1), mViewIndex(-1) { @@ -550,7 +551,7 @@ nsNavHistoryContainerResultNode::GetSortingComparator(PRUint32 aSortType) switch (aSortType) { case nsINavHistoryQueryOptions::SORT_BY_NONE: - return nsnull; + return &SortComparison_Bookmark; case nsINavHistoryQueryOptions::SORT_BY_TITLE_ASCENDING: return &SortComparison_TitleLess; case nsINavHistoryQueryOptions::SORT_BY_TITLE_DESCENDING: @@ -664,6 +665,19 @@ nsNavHistoryContainerResultNode::DoesChildNeedResorting(PRUint32 aIndex, } +// nsNavHistoryContainerResultNode::SortComparison_Bookmark +// +// When there are bookmark indices, we should never have ties, so we don't +// need to worry about tiebreaking. When there are no bookmark indices, +// everything will be -1 and we don't worry about sorting. + +PRInt32 PR_CALLBACK nsNavHistoryContainerResultNode::SortComparison_Bookmark( + nsNavHistoryResultNode* a, nsNavHistoryResultNode* b, void* closure) +{ + return a->mBookmarkIndex - b->mBookmarkIndex; +} + + // nsNavHistoryContainerResultNode::SortComparison_Title* // // These are a little more complicated because they do a localization @@ -2257,14 +2271,6 @@ nsNavHistoryQueryResultNode::OnItemRemoved(nsIURI* aBookmark, PRInt64 aFolder, return NS_OK; } NS_IMETHODIMP -nsNavHistoryQueryResultNode::OnItemMoved(nsIURI* aBookmark, PRInt64 aFolder, - PRInt32 aOldIndex, PRInt32 aNewIndex) -{ - if (mLiveUpdate == QUERYUPDATE_COMPLEX_WITH_BOOKMARKS) - return Refresh(); - return NS_OK; -} -NS_IMETHODIMP nsNavHistoryQueryResultNode::OnItemChanged(nsIURI* aBookmark, const nsACString& aProperty, const nsAString& aValue) @@ -2654,8 +2660,9 @@ nsNavHistoryFolderResultNode::Refresh() } // ignore errors from FillChildren, since we will still want to refresh - // the tree (there just might not be anything in it on error) - ClearChildren(PR_FALSE); + // the tree (there just might not be anything in it on error). Need to + // unregister as an observer because FillChildren will try to re-register us. + ClearChildren(PR_TRUE); FillChildren(); nsNavHistoryResult* result = GetResult(); @@ -2700,6 +2707,26 @@ nsNavHistoryFolderResultNode::StartIncrementalUpdate() } +// nsNavHistoryFolderResultNode::ReindexRange +// +// This function adds aDelta to all bookmark indices between the two +// endpoints, inclusive. It is used when items are added or removed from +// the bookmark folder. + +void +nsNavHistoryFolderResultNode::ReindexRange(PRInt32 aStartIndex, + PRInt32 aEndIndex, + PRInt32 aDelta) +{ + for (PRInt32 i = 0; i < mChildren.Count(); i ++) { + nsNavHistoryResultNode* node = mChildren[i]; + if (node->mBookmarkIndex >= aStartIndex && + node->mBookmarkIndex <= aEndIndex) + node->mBookmarkIndex += aDelta; + } +} + + // nsNavHistoryFolderResultNode::OnBeginUpdateBatch (nsINavBookmarkObserver) NS_IMETHODIMP @@ -2725,8 +2752,12 @@ nsNavHistoryFolderResultNode::OnItemAdded(nsIURI* aBookmark, PRInt64 aFolder, PRInt32 aIndex) { NS_ASSERTION(aFolder == mFolderId, "Got wrong bookmark update"); - if (mOptions->ExcludeItems()) - return NS_OK; // don't update items when we aren't displaying them + if (mOptions->ExcludeItems()) { + // don't update items when we aren't displaying them, but we still need + // to adjust bookmark indices to account for the insertion + ReindexRange(aIndex, PR_INT32_MAX, 1); + return NS_OK; + } // here, try to do something reasonable if the bookmark service gives us // a bogus index. @@ -2738,7 +2769,10 @@ nsNavHistoryFolderResultNode::OnItemAdded(nsIURI* aBookmark, PRInt64 aFolder, aIndex = mChildren.Count(); } if (! StartIncrementalUpdate()) - return NS_OK; + return NS_OK; // folder was completely refreshed for us + + // adjust bookmark indices + ReindexRange(aIndex, PR_INT32_MAX, 1); nsNavHistory* history = nsNavHistory::GetHistoryService(); NS_ENSURE_TRUE(history, NS_ERROR_OUT_OF_MEMORY); @@ -2746,6 +2780,7 @@ nsNavHistoryFolderResultNode::OnItemAdded(nsIURI* aBookmark, PRInt64 aFolder, nsNavHistoryResultNode* node; nsresult rv = history->UriToResultNode(aBookmark, mOptions, &node); NS_ENSURE_SUCCESS(rv, rv); + node->mBookmarkIndex = aIndex; if (GetSortType() == nsINavHistoryQueryOptions::SORT_BY_NONE) { // insert at natural bookmarks position @@ -2763,10 +2798,17 @@ nsNavHistoryFolderResultNode::OnItemRemoved(nsIURI* aBookmark, PRInt64 aFolder, PRInt32 aIndex) { NS_ASSERTION(aFolder == mFolderId, "Got wrong bookmark update"); - if (mOptions->ExcludeItems()) - return NS_OK; // don't update items when we aren't displaying them - if (! StartIncrementalUpdate()) + if (mOptions->ExcludeItems()) { + // don't update items when we aren't displaying them, but we do need to + // adjust everybody's bookmark indices to account for the removal + ReindexRange(aIndex, PR_INT32_MAX, -1); return NS_OK; + } + if (! StartIncrementalUpdate()) + return NS_OK; // folder was completely refreshed for us + + // shift all following indices down + ReindexRange(aIndex + 1, PR_INT32_MAX, -1); // don't trust the index from the bookmark service, find it ourselves. The // sorting could be different, or the bookmark services indices and ours might @@ -2780,37 +2822,6 @@ nsNavHistoryFolderResultNode::OnItemRemoved(nsIURI* aBookmark, PRInt64 aFolder, } -// nsNavHistoryFolderResultNode::OnItemMoved (nsINavBookmarkObserver) -// -// Moved within our folder. - -NS_IMETHODIMP -nsNavHistoryFolderResultNode::OnItemMoved(nsIURI* aBookmark, PRInt64 aFolder, - PRInt32 aOldIndex, PRInt32 aNewIndex) -{ - NS_ASSERTION(aFolder == mFolderId, "Got wrong bookmark update"); - if (mOptions->ExcludeItems()) - return NS_OK; // don't update items when we aren't displaying them - if (! StartIncrementalUpdate()) - return NS_OK; - - PRUint32 nodeIndex; - nsNavHistoryResultNode* node = FindChildURI(aBookmark, &nodeIndex); - if (! node) - return NS_ERROR_FAILURE; // can't find bookmark - - // Just refresh the entire container: a lot of stuff (possibly even open - // containers) may have moved, and we don't want to handle all the cases - // here in this uncommon and non-time-critical update. - nsNavHistoryResult* result = GetResult(); - NS_ENSURE_TRUE(result, NS_ERROR_FAILURE); - if (result->GetView()) - result->GetView()->InvalidateContainer( - NS_STATIC_CAST(nsNavHistoryContainerResultNode*, this)); - return NS_OK; -} - - // nsNavHistoryFolderResultNode::OnItemChanged (nsINavBookmarkObserver) NS_IMETHODIMP @@ -2948,7 +2959,10 @@ nsNavHistoryFolderResultNode::OnFolderAdded(PRInt64 aFolder, PRInt64 aParent, { NS_ASSERTION(aParent == mFolderId, "Got wrong bookmark update"); if (! StartIncrementalUpdate()) - return NS_OK; + return NS_OK; // folder was completely refreshed for us + + // adjust indices to account for insertion + ReindexRange(aIndex, PR_INT32_MAX, 1); nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService(); NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY); @@ -2956,6 +2970,7 @@ nsNavHistoryFolderResultNode::OnFolderAdded(PRInt64 aFolder, PRInt64 aParent, nsNavHistoryResultNode* node; nsresult rv = bookmarks->ResultNodeForFolder(aFolder, mOptions, &node); NS_ENSURE_SUCCESS(rv, rv); + node->mBookmarkIndex = aIndex; if (GetSortType() == nsINavHistoryQueryOptions::SORT_BY_NONE) { // insert at natural bookmarks position @@ -2980,9 +2995,11 @@ nsNavHistoryFolderResultNode::OnFolderRemoved(PRInt64 aFolder, PRInt64 aParent, NS_ASSERTION(aParent == mFolderId, "Got wrong bookmark update"); if (! StartIncrementalUpdate()) - return NS_OK; + return NS_OK; // we are completely refreshed + + // update indices + ReindexRange(aIndex + 1, PR_INT32_MAX, -1); - // trust our own index over what the bookmark service provides PRUint32 index; nsNavHistoryFolderResultNode* node = FindChildFolder(aFolder, &index); if (! node) { @@ -2994,9 +3011,6 @@ nsNavHistoryFolderResultNode::OnFolderRemoved(PRInt64 aFolder, PRInt64 aParent, // nsNavHistoryFolderResultNode::OnFolderMoved (nsINavBookmarkObserver) -// -// This scenario should not be too common and is not performance critical. -// Therefore, we just do two steps, a remove and an insert. NS_IMETHODIMP nsNavHistoryFolderResultNode::OnFolderMoved(PRInt64 aFolder, PRInt64 aOldParent, @@ -3006,12 +3020,45 @@ nsNavHistoryFolderResultNode::OnFolderMoved(PRInt64 aFolder, PRInt64 aOldParent, NS_ASSERTION(aOldParent == mFolderId || aNewParent == mFolderId, "Got a bookmark message that doesn't belong to us"); if (! StartIncrementalUpdate()) - return NS_OK; + return NS_OK; // entire container was refreshed for us - if (aOldParent == mFolderId) - OnFolderRemoved(aFolder, aOldParent, aOldIndex); - if (aNewParent == mFolderId) - OnFolderAdded(aFolder, aNewParent, aNewIndex); + if (aOldParent == aNewParent) { + // getting moved within the same folder, we don't want to do a remove and + // an add because that will lose your tree state. + + // adjust bookmark indices + ReindexRange(aOldIndex + 1, PR_INT32_MAX, -1); + ReindexRange(aNewIndex, PR_INT32_MAX, 1); + + PRUint32 index; + nsNavHistoryFolderResultNode* node = FindChildFolder(aFolder, &index); + if (! node) { + NS_NOTREACHED("Can't find folder that is moving!"); + return NS_ERROR_FAILURE; + } + NS_ASSERTION(index >= 0 && index < PRUint32(mChildren.Count()), + "Invalid index!"); + node->mBookmarkIndex = aNewIndex; + + // adjust position + PRInt32 sortType = GetSortType(); + SortComparator comparator = GetSortingComparator(sortType); + if (DoesChildNeedResorting(index, comparator)) { + // needs resorting, this will cause everything to be redrawn, so we + // don't need to do that explicitly later. + nsRefPtr lock(node); + RemoveChildAt(index, PR_TRUE); + InsertChildAt(node, FindInsertionPoint(node, comparator), PR_TRUE); + return NS_OK; + } + + } else { + // moving between two different folders, just do a remove and an add + if (aOldParent == mFolderId) + OnFolderRemoved(aFolder, aOldParent, aOldIndex); + if (aNewParent == mFolderId) + OnFolderAdded(aFolder, aNewParent, aNewIndex); + } return NS_OK; } @@ -3075,22 +3122,24 @@ NS_IMETHODIMP nsNavHistoryFolderResultNode::OnSeparatorAdded(PRInt64 aParent, PRInt32 aIndex) { NS_ASSERTION(aParent == mFolderId, "Got wrong bookmark update"); - if (mOptions->ExcludeItems()) - return NS_OK; // don't update items when we aren't displaying them - if (! StartIncrementalUpdate()) - return NS_OK; - - // We remove all separators if the folder view is sorted, so only - // bother updating if there is no sort in effect. - if (GetSortType() != nsINavHistoryQueryOptions::SORT_BY_NONE) { + if (mOptions->ExcludeItems()) { + // don't update items when we aren't displaying them, except we need to + // update the indices + ReindexRange(aIndex, PR_INT32_MAX, 1); return NS_OK; } + if (! StartIncrementalUpdate()) + return NS_OK; // folder is completely refreshed for us + + // adjust indices + ReindexRange(aIndex, PR_INT32_MAX, 1); nsNavBookmarks* bookmarks = nsNavBookmarks::GetBookmarksService(); NS_ENSURE_TRUE(bookmarks, NS_ERROR_OUT_OF_MEMORY); nsNavHistoryResultNode* node = new nsNavHistorySeparatorResultNode(); NS_ENSURE_TRUE(node, NS_ERROR_OUT_OF_MEMORY); + node->mBookmarkIndex = aIndex; return InsertChildAt(node, aIndex); } @@ -3102,16 +3151,16 @@ nsNavHistoryFolderResultNode::OnSeparatorRemoved(PRInt64 aParent, PRInt32 aIndex) { NS_ASSERTION(aParent == mFolderId, "Got wrong bookmark update"); - if (mOptions->ExcludeItems()) - return NS_OK; // don't update items when we aren't displaying them - if (! StartIncrementalUpdate()) - return NS_OK; - - // We remove all separators if the folder view is sorted, so only - // bother updating if there is no sort in effect. - if (GetSortType() != nsINavHistoryQueryOptions::SORT_BY_NONE) { + if (mOptions->ExcludeItems()) { + // don't update items when we aren't displaying them, except we need to + // update everybody's indices + ReindexRange(aIndex, PR_INT32_MAX, -1); return NS_OK; } + if (! StartIncrementalUpdate()) + return NS_OK; // folder is completely refreshed for us + + ReindexRange(aIndex, PR_INT32_MAX, -1); if (aIndex >= mChildren.Count()) { NS_NOTREACHED("Removing separator at invalid index"); @@ -3126,25 +3175,6 @@ nsNavHistoryFolderResultNode::OnSeparatorRemoved(PRInt64 aParent, return RemoveChildAt(aIndex); } -// nsNavHistoryFolderResultNode::RecursiveSort - -void -nsNavHistoryFolderResultNode::RecursiveSort( - nsICollation* aCollation, SortComparator aComparator) -{ - if (GetSortType() != nsINavHistoryQueryOptions::SORT_BY_NONE) { - // We weren't sorted before, but now we are. - // Remove any separators so that they don't appear in the sorted list. - - for (PRUint32 i = mChildren.Count() - 1; i != PRUint32(-1); --i) { - if (mChildren[i]->IsSeparator()) { - mChildren.RemoveObjectAt(i); - } - } - } - - nsNavHistoryContainerResultNode::RecursiveSort(aCollation, aComparator); -} // nsNavHistorySeparatorResultNode // @@ -3529,20 +3559,6 @@ nsNavHistoryResult::OnItemRemoved(nsIURI *aBookmark, } -// nsNavHistoryResult::OnItemMoved (nsINavBookmarkObserver) - -NS_IMETHODIMP -nsNavHistoryResult::OnItemMoved(nsIURI *aBookmark, PRInt64 aFolder, - PRInt32 aOldIndex, PRInt32 aNewIndex) -{ - ENUMERATE_BOOKMARK_OBSERVERS_FOR_FOLDER(aFolder, - OnItemMoved(aBookmark, aFolder, aOldIndex, aNewIndex)); - ENUMERATE_HISTORY_OBSERVERS(OnItemMoved(aBookmark, aFolder, aOldIndex, aNewIndex)); - return NS_OK; - -} - - // nsNavHistoryResult::OnItemChanged (nsINavBookmarkObserver) NS_IMETHODIMP @@ -3941,6 +3957,14 @@ nsNavHistoryResultTreeViewer::BuildVisibleSection( } } + // don't display separators when sorted + if (cur->IsSeparator()) { + if (aContainer->GetSortType() != nsINavHistoryQueryOptions::SORT_BY_NONE) { + cur->mViewIndex = -1; + continue; + } + } + // add item cur->mViewIndex = aVisibleStartIndex + aVisible->Length(); if (! aVisible->AppendElement(cur)) @@ -4156,6 +4180,10 @@ nsNavHistoryResultTreeViewer::ItemInserted( { if (! mTree) return NS_OK; // nothing to do + if (! mResult) { + NS_NOTREACHED("Got an inserted message with no result"); + return NS_ERROR_UNEXPECTED; + } nsRefPtr parent; aParent->QueryInterface(NS_GET_IID(nsNavHistoryContainerResultNode), @@ -4170,6 +4198,14 @@ nsNavHistoryResultTreeViewer::ItemInserted( return NS_ERROR_UNEXPECTED; } + // don't display separators when sorted + if (item->IsSeparator()) { + if (parent->GetSortType() != nsINavHistoryQueryOptions::SORT_BY_NONE) { + item->mViewIndex = -1; + return NS_OK; + } + } + // update parent when inserting the first item because twisty may have changed if (parent->mChildren.Count() == 1) ItemChanged(aParent); @@ -4977,6 +5013,16 @@ NS_IMETHODIMP nsNavHistoryResultTreeViewer::GetCellText(PRInt32 row, NS_ENSURE_SUCCESS(rv, rv); _retval = value; } +#if 0 + // this code is for debugging bookmark indices. It will prepend the + // bookmark index to the title of each item. + nsString newTitle; + newTitle.AssignLiteral("("); + newTitle.AppendInt(node->mBookmarkIndex); + newTitle.AppendLiteral(") "); + newTitle.Append(_retval); + _retval = newTitle; +#endif break; } case Column_URI: @@ -5089,33 +5135,65 @@ nsNavHistoryResultTreeViewer::CycleHeader(nsITreeColumn* col) PRInt32 colIndex; col->GetIndex(&colIndex); + // Sometimes you want a tri-state sorting, and sometimes you don't. This + // rule allows tri-state sorting when the root node is a folder. This will + // catch the most common cases. When you are looking at folders, you want + // the third state to reset the sorting to the natural bookmark order. + // When you are looking at history, that third state has no meaning so we + // try to disallow it. + // + // The problem occurs when you have a query that results in bookmark folders. + // One example of this is the subscriptions view. In these cases, this rule + // doesn't allow you to sort those sub-folders by their natural order. + PRBool allowTriState = PR_FALSE; + if (mResult->mRootNode->IsFolder()) + allowTriState = PR_TRUE; + PRInt32 oldSort = mResult->mSortingMode; PRInt32 newSort; switch (GetColumnType(col)) { case Column_Title: - if (oldSort == nsINavHistoryQueryOptions::SORT_BY_TITLE_ASCENDING) + if (oldSort == nsINavHistoryQueryOptions::SORT_BY_TITLE_ASCENDING) { newSort = nsINavHistoryQueryOptions::SORT_BY_TITLE_DESCENDING; - else - newSort = nsINavHistoryQueryOptions::SORT_BY_TITLE_ASCENDING; + } else { + if (allowTriState && oldSort == nsINavHistoryQueryOptions::SORT_BY_TITLE_DESCENDING) + newSort = nsINavHistoryQueryOptions::SORT_BY_NONE; + else + newSort = nsINavHistoryQueryOptions::SORT_BY_TITLE_ASCENDING; + } break; case Column_URI: - if (oldSort == nsINavHistoryQueryOptions::SORT_BY_URI_ASCENDING) + if (oldSort == nsINavHistoryQueryOptions::SORT_BY_URI_ASCENDING) { newSort = nsINavHistoryQueryOptions::SORT_BY_URI_DESCENDING; - else - newSort = nsINavHistoryQueryOptions::SORT_BY_URI_ASCENDING; + } else { + if (allowTriState && oldSort == nsINavHistoryQueryOptions::SORT_BY_URI_DESCENDING) + newSort = nsINavHistoryQueryOptions::SORT_BY_NONE; + else + newSort = nsINavHistoryQueryOptions::SORT_BY_URI_ASCENDING; + } break; case Column_Date: - if (oldSort == nsINavHistoryQueryOptions::SORT_BY_DATE_ASCENDING) + if (oldSort == nsINavHistoryQueryOptions::SORT_BY_DATE_ASCENDING) { newSort = nsINavHistoryQueryOptions::SORT_BY_DATE_DESCENDING; - else - newSort = nsINavHistoryQueryOptions::SORT_BY_DATE_ASCENDING; + } else { + if (allowTriState && oldSort == nsINavHistoryQueryOptions::SORT_BY_DATE_DESCENDING) + newSort = nsINavHistoryQueryOptions::SORT_BY_NONE; + else + newSort = nsINavHistoryQueryOptions::SORT_BY_DATE_ASCENDING; + } break; case Column_VisitCount: - // visit count default is unusual because it is descending - if (oldSort == nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_DESCENDING) + // visit count default is unusual because we sort by descending by default + // because you are most likely to be looking for highly visited sites when + // you click it + if (oldSort == nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_DESCENDING) { newSort = nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_ASCENDING; - else - newSort = nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_DESCENDING; + } else { + if (allowTriState && oldSort == nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_ASCENDING) + newSort = nsINavHistoryQueryOptions::SORT_BY_NONE; + else + newSort = nsINavHistoryQueryOptions::SORT_BY_VISITCOUNT_DESCENDING; + } break; default: return NS_ERROR_INVALID_ARG;