From dcf440396d2cd2e9aeb23e55e18022682a123048 Mon Sep 17 00:00:00 2001
From: Alexander Ljungberg <aljungberg@wireload.net>
Date: Thu, 13 Jan 2011 18:57:36 -0300
Subject: [PATCH] Fixed: the outline view was temporarily inconsistent to
 outside observers while expanding a node.

In particular, a selection did change notification was sent before the rows the selection referred to had had a chance to be loaded.
---
 AppKit/CPOutlineView.j           | 11 +++++++++--
 Tests/AppKit/CPOutlineViewTest.j | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/AppKit/CPOutlineView.j b/AppKit/CPOutlineView.j
index f27952646..a2b8b8f2f 100644
--- a/AppKit/CPOutlineView.j
+++ b/AppKit/CPOutlineView.j
@@ -333,7 +333,8 @@ CPOutlineViewDropOnItemIndex = -1;
         [self _noteItemWillExpand:anItem];
 
         // Shift selection indexes below so that the same items remain selected.
-        var newRowCount = [_outlineViewDataSource outlineView:self numberOfChildrenOfItem:anItem];
+        var newRowCount = [_outlineViewDataSource outlineView:self numberOfChildrenOfItem:anItem],
+            newSelection = nil;
         if (newRowCount)
         {
             var selection = [self selectedRowIndexes],
@@ -343,13 +344,19 @@ CPOutlineViewDropOnItemIndex = -1;
             {
                 [self _noteSelectionIsChanging];
                 [selection shiftIndexesStartingAtIndex:expandIndex by:newRowCount];
-                [self _setSelectedRowIndexes:selection];
+                newSelection = selection;
             }
         }
 
         itemInfo.isExpanded = YES;
         [self _noteItemDidExpand:anItem];
         [self reloadItem:anItem reloadChildren:YES];
+
+        // Update the selection - and send the associated notification - first
+        // after the items have loaded so that the new selection is consistent
+        // with the actual rows for any observers.
+        if (newSelection !== nil)
+            [self _setSelectedRowIndexes:selection];
     }
 
     if (shouldExpandChildren)
diff --git a/Tests/AppKit/CPOutlineViewTest.j b/Tests/AppKit/CPOutlineViewTest.j
index 0359f4fb5..25dcef986 100644
--- a/Tests/AppKit/CPOutlineViewTest.j
+++ b/Tests/AppKit/CPOutlineViewTest.j
@@ -112,17 +112,23 @@
 
     var preSelection = [CPIndexSet indexSet];
     [preSelection addIndex:[outlineView rowForItem:".1.1"]];
-    [preSelection addIndex:[outlineView rowForItem:".2"]];
+    [preSelection addIndex:[outlineView rowForItem:".3.1"]];
 
     [outlineView selectRowIndexes:preSelection byExtendingSelection:NO];
 
+    // Test that by the time the selection notification is sent out, rows have
+    // been expanded. E.g. the outline view is made consistent before notifying.
+    var delegate = [TestNotificationsDelegate new];
+    [delegate setTester:self];
+    [outlineView setDelegate:delegate];
+
     [outlineView expandItem:".1.2"];
     afterSelection = [outlineView selectedRowIndexes];
 
     [self assert:2 equals:[afterSelection count] message:"selections should remain"];
 
     [self assert:".1.1" equals:[outlineView itemAtRow:[afterSelection firstIndex]] message:".1.1 selection should remain"];
-    [self assert:".2" equals:[outlineView itemAtRow:[afterSelection lastIndex]] message:".2 selection should remain"];
+    [self assert:".3.1" equals:[outlineView itemAtRow:[afterSelection lastIndex]] message:".3.1 selection should remain"];
 }
 
 @end
@@ -167,3 +173,25 @@
 }
 
 @end
+
+@implementation TestNotificationsDelegate : CPObject
+{
+    id  tester @accessors;
+}
+
+- (void)outlineViewSelectionDidChange:(CPNotification)aNotification
+{
+    // Verify that the state is consistent - every selected row has been loaded.
+    var anOutlineView = [aNotification object],
+        selection = [anOutlineView selectedRowIndexes],
+        rows = [];
+
+    [selection getIndexes:rows maxCount:-1 inIndexRange:nil];
+
+    for (var i = 0, count = [rows count]; i < count; i++)
+    {
+        [tester assertTrue:[anOutlineView itemAtRow:rows[i]] !== nil message:"selected row " + i + " should exist"];
+    }
+}
+
+@end