From a0c7305334a37fddb5ffb314b5d764900d5585d0 Mon Sep 17 00:00:00 2001
From: Alexander Ljungberg <aljungberg@wireload.net>
Date: Thu, 26 Apr 2012 15:19:18 +0100
Subject: [PATCH] Fixed: collection views cleared their selection whenever the
 content was changed.

This was unlike Cocoa and prevented CPArrayController's selectsInsertedObjects from working.
---
 AppKit/CPCollectionView.j           | 25 ++++++++++++++++---------
 Tests/AppKit/CPCollectionViewTest.j | 20 ++++++++++++++++++--
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/AppKit/CPCollectionView.j b/AppKit/CPCollectionView.j
index 6507978133..1ed66000d3 100644
--- a/AppKit/CPCollectionView.j
+++ b/AppKit/CPCollectionView.j
@@ -242,13 +242,15 @@
     Sets the content of the collection view to the content in \c anArray.
     This array can be of any type, and each element will be passed to the \c -setRepresentedObject: method.
     It's the responsibility of your custom collection view item to interpret the object.
-    @param anArray the content array
+
+    If the new content array is smaller than the previous one, note that [receiver selectionIndexes] may
+    refer to out of range indices. \c selectionIndexes is not changed as a result of calling the
+    \c setContent: method.
+
+    @param anArray a content array
 */
 - (void)setContent:(CPArray)anArray
 {
-    // reset the _selectionIndexes
-    [self setSelectionIndexes:[CPIndexSet indexSet]];
-
     _content = anArray;
 
     [self reloadContent];
@@ -284,9 +286,11 @@
 
     if (!_isSelectable)
     {
-        var index = CPNotFound;
+        var index = CPNotFound,
+            itemCount = [_items count];
 
-        while ((index = [_selectionIndexes indexGreaterThanIndex:index]) != CPNotFound)
+        // Be wary of invalid selection ranges since setContent: does not clear selection indexes.
+        while ((index = [_selectionIndexes indexGreaterThanIndex:index]) != CPNotFound && index < itemCount)
             [_items[index] setSelected:NO];
     }
 }
@@ -345,9 +349,11 @@
     if (!_isSelectable || [_selectionIndexes isEqual:anIndexSet])
         return;
 
-    var index = CPNotFound;
+    var index = CPNotFound,
+        itemCount = [_items count];
 
-    while ((index = [_selectionIndexes indexGreaterThanIndex:index]) !== CPNotFound)
+    // Be wary of invalid selection ranges since setContent: does not clear selection indexes.
+    while ((index = [_selectionIndexes indexGreaterThanIndex:index]) !== CPNotFound && index < itemCount)
         [_items[index] setSelected:NO];
 
     _selectionIndexes = anIndexSet;
@@ -403,7 +409,8 @@
     }
 
     index = CPNotFound;
-    while ((index = [_selectionIndexes indexGreaterThanIndex:index]) != CPNotFound)
+    // Be wary of invalid selection ranges since setContent: does not clear selection indexes.
+    while ((index = [_selectionIndexes indexGreaterThanIndex:index]) != CPNotFound && index < count)
         [_items[index] setSelected:YES];
 
     [self tile];
diff --git a/Tests/AppKit/CPCollectionViewTest.j b/Tests/AppKit/CPCollectionViewTest.j
index eefe2a9bcb..deabdecbe4 100644
--- a/Tests/AppKit/CPCollectionViewTest.j
+++ b/Tests/AppKit/CPCollectionViewTest.j
@@ -70,11 +70,14 @@
     [self assert:"selection changed: 1" equals:_globalResults];
     _globalResults = nil;
 
-    // setting the same content again should still trigger the delegate but with no selection
+    // setContent: by itself should not affect the selection.
     [_collectionView setContent:content1];
-    [self assert:"selection changed: 0" equals:_globalResults];
+    [self assert:nil equals:_globalResults];
+    // Manually clear the selection.
+    [_collectionView setSelectionIndexes:[CPIndexSet indexSet]];
     _globalResults = nil;
 
+
     // now lets change the contents
     [_collectionView setContent:content2];
     // we set the selection to 0 again, but on the NEW content
@@ -94,6 +97,19 @@
     [self assert:[CPIndexSet indexSet] equals:[_collectionView selectionIndexes]];
 }
 
+- (void)testSetContentAndSelectionIndexes
+{
+    // Changing the content does not automatically clear the selection indexes. The previous
+    // selection indexes are preserved, even if now invalid or out of range. This is what
+    // Cocoa does and necessary to prevent a new empty selection from overwriting
+    // CPArrayController's selectsInsertedObjects selections.
+
+    [_collectionView setContent:[1, 2, 3]];
+    [_collectionView setSelectionIndexes:[CPIndexSet indexSetWithIndex:1]];
+    [_collectionView setContent:[3, 1, 2]];
+    [self assert:[CPIndexSet indexSetWithIndex:1] equals:[_collectionView selectionIndexes]];
+}
+
 @end
 
 @implementation _CPCollectionViewWithHooks : CPCollectionView