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 cappuccino#2317
cacaodev · Feb 24, 2015 · 808411b · 808411b
1 parent 3adfa3c
commit 808411b
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 6 deletions.
diff --git 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
@@ -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)],