FLEX-34424 CAUSE: HierarchicalCollectionViewCursor deals with deletions of nodes before the current node in two distinct ways: 1) relatively: if it's possible, it calls movePrevious() as many times as the number of nodes removed; if not, 2) absolutely: it seeks to the location of the first deleted node from the beginning of the hierarchy. The problem was in the conditions used to determine whether to use method 1) or 2): indeed, method 1) should be used when the node that used to be at 'currentIndex' has been directly affected by the deletion (i.e. that node is among the ones deleted). However, it should also be used when it has been indirectly (but irretrievably) affected, because enough nodes have been removed from its siblings collection that there is no item anymore on the value of the CursorBookmark we have stored for it (i.e. if it was the 8th node in a collection of 9 nodes, and two nodes before it have been deleted, there's no 8th node to retrieve anymore). The way we know this is that HierarchicalCollectionViewCursor.current now returns null. (So when 'current' was null, it was impossible to retrieve the parent stack of the items up to the root, and everything would fail from there onward.)
SOLUTION: Choose deletion strategy 1) when 'current == null'. NOTES: -This now makes HierarchicalCollectionViewCursor_FLEX_34424_Test pass. -This implies that the computation of the parent nodes (in the 'parentStack' array) now happens after this check, whereas it used to happen before. -It also implies that the index HierarchicalCollectionViewCursor seeks to in strategy 1) is different when 'current == null' (namely, it's 'currentIndex - n'). -Renamed some variables in these conditional statements, to make it easier to decypher what is happening. Project: http://git-wip-us.apache.org/repos/asf/flex-sdk/repo Commit: http://git-wip-us.apache.org/repos/asf/flex-sdk/commit/82d6b516 Tree: http://git-wip-us.apache.org/repos/asf/flex-sdk/tree/82d6b516 Diff: http://git-wip-us.apache.org/repos/asf/flex-sdk/diff/82d6b516 Branch: refs/heads/develop Commit: 82d6b51694e846f0fb4e5505bf5a1b0601c21e7b Parents: cc5f71c Author: Mihai Chira <mih...@apache.org> Authored: Mon Jul 28 17:44:51 2014 +0100 Committer: Mihai Chira <mih...@apache.org> Committed: Mon Jul 28 18:21:11 2014 +0100 ---------------------------------------------------------------------- .../HierarchicalCollectionViewCursor.as | 27 ++++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/flex-sdk/blob/82d6b516/frameworks/projects/advancedgrids/src/mx/collections/HierarchicalCollectionViewCursor.as ---------------------------------------------------------------------- diff --git a/frameworks/projects/advancedgrids/src/mx/collections/HierarchicalCollectionViewCursor.as b/frameworks/projects/advancedgrids/src/mx/collections/HierarchicalCollectionViewCursor.as index df6ee81..954d948 100644 --- a/frameworks/projects/advancedgrids/src/mx/collections/HierarchicalCollectionViewCursor.as +++ b/frameworks/projects/advancedgrids/src/mx/collections/HierarchicalCollectionViewCursor.as @@ -1145,16 +1145,12 @@ public class HierarchicalCollectionViewCursor extends EventDispatcher var changingNode:Object; var parentOfChangingNode:Object; var parentOfCurrentNode:Object; - var parentStack:Array = getParentStack(current); + var parentStack:Array; var isBefore:Boolean = false; var parentOfChangingNodeIndex:int; var isChangingNodeParentAncestorOfSelectedNode:Boolean; var bookmarkInChangingCollection:CursorBookmark; var changingNodeCollectionBookmarkIndex:int; - - // remember the current parent - parentOfCurrentNode = parentStack[parentStack.length - 1]; - var changingNodeAndSiblings:ICollectionView; var changingCollectionCursor:IViewCursor; @@ -1167,6 +1163,9 @@ public class HierarchicalCollectionViewCursor extends EventDispatcher isBefore = true; } + parentStack = getParentStack(current); + parentOfCurrentNode = parentStack[parentStack.length - 1]; + for (i = 0; i < n; i++) { changingNode = event.items[i]; @@ -1226,20 +1225,23 @@ public class HierarchicalCollectionViewCursor extends EventDispatcher n = event.items.length; if (event.location <= currentIndex) { - if (event.location + n >= currentIndex) + var lastIndexAffectedByDeletion:int = event.location + n; + var isCurrentIndexAmongRemovedNodes:Boolean = lastIndexAffectedByDeletion >= currentIndex; + var currentItemNotFoundAmongItsSiblings:Boolean = isCurrentIndexAmongRemovedNodes ? false : current == null; + + if (isCurrentIndexAmongRemovedNodes || currentItemNotFoundAmongItsSiblings) { - // the current node was removed // the list classes expect that we // leave the cursor on whatever falls // into that slot - var newIndex:int = event.location; + var indexToReturnTo:int = isCurrentIndexAmongRemovedNodes ? event.location : currentIndex - n; moveToFirst(); - seek(CursorBookmark.FIRST, newIndex); + seek(CursorBookmark.FIRST, indexToReturnTo); for (i = 0; i < n; i++) { - changingNode = event.items[i]; - delete collection.parentMap[UIDUtil.getUID(changingNode)]; + delete collection.parentMap[UIDUtil.getUID(event.items[i])]; } + return; } @@ -1247,6 +1249,9 @@ public class HierarchicalCollectionViewCursor extends EventDispatcher isBefore = true; } + parentStack = getParentStack(current); + parentOfCurrentNode = parentStack[parentStack.length - 1]; + for (i = 0; i < n; i++) { changingNode = event.items[i];