From 808411bc0bd64fb63bd53af9dbf332bf0735421f Mon Sep 17 00:00:00 2001
From: cacaodev <cacaodev@gmail.com>
Date: Tue, 24 Feb 2015 16:15:20 +0100
Subject: [PATCH] FIXED: Exception when rows were removed in a table with
 binded columns.

Before this fix, the CPTableColumn binder was not correctly reloading
the table when the number of rows changed. Now we reload fully the
dataviews when the number of rows changes. If no rows are
inserted/deleted, there is an optimization: we just need to reload the
objectValues and leave the dataviews untouched.

With Test in Tests/AppKitCPTableViewTest.j
Fixes #2317
---
 AppKit/CPTableColumn.j         | 21 ++++++++++-----
 Tests/AppKit/CPTableViewTest.j | 47 ++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/AppKit/CPTableColumn.j b/AppKit/CPTableColumn.j
index fe7418ce1..08de6d80c 100644
--- a/AppKit/CPTableColumn.j
+++ b/AppKit/CPTableColumn.j
@@ -546,13 +546,22 @@ CPTableColumnUserResizingMask   = 1 << 1;
 - (void)setValueFor:(CPString)aBinding
 {
     var tableView = [_source tableView],
-        column = [[tableView tableColumns] indexOfObjectIdenticalTo:_source],
-        rowIndexes = [CPIndexSet indexSetWithIndexesInRange:CPMakeRange(0, [tableView numberOfRows])],
-        columnIndexes = [CPIndexSet indexSetWithIndex:column];
+        newNumberOfRows = [tableView _numberOfRows];
 
-    // Reloads objectValues only, not the views.
-    // FIXME: reload data for all rows or just exposed rows ?
-    [tableView _reloadDataForRowIndexes:rowIndexes columnIndexes:columnIndexes];
+    if ([tableView numberOfRows] == newNumberOfRows)
+    {
+        var rowIndexes = [CPIndexSet indexSetWithIndexesInRange:CPMakeRange(0, newNumberOfRows)],
+            column = [[tableView tableColumns] indexOfObjectIdenticalTo:_source],
+            columnIndexes = [CPIndexSet indexSetWithIndex:column];
+
+        // Reloads objectValues only, not the views.
+        // FIXME: reload data for all rows or just rows intersecting exposed rows ?
+        [tableView _reloadDataForRowIndexes:rowIndexes columnIndexes:columnIndexes];
+    }
+    else
+    {
+        [tableView reloadData];
+    }
 }
 
 - (CPSortDescriptor)_defaultSortDescriptorPrototype
diff --git a/Tests/AppKit/CPTableViewTest.j b/Tests/AppKit/CPTableViewTest.j
index 880dbcd99..0288bd017 100644
--- a/Tests/AppKit/CPTableViewTest.j
+++ b/Tests/AppKit/CPTableViewTest.j
@@ -278,6 +278,53 @@
     [contentBindingTable reloadData];
 }
 
+- (void)testColumnValueBinding
+{
+    var table = [[CPTableView alloc] initWithFrame:CGRectMake(0, 0, 100, 100)],
+        container = [TestDataSource new];
+
+    var tc = [[CPTableColumn alloc] initWithIdentifier:@"A"];
+    [table addTableColumn:tc];
+
+    [container setTableEntries:@[@{@"name":@"B1"}, @{@"name":@"B2"}, @{@"name":@"B3"}]];
+
+    var ac = [[CPArrayController alloc] init];
+    [ac bind:@"contentArray" toObject:container withKeyPath:@"tableEntries" options:nil];
+    [tc bind:@"value" toObject:ac withKeyPath:@"arrangedObjects.name" options:nil];
+
+    [[theWindow contentView] addSubview:table];
+    [theWindow makeFirstResponder:table];
+
+    [[CPRunLoop currentRunLoop] limitDateForMode:CPDefaultRunLoopMode];
+
+    // Should remove all table rows.
+    [self assertNoThrow:function()
+    {
+        [container setTableEntries:@[]];
+    }];
+
+    [container setTableEntries:@[@{@"name":@"B1"}, @{@"name":@"B2"}, @{@"name":@"B3"}]];
+
+    [self assertNoThrow:function()
+    {
+        [container setTableEntries:nil];
+    }];
+
+    [container setTableEntries:@[@{@"name":@"B1"}, @{@"name":@"B2"}, @{@"name":@"B3"}]];
+
+    // Change a value in row 0. The number of rows stays the same.
+    [container setTableEntries:@[@{@"name":@"B4"}, @{@"name":@"B2"}, @{@"name":@"B3"}]];
+
+    // Checks that the displayed data matches the model data.
+    [table enumerateAvailableViewsUsingBlock:function(dataView, aRow, aColumn, stop)
+    {
+        var data_value = [[[container tableEntries] objectAtIndex:aRow] objectForKey:@"name"];
+        [self assert:[dataView objectValue] equals:data_value];
+    }];
+
+    // We could also check that the dataviews in rows 1 and 2 are identical to the ones before mutation.
+}
+
 - (void)testInitiallyHiddenColumns
 {
     var table = [[CPTableView alloc] initWithFrame:CGRectMake(0, 0, 400, 400)],