Bug 408659 - Tree selection shouldn't go away when an item is resorted or moved. r=dietrich.

This commit is contained in:
mozilla.mano@sent.com 2007-12-28 18:59:22 -08:00
parent b72e37db8d
commit 1ff1ed4f9e
7 changed files with 195 additions and 83 deletions

View File

@ -288,6 +288,26 @@
}
},
itemMoved:
function PMV_itemMoved(aItem, aOldParent, aOldIndex, aNewParent,
aNewIndex) {
// This cannot actually happen yet (see IDL)
if (aNewParent != aOldParent);
return;
var popup = this._getPopupForContainer(aNewParent);
var index = popup._startMarker + 1 + aNewIndex;
var children = popup.childNodes;
for (var i = 0; i < children.length; i++) {
var menuItem = children[i];
if (menuItem.node == aItem) {
popup.removeChild(menuItem);
popup.insertBefore(menuItem, children[index]);
return;
}
}
},
itemChanged: function PMV_itemChanged(aNode) {
// this check can be removed once we fix bug #382397
var parentNode = aNode.parent;

View File

@ -496,6 +496,37 @@
}
},
itemMoved:
function TV_V_itemMoved(aItem, aOldParent, aOldIndex, aNewParent,
aNewIndex) {
// This cannot actually happen yet (see IDL)
if (aNewParent != aOldParent);
return;
if (aNewParent == this._self.getResultNode()) {
var children = this._self.childNodes;
for (var i = 0; i < children.length; i++) {
var button = children[i];
if (button.node == aItem) {
this._self.removeChild(button);
this._self.insertBefore(button, children[aNewIndex]);
this._self.updateChevron();
return;
}
}
}
var popup = this._getPopupForContainer(aNewParent);
var children = popup.childNodes;
for (var i = 0; i < children.length; i++) {
var menuItem = children[i];
if (menuItem.node == aItem) {
popup.removeChild(menuItem);
popup.insertBefore(menuItem, children[aNewIndex]);
return;
}
}
},
itemChanged: function TV_V_itemChanged(aNode) {
// this check can be removed once we fix bug #382397
var parentNode = aNode.parent;

View File

@ -298,6 +298,13 @@ PlacesTreeView.prototype = {
var startReplacement = aContainer.viewIndex + 1;
var replaceCount = this._countVisibleRowsForItem(aContainer);
// We don't replace the container item itself so we decrease the
// replaceCount by 1. We don't do so though if there is no visible item
// for the container. This happens when aContainer is the root node and
// showRoot is not set.
if (aContainer.viewIndex != -1)
replaceCount-=1;
// Persist selection state
var nodesToSelect = [];
var selection = this.selection;
@ -305,20 +312,14 @@ PlacesTreeView.prototype = {
for (var rangeIndex = 0; rangeIndex < rc; rangeIndex++) {
var min = { }, max = { };
selection.getRangeAt(rangeIndex, min, max);
if (min.value > startReplacement + replaceCount)
var lastIndex = Math.min(max.value, startReplacement + replaceCount -1);
if (min.value < startReplacement || min.value > lastIndex)
continue;
for (var nodeIndex = min.value; nodeIndex <= max.value; nodeIndex++)
for (var nodeIndex = min.value; nodeIndex <= lastIndex; nodeIndex++)
nodesToSelect.push(this._visibleElements[nodeIndex]);
}
// We don't replace the container item itself so we decrease the
// replaceCount by 1. We don't do so though if there is no visible item
// for the container. This happens when aContainer is the root node and
// showRoot is not set.
if (aContainer.viewIndex != -1)
replaceCount-=1;
// Mark the removes as invisible
for (var i = 0; i < replaceCount; i++)
this._visibleElements[startReplacement + i].viewIndex = -1;
@ -660,7 +661,7 @@ PlacesTreeView.prototype = {
itemRemoved: function PTV_itemRemoved(aParent, aItem, aOldIndex) {
NS_ASSERT(this._result, "Got a notification but have no result!");
if (!this._tree)
return; // nothing to do
return; // nothing to do
var oldViewIndex = aItem.viewIndex;
if (oldViewIndex < 0)
@ -711,6 +712,55 @@ PlacesTreeView.prototype = {
this.itemChanged(aParent);
},
/**
* Be careful, aOldIndex and aNewIndex specify the index in the
* corresponding parent nodes, not the visible indexes.
*/
itemMoved:
function PTV_itemMoved(aItem, aOldParent, aOldIndex, aNewParent, aNewIndex) {
NS_ASSERT(this._result, "Got a notification but have no result!");
if (!this._tree)
return; // nothing to do
var oldViewIndex = aItem.viewIndex;
if (oldViewIndex < 0)
return; // item was already invisible, nothing to do
// this may have been a container, in which case it has a lot of rows
var count = this._countVisibleRowsForItem(aItem);
// Persist selection state
var nodesToSelect = [];
var selection = this.selection;
var rc = selection.getRangeCount();
for (var rangeIndex = 0; rangeIndex < rc; rangeIndex++) {
var min = { }, max = { };
selection.getRangeAt(rangeIndex, min, max);
var lastIndex = Math.min(max.value, oldViewIndex + count -1);
if (min.value < oldViewIndex || min.value > lastIndex)
continue;
for (var nodeIndex = min.value; nodeIndex <= lastIndex; nodeIndex++)
nodesToSelect.push(this._visibleElements[nodeIndex]);
}
if (nodesToSelect.length > 0)
selection.selectEventsSuppressed = true;
// remove the nodes, let itemInserted restore all of its contents
this._visibleElements.splice(oldViewIndex, count);
this._tree.rowCountChanged(oldViewIndex, -count);
this.itemInserted(aNewParent, aItem, aNewIndex);
// restore selection
if (nodesToSelect.length > 0) {
for each (var node in nodesToSelect) {
var index = node.viewIndex;
selection.rangedSelect(index, index, true);
}
selection.selectEventsSuppressed = false;
}
},
/**
* Be careful, the parameter 'aIndex' here specifies the index in the parent
* node of the item, not the visible index.

View File

@ -485,27 +485,42 @@ interface nsINavHistoryResultViewObserver : nsISupports
*
* @see nsINavHistoryResult for where this fits in
*/
[scriptable, uuid(f208e54c-834f-4a6c-bd4d-a476015bc139)]
[scriptable, uuid(76cad155-21d5-427e-9740-01386cc8b923)]
interface nsINavHistoryResultViewer : nsISupports
{
/**
* Called when 'item' is inserted into 'parent' at index 'newIndex'. The item
* previously at index (if any) and everything below it will have been
* shifted down by one. The item may be a container or a leaf.
* Called when 'aItem' is inserted into 'aParent' at index 'aNewIndex'.
* The item previously at index (if any) and everything below it will have
* been shifted down by one. The item may be a container or a leaf.
*/
void itemInserted(in nsINavHistoryContainerResultNode parent,
in nsINavHistoryResultNode item,
in unsigned long newIndex);
void itemInserted(in nsINavHistoryContainerResultNode aParent,
in nsINavHistoryResultNode aItem,
in unsigned long aNewIndex);
/**
* Called whan 'item' is removed from 'parent' at 'oldIndex'. The item may be
* a container or a leaf. This function will be called after the item has
* been removed from its parent list, but before anything else (including
* Called whan 'aItem' is removed from 'aParent' at 'aOldIndex'. The item
* may be a container or a leaf. This function will be called after the item
* has been removed from its parent list, but before anything else (including
* NULLing out the item's parent) has happened.
*/
void itemRemoved(in nsINavHistoryContainerResultNode parent,
in nsINavHistoryResultNode item,
in unsigned long oldIndex);
void itemRemoved(in nsINavHistoryContainerResultNode aParent,
in nsINavHistoryResultNode aItem,
in unsigned long aOldIndex);
/**
* Called whan 'aItem' is moved from 'aOldParent' at 'aOldIndex' to
* aNewParent at aNewIndex. The item may be a container or a leaf.
*
* XXX: at the moment, this method is called only when an item is moved
* within the same container. When an item is moved between containers,
* a new node is created for the item, and the itemRemoved/itemAdded methods
* are used.
*/
void itemMoved(in nsINavHistoryResultNode aItem,
in nsINavHistoryContainerResultNode aOldParent,
in unsigned long aOldIndex,
in nsINavHistoryContainerResultNode aNewParent,
in unsigned long aNewIndex);
/**
* Called when an item has been changed and should be repainted. This only

View File

@ -588,22 +588,10 @@ nsNavHistoryContainerResultNode::ReverseUpdateStats(PRInt32 aAccessCountChange)
SortComparator comparator = GetSortingComparator(sortMode);
nsCAutoString sortingAnnotation;
GetSortingAnnotation(sortingAnnotation);
int ourIndex = mParent->FindChild(this);
if (mParent->DoesChildNeedResorting(ourIndex, comparator, sortingAnnotation.get())) {
// prevent us from being destroyed when removed from the parent
nsRefPtr<nsNavHistoryContainerResultNode> ourLock = this;
nsNavHistoryContainerResultNode* ourParent = mParent;
// Performance: moving items by removing and re-inserting is not very
// efficient because there may be a lot of unnecessary renumbering of
// items. I don't think the overhead is worth the extra complexity in
// this case.
ourParent->RemoveChildAt(ourIndex, PR_TRUE);
ourParent->InsertSortedChild(this, PR_TRUE);
resorted = PR_TRUE;
}
PRUint32 ourIndex = mParent->FindChild(this);
resorted = EnsureItemPosition(ourIndex);
}
if (! resorted) {
if (!resorted) {
// repaint visible rows
nsNavHistoryResult* result = GetResult();
if (result && result->GetView() && mParent->AreChildrenVisible()) {
@ -1414,6 +1402,42 @@ nsNavHistoryContainerResultNode::InsertSortedChild(
return InsertChildAt(aNode, mChildren.Count(), aIsTemporary);
}
// nsNavHistoryContainerResultNode::EnsureItemPosition
//
// This checks if the item at aIndex is located correctly given the sorting
// move. If it's not, the item is moved, and the result view are notified.
//
// Returns true if the item position has been changed, false otherwise.
PRBool
nsNavHistoryContainerResultNode::EnsureItemPosition(PRUint32 aIndex) {
NS_ASSERTION(aIndex >= 0 && aIndex < mChildren.Count(), "Invalid index");
if (aIndex < 0 || aIndex >= mChildren.Count())
return PR_FALSE;
SortComparator comparator = GetSortingComparator(GetSortType());
if (!comparator)
return PR_FALSE;
nsCAutoString sortAnno;
GetSortingAnnotation(sortAnno);
if (!DoesChildNeedResorting(aIndex, comparator, sortAnno.get()))
return PR_FALSE;
nsRefPtr<nsNavHistoryResultNode> node(mChildren[aIndex]);
mChildren.RemoveObjectAt(aIndex);
PRUint32 newIndex = FindInsertionPoint(node, comparator,sortAnno.get());
mChildren.InsertObjectAt(node.get(), newIndex);
nsNavHistoryResult* result = GetResult();
NS_ENSURE_TRUE(result, PR_TRUE);
if (result->GetView() && AreChildrenVisible())
result->GetView()->ItemMoved(node, this, aIndex, this, newIndex);
return PR_TRUE;
}
// nsNavHistoryContainerResultNode::MergeResults
//
@ -1694,14 +1718,7 @@ nsNavHistoryContainerResultNode::UpdateURIs(PRBool aRecursive, PRBool aOnlyOne,
if (aUpdateSort) {
PRInt32 childIndex = parent->FindChild(node);
if (childIndex >= 0 && parent->DoesChildNeedResorting(childIndex, comparator,
sortingAnnotation.get())) {
// child position changed
parent->RemoveChildAt(childIndex, PR_TRUE);
parent->InsertChildAt(node, parent->FindInsertionPoint(node, comparator,
sortingAnnotation.get()),
PR_TRUE);
} else if (childrenVisible) {
if ((childIndex < 0 || !parent->EnsureItemPosition(childIndex) && childrenVisible)) {
result->GetView()->ItemChanged(node);
}
} else if (childrenVisible) {
@ -3379,18 +3396,8 @@ nsNavHistoryResultNode::OnItemChanged(PRInt64 aItemId,
// DO NOT OPTIMIZE THIS TO CHECK aProperty
// the sorting methods fall back to each other so we need to re-sort the
// result even if it's not set to sort by the given property
nsNavHistoryContainerResultNode::SortComparator comparator =
mParent->GetSortingComparator(mParent->GetSortType());
PRInt32 ourIndex = mParent->FindChild(this);
nsCAutoString sortAnno;
mParent->GetSortingAnnotation(sortAnno);
if (mParent->DoesChildNeedResorting(ourIndex, comparator, sortAnno.get())) {
nsCOMPtr<nsINavHistoryResultNode> nodeLock(this);
mParent->RemoveChildAt(ourIndex, PR_TRUE);
mParent->InsertChildAt(this,
mParent->FindInsertionPoint(this, comparator, sortAnno.get()),
PR_TRUE);
}
mParent->EnsureItemPosition(ourIndex);
return NS_OK;
}
@ -3446,14 +3453,7 @@ nsNavHistoryFolderResultNode::OnItemVisited(PRInt64 aItemId,
PRInt32 childIndex = FindChild(node);
NS_ASSERTION(childIndex >= 0, "Could not find child we just got a reference to");
if (childIndex >= 0) {
SortComparator comparator = GetSortingComparator(GetSortType());
nsCAutoString sortingAnnotation;
GetSortingAnnotation(sortingAnnotation);
nsCOMPtr<nsINavHistoryResultNode> nodeLock(node);
RemoveChildAt(childIndex, PR_TRUE);
InsertChildAt(node,
FindInsertionPoint(node, comparator, sortingAnnotation.get()),
PR_TRUE);
EnsureItemPosition(childIndex);
}
} else if (result->GetView() && AreChildrenVisible()) {
// no sorting changed, just redraw the row if visible
@ -3493,21 +3493,8 @@ nsNavHistoryFolderResultNode::OnItemMoved(PRInt64 aItemId, PRInt64 aOldParent,
node->mBookmarkIndex = aNewIndex;
// adjust position
PRInt32 sortType = GetSortType();
SortComparator comparator = GetSortingComparator(sortType);
nsCAutoString sortingAnnotation;
GetSortingAnnotation(sortingAnnotation);
if (DoesChildNeedResorting(index, comparator, sortingAnnotation.get())) {
// needs resorting, this will cause everything to be redrawn, so we
// don't need to do that explicitly later.
nsRefPtr<nsNavHistoryResultNode> lock(node);
RemoveChildAt(index, PR_TRUE);
InsertChildAt(node,
FindInsertionPoint(node, comparator, sortingAnnotation.get()),
PR_TRUE);
return NS_OK;
}
EnsureItemPosition(index);
return NS_OK;
} else {
// moving between two different folders, just do a remove and an add
if (aOldParent == mItemId)

View File

@ -653,6 +653,7 @@ public:
PRBool aIsTemporary = PR_FALSE);
nsresult InsertSortedChild(nsNavHistoryResultNode* aNode,
PRBool aIsTemporary = PR_FALSE);
PRBool EnsureItemPosition(PRUint32 aIndex);
void MergeResults(nsCOMArray<nsNavHistoryResultNode>* aNodes);
nsresult ReplaceChildURIAt(PRUint32 aIndex, nsNavHistoryResultNode* aNode);
nsresult RemoveChildAt(PRInt32 aIndex, PRBool aIsTemporary = PR_FALSE);

View File

@ -82,6 +82,10 @@ var viewer = {
dump("itemReplaced: " + newItem.uri + "\n");
this.replacedItem = item;
},
movedItem: null,
itemMoved: function(item, oldParent, oldIndex, newParent, newIndex) {
this.movedItem = item;
},
openedContainer: null,
containerOpened: function(item) {
this.openedContainer = item;
@ -112,6 +116,7 @@ var viewer = {
this.removedItem = null;
this.changedItem = null;
this.replacedItem = null;
this.movedItem = null;
this.openedContainer = null;
this.closedContainer = null;
this.invalidatedContainer = null;
@ -210,10 +215,13 @@ function run_test() {
bmsvc.setItemTitle(testBookmark, "baz");
do_check_eq(viewer.changedItem.title, "baz");
var testBookmark2 = bmsvc.insertBookmark(bmsvc.bookmarksMenuFolder, uri("http://google.com"), bmsvc.DEFAULT_INDEX, "foo");
bmsvc.moveItem(testBookmark2, bmsvc.bookmarksMenuFolder, 0);
do_check_eq(viewer.movedItem.itemId, testBookmark2);
// nsINavHistoryResultViewer.itemRemoved
var removedBookmark = bmsvc.insertBookmark(bmsvc.bookmarksMenuFolder, uri("http://google.com"), bmsvc.DEFAULT_INDEX, "foo");
bmsvc.removeItem(removedBookmark);
do_check_eq(removedBookmark, viewer.removedItem.itemId);
bmsvc.removeItem(testBookmark2);
do_check_eq(testBookmark2, viewer.removedItem.itemId);
// XXX nsINavHistoryResultViewer.itemReplaced
// NHQRN.onVisit()->NHCRN.MergeResults()->NHCRN.ReplaceChildURIAt()->NHRV.ItemReplaced()