From 0736f86f32903108a449750d9f20ae1902e5b830 Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Wed, 7 Mar 2012 16:49:41 +0000 Subject: [PATCH] Fixed: "[CPString count] unrecognized selector" error when binding an array controller's content array to a key of another AC's selection. This change also avoids wrapping a selection proxy in a KVO array proxy, which isn't helpful and probably hurts performance. --- AppKit/CPArrayController.j | 31 ++++++++++++++++++++++++---- AppKit/CPObjectController.j | 2 +- Tests/AppKit/CPArrayControllerTest.j | 19 +++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/AppKit/CPArrayController.j b/AppKit/CPArrayController.j index dbd318da7..359876612 100644 --- a/AppKit/CPArrayController.j +++ b/AppKit/CPArrayController.j @@ -1076,22 +1076,45 @@ var destination = [_info objectForKey:CPObservedObjectKey], keyPath = [_info objectForKey:CPObservedKeyPathKey], options = [_info objectForKey:CPOptionsKey], - isCompound = [self handlesContentAsCompoundValue]; + isCompound = [self handlesContentAsCompoundValue], + dotIndex = keyPath.lastIndexOf("."), + firstPart = dotIndex !== CPNotFound ? keyPath.substring(0, dotIndex) : nil, + isSelectionProxy = firstPart && [[destination valueForKeyPath:firstPart] isKindOfClass:CPControllerSelectionProxy]; - if (!isCompound) + if (!isCompound && !isSelectionProxy) { newValue = [destination mutableArrayValueForKeyPath:keyPath]; } else { - // handlesContentAsCompoundValue == YES so we cannot just set up a proxy. + // 1. If handlesContentAsCompoundValue we cannot just set up a proxy. // Every read and every write must go through transformValue and // reverseTransformValue, and the resulting object cannot be described by // a key path. + + // 2. If isSelectionProxy, we don't want to proxy a proxy - that's bad + // for performance and won't work with markers. + newValue = [destination valueForKeyPath:keyPath]; } - newValue = [self transformValue:newValue withOptions:options]; + var isPlaceholder = CPIsControllerMarker(newValue); + if (isPlaceholder) + { + if (newValue === CPNotApplicableMarker && [options objectForKey:CPRaisesForNotApplicableKeysBindingOption]) + { + [CPException raise:CPGenericException + reason:@"can't transform non applicable key on: " + _source + " value: " + newValue]; + } + + newValue = [self _placeholderForMarker:newValue]; + + // This seems to be what Cocoa does. + if (!newValue) + newValue = [CPMutableArray array]; + } + else + newValue = [self transformValue:newValue withOptions:options]; if (isCompound) { diff --git a/AppKit/CPObjectController.j b/AppKit/CPObjectController.j index 0a1bdae67..82589d689 100644 --- a/AppKit/CPObjectController.j +++ b/AppKit/CPObjectController.j @@ -678,7 +678,7 @@ var CPObjectControllerContentKey = @"CPObjectControllerCo { value = [theValues objectAtIndex:0]; - for (var i = 0, count= [theValues count]; i < count && value != CPMultipleValuesMarker; i++) + for (var i = 0, count = [theValues count]; i < count && value != CPMultipleValuesMarker; i++) { if (![value isEqual:[theValues objectAtIndex:i]]) value = CPMultipleValuesMarker; diff --git a/Tests/AppKit/CPArrayControllerTest.j b/Tests/AppKit/CPArrayControllerTest.j index f20caf7a9..1b676222c 100644 --- a/Tests/AppKit/CPArrayControllerTest.j +++ b/Tests/AppKit/CPArrayControllerTest.j @@ -627,6 +627,25 @@ [self assert:[CPIndexSet indexSetWithIndexesInRange:CPMakeRange(0, 4)] equals:indexes]; } +- (void)testBindToNoSelectionMarker +{ + var arrayController1 = [CPArrayController new], + arrayController2 = [CPArrayController new]; + + [arrayController2 setContent:[CPDictionary dictionaryWithObject:[1, 2, 3] forKey:@"x"]]; + + [arrayController1 bind:@"contentArray" toObject:arrayController2 withKeyPath:@"selection.x" options:nil]; + // This used to cause a bug where the _CPKVCArray wrapping the selection proxy tried to call 'count' on + // CPNoSelectionMarker while attempting to copy itself. + [arrayController2 setSelectionIndexes:[CPIndexSet indexSet]]; + + [self assert:[] equals:[arrayController1 arrangedObjects] message:"arranged objects of an empty selection should be empty"]; + + // Make sure the regular case works. + [arrayController2 setSelectionIndexes:[CPIndexSet indexSetWithIndexesInRange:CPMakeRange(0, 1)]]; + [self assert:[1, 2, 3] equals:[arrayController1 arrangedObjects] message:"normal selection"]; +} + - (void)observeValueForKeyPath:keyPath ofObject:anActivity change:change