From 86e3717222b1d951928af5655300b7d1c108b6d5 Mon Sep 17 00:00:00 2001 From: Randall Luecke Date: Tue, 19 Apr 2011 01:46:41 -0400 Subject: [PATCH 01/29] Fixes for splitview documentation --- AppKit/CPSplitView.j | 70 ++++++++++++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/AppKit/CPSplitView.j b/AppKit/CPSplitView.j index 16e58d422..f47f1e4b7 100644 --- a/AppKit/CPSplitView.j +++ b/AppKit/CPSplitView.j @@ -739,41 +739,59 @@ var CPSplitViewHorizontalImage = nil, Sets the delegate of the receiver. Possible delegate methods to implement are listed below. -
-        - (void)splitViewDidResizeSubviews:(CPSplitView)aSplitView;
-            Notifies the delegate when the subviews have resized.
+Notifies the delegate when the subviews have resized.
+@code
+- (void)splitViewDidResizeSubviews:(CPNotification)aNotification;
+@endcode
 
-        - (void)splitViewWillResizeSubviews:(CPSplitView)aSplitView;
-            Notifies the delegate when the subviews will be resized.
+Notifies the delegate when the subviews will be resized.
+@code
+- (void)splitViewWillResizeSubviews:(CPNotification)aNotification;
+@endcode
 
-        - (CGRect)splitView:(CPSplitView)aSplitView effectiveRect:(CGRect)aRect forDrawnRect:(CGRect)aDrawnRect ofDividerAtIndex:(int)aDividerIndex;
-            Lets the delegate specify a different rect for which the user can drag the splitView divider.
+Lets the delegate specify a different rect for which the user can drag the splitView divider.
+@code
+- (CGRect)splitView:(CPSplitView)aSplitView effectiveRect:(CGRect)aRect forDrawnRect:(CGRect)aDrawnRect ofDividerAtIndex:(int)aDividerIndex;
+@endcode
 
-        - (CGRect)splitView:(CPSplitView)aSplitView additionalEffectiveRectOfDividerAtIndex:(int)indexOfDivider;
-            Lets the delegate specify an additional rect for which the user can drag the splitview divider.
+Lets the delegate specify an additional rect for which the user can drag the splitview divider.
+@code
+- (CGRect)splitView:(CPSplitView)aSplitView additionalEffectiveRectOfDividerAtIndex:(int)indexOfDivider;
+@endcode
 
-        - (BOOL)splitView:(CPSplitView)aSplitView canCollapseSubview:(CPView)aSubview;
-            Notifies the delegate that the splitview is about to be collapsed. This usually happens when the user
-            Double clicks on the divider. Return YES if the subview can be collapsed, otherwise NO.
+Notifies the delegate that the splitview is about to be collapsed. This usually happens when the user
+Double clicks on the divider. Return YES if the subview can be collapsed, otherwise NO.
+@code
+- (BOOL)splitView:(CPSplitView)aSplitView canCollapseSubview:(CPView)aSubview;
+@endcode
 
-         - (BOOL)splitView:(CPSplitView)aSplitView shouldCollapseSubview:(CPView)aSubview forDoubleClickOnDividerAtIndex:(int)indexOfDivider;
-            Notifies the delegate that the subview at indexOfDivider is about to be collapsed. This usually happens when the user
-            Double clicks on the divider. Return YES if the subview should be collapsed, otherwise NO.
+Notifies the delegate that the subview at indexOfDivider is about to be collapsed. This usually happens when the user
+Double clicks on the divider. Return YES if the subview should be collapsed, otherwise NO.
+@code
+ - (BOOL)splitView:(CPSplitView)aSplitView shouldCollapseSubview:(CPView)aSubview forDoubleClickOnDividerAtIndex:(int)indexOfDivider;
+@endcode
 
-        - (float)splitView:(CPSplitView)aSpiltView constrainSplitPosition:(float)proposedPosition ofSubviewAt:(int)subviewIndex;
-            Allows the delegate to constrain the subview beings resized. This method is called continuously as the user resizes the divider.
-            For example if the subview needs to have a width which is a multiple of a certain number you could return that multiple with this method.
+Allows the delegate to constrain the subview beings resized. This method is called continuously as the user resizes the divider.
+For example if the subview needs to have a width which is a multiple of a certain number you could return that multiple with this method.
+@code
+- (float)splitView:(CPSplitView)aSpiltView constrainSplitPosition:(float)proposedPosition ofSubviewAt:(int)subviewIndex;
+@endcode
 
-        - (float)splitView:(CPSplitView)aSplitView constrainMinCoordinate:(float)proposedMin ofSubviewAt:(int)subviewIndex;
-            Allows the delegate to constrain the minimum position of a subview.
+Allows the delegate to constrain the minimum position of a subview.
+@code
+- (float)splitView:(CPSplitView)aSplitView constrainMinCoordinate:(float)proposedMin ofSubviewAt:(int)subviewIndex;
+@endcode
 
-        - (float)splitView:(CPSplitView)aSplitView constrainMaxCoordinate:(float)proposedMax ofSubviewAt:(int)subviewIndex;
-            Allows the delegate to constrain the maximum position of a subview.
+Allows the delegate to constrain the maximum position of a subview.
+@code
+- (float)splitView:(CPSplitView)aSplitView constrainMaxCoordinate:(float)proposedMax ofSubviewAt:(int)subviewIndex;
+@endcode
 
-        - (void)splitView:(CPSplitView)aSplitView resizeSubviewsWithOldSize:(CGSize)oldSize;
-            Allows the splitview to specify a custom resizing behavior. This is called when the splitview is resized.
-            The sum of the views and the sum of the dividers should be equal to the size of the splitview.
-    
+Allows the splitview to specify a custom resizing behavior. This is called when the splitview is resized. +The sum of the views and the sum of the dividers should be equal to the size of the splitview. +@code +- (void)splitView:(CPSplitView)aSplitView resizeSubviewsWithOldSize:(CGSize)oldSize; +@endcode @param delegate - The delegate of the splitview. */ From dc264231c825d46354f50a692cf5a8ffeef3bd05 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Wed, 20 Apr 2011 08:58:30 +0200 Subject: [PATCH 02/29] open menu bar menus directly --- AppKit/CPMenu/_CPMenuManager.j | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/AppKit/CPMenu/_CPMenuManager.j b/AppKit/CPMenu/_CPMenuManager.j index a95bc5fba..15af786fb 100644 --- a/AppKit/CPMenu/_CPMenuManager.j +++ b/AppKit/CPMenu/_CPMenuManager.j @@ -243,9 +243,15 @@ var STICKY_TIME_INTERVAL = 500, // Close the current menu item because we are going to select a new one after a short delay [self showMenu:nil fromMenu:activeMenu atPoint:CGPointMakeZero()]; - _showTimerID = setTimeout(function() { + + if (![activeMenuContainer isMenuBar]) + { + _showTimerID = setTimeout(function() { + [self showMenu:[activeItem submenu] fromMenu:[activeItem menu] atPoint:newMenuOrigin]; + }, 250); + } + else [self showMenu:[activeItem submenu] fromMenu:[activeItem menu] atPoint:newMenuOrigin]; - }, 250); } } From 8e3c26c3139737788e9e17a0587e61963cca7d5f Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Wed, 20 Apr 2011 10:38:22 +0200 Subject: [PATCH 03/29] implement CPMenu user interface validation --- AppKit/CPMenu/CPMenu.j | 24 ++++++++++++++++++++++-- AppKit/CPMenu/_CPMenuWindow.j | 6 ++++++ AppKit/CPMenuItem/CPMenuItem.j | 16 ++++++++++++++-- AppKit/Cib/CPCibControlConnector.j | 4 ---- 4 files changed, 42 insertions(+), 8 deletions(-) diff --git a/AppKit/CPMenu/CPMenu.j b/AppKit/CPMenu/CPMenu.j index 059c918b7..b5cf9a850 100644 --- a/AppKit/CPMenu/CPMenu.j +++ b/AppKit/CPMenu/CPMenu.j @@ -536,7 +536,6 @@ var _CPMenuBarVisible = NO, */ - (void)submenuAction:(id)aSender { - } /*! @@ -584,11 +583,30 @@ var _CPMenuBarVisible = NO, } /*! - Not implemented. + Enables or disables the receiver’s menu items. + If the target does not implement the menu item's action method the item is disabled. + If the target responsds to selector validateMenuItem: or validateUserInterfaceItem: (in that order) the return value is used. */ - (void)update { + if (![self autoenablesItems]) + return; + var items = [self itemArray]; + for (var i = 0; i < [items count]; i++) + { + var item = [items objectAtIndex:i], + validator = [CPApp targetForAction:[item action] to:[item target] from:item]; + + if (!validator || ![validator respondsToSelector:[item action]]) + [item _setEnabled:NO]; + else if ([validator respondsToSelector:@selector(validateMenuItem:)]) + [item _setEnabled:[validator validateMenuItem:item]]; + else if ([validator respondsToSelector:@selector(validateUserInterfaceItem:)]) + [item _setEnabled:[validator validateUserInterfaceItem:item]]; + } + + [[_menuWindow _menuView] tile]; } // Managing the Title @@ -1100,6 +1118,8 @@ var CPMenuTitleKey = @"CPMenuTitleKey", _showsStateColumn = ![aCoder containsValueForKey:CPMenuShowsStateColumnKey] || [aCoder decodeBoolForKey:CPMenuShowsStateColumnKey]; + _autoenablesItems = YES; + [self setMinimumWidth:0]; } diff --git a/AppKit/CPMenu/_CPMenuWindow.j b/AppKit/CPMenu/_CPMenuWindow.j index cb52e9822..053d493d8 100644 --- a/AppKit/CPMenu/_CPMenuWindow.j +++ b/AppKit/CPMenu/_CPMenuWindow.j @@ -208,8 +208,14 @@ var STICKY_TIME_INTERVAL = 500, return [_menuView menu]; } +- (_CPMenuView)_menuView +{ + return _menuView; +} + - (void)orderFront:(id)aSender { + [[self menu] update]; [self setFrame:_unconstrainedFrame]; [super orderFront:aSender]; diff --git a/AppKit/CPMenuItem/CPMenuItem.j b/AppKit/CPMenuItem/CPMenuItem.j index e0dcd305e..fd8f117db 100644 --- a/AppKit/CPMenuItem/CPMenuItem.j +++ b/AppKit/CPMenuItem/CPMenuItem.j @@ -133,10 +133,17 @@ if ([_menu autoenablesItems]) return; - _isEnabled = isEnabled; + [self _setEnabled:isEnabled]; +} + +- (void)_setEnabled:(BOOL)isEnabled +{ + if (_isEnabled === isEnabled) + return; + + _isEnabled = !!isEnabled; [_menuItemView setDirty]; - [_menu itemChanged:self]; } @@ -851,6 +858,11 @@ CPControlKeyMask return ![self submenu] && [self menu] === [CPApp mainMenu]; } +- (CPString)description +{ + return [super description] + @" target: " + [self target] + @" action: " + CPStringFromSelector([self action]); +} + @end var CPMenuItemIsSeparatorKey = @"CPMenuItemIsSeparatorKey", diff --git a/AppKit/Cib/CPCibControlConnector.j b/AppKit/Cib/CPCibControlConnector.j index 68934e572..ddb64d8c3 100644 --- a/AppKit/Cib/CPCibControlConnector.j +++ b/AppKit/Cib/CPCibControlConnector.j @@ -47,12 +47,8 @@ // If the destination doesn't respond to this selector, warn but don't die. if (_destination && ![_destination respondsToSelector:selector]) - { CPLog.warn(@"Could not connect the action " + selector + @" to target of class " + [_destination className]); - return; - } - // Not being able to set the action is a fatal error. if ([_source respondsToSelector:@selector(setAction:)]) objj_msgSend(_source, @selector(setAction:), selector); From ba3a2a9f8963b5358ab1a954f2d6a65cd0e4d277 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Wed, 20 Apr 2011 11:20:07 +0200 Subject: [PATCH 04/29] add unit test for CPMenu user interface validation --- .../CPMenuValidatedUserInterfaceItemTest.j | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) create mode 100644 Tests/AppKit/CPMenuValidatedUserInterfaceItemTest.j diff --git a/Tests/AppKit/CPMenuValidatedUserInterfaceItemTest.j b/Tests/AppKit/CPMenuValidatedUserInterfaceItemTest.j new file mode 100644 index 000000000..e4fbb2115 --- /dev/null +++ b/Tests/AppKit/CPMenuValidatedUserInterfaceItemTest.j @@ -0,0 +1,52 @@ +@implementation CPMenuValidatedUserInterfaceItemTest : OJTestCase +{ + CPMenu _menu @accessors(property=menu); +} + +- (void)setUp +{ + [[CPApplication sharedApplication] setDelegate:[[MenuTarget alloc] init]]; + + _menu = [[CPMenu alloc] init]; + + [_menu addItem:[[CPMenuItem alloc] initWithTitle:@"implemented" action:@selector(implementedAction:) keyEquivalent:nil]]; + [_menu addItem:[[CPMenuItem alloc] initWithTitle:@"disabled" action:@selector(disabledAction:) keyEquivalent:nil]]; + [_menu addItem:[[CPMenuItem alloc] initWithTitle:@"unimplemented" action:@selector(unimplementedAction:) keyEquivalent:nil]]; +} + +- (void)testAutoenable +{ + // Update is what actually performs the autoenabling so we need to call it manually. + // It's normally called automatically just before a menu becomes visible. + [[self menu] update]; + + [self assertTrue:[[[self menu] itemWithTitle:@"implemented"] isEnabled] message:@"The implemented action should be enabled"]; + [self assertFalse:[[[self menu] itemWithTitle:@"disabled"] isEnabled] message:@"The disbabled action should be disabled"]; + [self assertFalse:[[[self menu] itemWithTitle:@"unimplemented"] isEnabled] message:@"The unimplemented action should be disabled"]; +} + +@end + +@implementation MenuTarget : CPObject +{ +} + +- (BOOL)validateMenuItem:(CPMenuItem)theMenuItem +{ + if ([theMenuItem action] === @selector(disabledAction:)) + return NO; + + return YES; +} + +- (@action)implementedAction:(id)theSender +{ + +} + +- (@action)disabledAction:(id)theSender +{ + +} + +@end \ No newline at end of file From f8674b6d87e8a3397835eb7b64bc7aa5df5c1968 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Wed, 20 Apr 2011 14:40:47 +0200 Subject: [PATCH 05/29] add collectionView:menuForItemAtIndex delegate --- AppKit/CPCollectionView.j | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/AppKit/CPCollectionView.j b/AppKit/CPCollectionView.j index 61e68082d..a48c11c39 100644 --- a/AppKit/CPCollectionView.j +++ b/AppKit/CPCollectionView.j @@ -589,9 +589,7 @@ _mouseDownEvent = anEvent; var location = [self convertPoint:[anEvent locationInWindow] fromView:nil], - row = FLOOR(location.y / (_itemSize.height + _verticalMargin)), - column = FLOOR(location.x / (_itemSize.width + _horizontalMargin)), - index = row * _numberOfColumns + column; + index = [self _indexAtPoint:location]; if (index >= 0 && index < _items.length) { @@ -727,6 +725,28 @@ return _delegate; } +/*! + @ignore +*/ +- (CPMenu)menuForEvent:(CPEvent)theEvent +{ + if (![[self delegate] respondsToSelector:@selector(collectionView:menuForItemAtIndex:)]) + return [super menuForEvent:theEvent]; + + var location = [self convertPoint:[theEvent locationInWindow] fromView:nil], + index = [self _indexAtPoint:location]; + + return [_delegate collectionView:self menuForItemAtIndex:index]; +} + +- (int)_indexAtPoint:(CGPoint)thePoint +{ + var row = FLOOR(thePoint.y / (_itemSize.height + _verticalMargin)), + column = FLOOR(thePoint.x / (_itemSize.width + _horizontalMargin)); + + return row * _numberOfColumns + column; +} + - (CPCollectionViewItem)itemAtIndex:(unsigned)anIndex { return [_items objectAtIndex:anIndex]; From 71d8582f94dfff3b82a9d8600f2e54284ef7c56c Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Wed, 20 Apr 2011 16:21:28 +0200 Subject: [PATCH 06/29] make unhandled keyboard events travel past the window --- AppKit/CPWindow/CPWindow.j | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/AppKit/CPWindow/CPWindow.j b/AppKit/CPWindow/CPWindow.j index dcdc9e727..8a2f6e43f 100644 --- a/AppKit/CPWindow/CPWindow.j +++ b/AppKit/CPWindow/CPWindow.j @@ -193,24 +193,24 @@ var SHADOW_MARGIN_LEFT = 20.0, _CPWindowShadowColor = nil; var CPWindowSaveImage = nil, - CPWindowSavingImage = nil; + CPWindowSavingImage = nil, -var CPWindowResizeTime = 0.2; + CPWindowResizeTime = 0.2; /* Keys for which action messages will be sent by default when unhandled, e.g. complete:. */ var CPWindowActionMessageKeys = [ - CPLeftArrowFunctionKey, - CPRightArrowFunctionKey, - CPUpArrowFunctionKey, - CPDownArrowFunctionKey, - CPPageUpFunctionKey, - CPPageDownFunctionKey, - CPHomeFunctionKey, - CPEndFunctionKey, - CPEscapeFunctionKey -]; + CPLeftArrowFunctionKey, + CPRightArrowFunctionKey, + CPUpArrowFunctionKey, + CPDownArrowFunctionKey, + CPPageUpFunctionKey, + CPPageDownFunctionKey, + CPHomeFunctionKey, + CPEndFunctionKey, + CPEscapeFunctionKey + ]; /*! @ingroup appkit @@ -2377,7 +2377,9 @@ CPTexturedBackgroundWindowMask // an event going of the responder chain is passed to the input system as a last resort. // However, the only methods I could get Cocoa to call automatically are // moveUp: moveDown: moveLeft: moveRight: pageUp: pageDown: and complete: - [self _processKeyboardUIKey:anEvent]; + // Unhandled events just travel further up the responder chain _past_ the window. + if (![self _processKeyboardUIKey:anEvent]) + [super keyDown:anEvent]; } /* @@ -2412,6 +2414,8 @@ CPTexturedBackgroundWindowMask // The difference is that doCommandBySelector: will also send the action to the window and application delegates. [[self firstResponder] doCommandBySelector:@selector(complete:)]; } + + return NO; } - (void)_dirtyKeyViewLoop From 561d22168a0fa0ea65b7ef97c6176195cb8b0272 Mon Sep 17 00:00:00 2001 From: Randall Luecke Date: Wed, 20 Apr 2011 12:19:29 -0400 Subject: [PATCH 07/29] Stop references function arguments with arguments["name"] in CG. --- AppKit/CoreGraphics/CGContext.j | 2 +- AppKit/CoreGraphics/CGContextCanvas.j | 6 +++--- AppKit/CoreGraphics/CGGradient.j | 6 +++--- AppKit/CoreGraphics/CGPath.j | 4 ++-- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/AppKit/CoreGraphics/CGContext.j b/AppKit/CoreGraphics/CGContext.j index 34eb0c603..88de992c1 100644 --- a/AppKit/CoreGraphics/CGContext.j +++ b/AppKit/CoreGraphics/CGContext.j @@ -606,7 +606,7 @@ function CGContextStrokeLineSegments(aContext, points, count) { var i = 0; - if (arguments["count"] == NULL) + if (count === NULL) var count = points.length; CGContextBeginPath(aContext); diff --git a/AppKit/CoreGraphics/CGContextCanvas.j b/AppKit/CoreGraphics/CGContextCanvas.j index 5a12aae01..288ab147f 100644 --- a/AppKit/CoreGraphics/CGContextCanvas.j +++ b/AppKit/CoreGraphics/CGContextCanvas.j @@ -144,7 +144,7 @@ function CGContextAddRects(aContext, rects, count) { var i = 0; - if (arguments["count"] == NULL) + if (count === NULL) var count = rects.length; for (; i < count; ++i) @@ -194,7 +194,7 @@ function CGContextFillRects(aContext, rects, count) { var i = 0; - if (arguments["count"] == NULL) + if (count === NULL) var count = rects.length; for (; i < count; ++i) @@ -225,7 +225,7 @@ function CGContextClipToRect(aContext, aRect) function CGContextClipToRects(aContext, rects, count) { - if (arguments["count"] == NULL) + if (count === NULL) var count = rects.length; _CGContextBeginPathCanvas(aContext); diff --git a/AppKit/CoreGraphics/CGGradient.j b/AppKit/CoreGraphics/CGGradient.j index 29b5d149f..e8396aa3d 100644 --- a/AppKit/CoreGraphics/CGGradient.j +++ b/AppKit/CoreGraphics/CGGradient.j @@ -29,10 +29,10 @@ kCGGradientDrawsAfterEndLocation = 1 << 1; function CGGradientCreateWithColorComponents(aColorSpace, components, locations, count) { - if (arguments["locations"] == NULL) + if (locations === NULL) var locations = [0.0, 1.0]; - if (arguments["count"] == NULL) + if (count === NULL) var count = locations.length; var colors = []; @@ -58,4 +58,4 @@ function CGGradientRelease() function CGGradientRetain(aGradient) { return aGradient; -} \ No newline at end of file +} diff --git a/AppKit/CoreGraphics/CGPath.j b/AppKit/CoreGraphics/CGPath.j index 5dde217cb..bc0fe4c2f 100644 --- a/AppKit/CoreGraphics/CGPath.j +++ b/AppKit/CoreGraphics/CGPath.j @@ -141,7 +141,7 @@ function CGPathAddLines(aPath, aTransform, points, count) { var i = 1; - if (arguments["count"] == NULL) + if (count === NULL) var count = points.length; if (!aPath || count < 2) @@ -224,7 +224,7 @@ function CGPathAddRects(aPath, aTransform, rects, count) { var i = 0; - if (arguments["count"] == NULL) + if (count === NULL) var count = rects.length; for (; i < count; ++i) From b249a1db13760823a513f4a3dbb18204a7409984 Mon Sep 17 00:00:00 2001 From: Gerrit Riessen Date: Tue, 19 Apr 2011 12:10:17 +0200 Subject: [PATCH 08/29] test demostrating limitations of CGGradientCreateWithColorComponents --- Tests/AppKit/CGGradientTest.j | 66 +++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 Tests/AppKit/CGGradientTest.j diff --git a/Tests/AppKit/CGGradientTest.j b/Tests/AppKit/CGGradientTest.j new file mode 100644 index 000000000..187dd713d --- /dev/null +++ b/Tests/AppKit/CGGradientTest.j @@ -0,0 +1,66 @@ +@import + +@implementation CGGradientTest : OJTestCase + +/* + Using default values for location and count. +*/ +- (void)testCreateWithColorComponentsDefault +{ + var components = [ 0.2,0.2,0.4,0.4, + 0.6,0.6,0.8,0.8 ]; + + var tmp = CGGradientCreateWithColorComponents(CGColorSpaceCreateDeviceRGB(), + components); + [self assert:2 equals:tmp.colors.length]; + [self assert:2 equals:tmp.locations.length]; + + [self assert:0 equals:tmp.locations[0]]; + [self assert:1.0 equals:tmp.locations[1]]; + + [self assert:components.slice(4,8) equals:tmp.colors[1].components]; + [self assert:components.slice(0,4) equals:tmp.colors[0].components]; +} + +/* + Using location and components to specify two color stops that aren't + evenly spread out. +*/ +- (void)testCreateWithColorComponentsUsingTwoColorStops +{ + var components = [ 0.2,0.2,0.4,0.4, + 0.6,0.6,0.8,0.8 ]; + var locations = [0.3,0.6]; + + var tmp = CGGradientCreateWithColorComponents(CGColorSpaceCreateDeviceRGB(), + components,locations); + [self assert:2 equals:tmp.colors.length]; + [self assert:2 equals:tmp.locations.length]; + + [self assert:0.3 equals:tmp.locations[0]]; + [self assert:0.6 equals:tmp.locations[1]]; + + [self assert:components.slice(4,8) equals:tmp.colors[1].components]; + [self assert:components.slice(0,4) equals:tmp.colors[0].components]; +} + +/* + Define three color components and ensure that the Gradient defined contains 3 color stops. +*/ +- (void)testCanHaveMoreThanTwoColorStops +{ + var components = [ 1,2,3,4, 5,6,7,8, 9,10,11,12 ]; + var locations = [1,2,3]; + + var tmp = CGGradientCreateWithColorComponents(CGColorSpaceCreateDeviceRGB(), + components, locations, 3); + [self assert:3 equals:tmp.colors.length]; + [self assert:3 equals:tmp.locations.length]; + + [self assert:1 equals:tmp.locations[0]]; + [self assert:2 equals:tmp.locations[1]]; + [self assert:3 equals:tmp.locations[2]]; +} + + +@end From dc5d8f5fd312e1eb6f0b4daa15bc13fb95f993b3 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Wed, 20 Apr 2011 21:00:52 +0200 Subject: [PATCH 09/29] never disable menu items with submenus --- AppKit/CPMenu/CPMenu.j | 8 +++- .../CPMenuValidatedUserInterfaceItemTest.j | 44 ++++++++++++++++--- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/AppKit/CPMenu/CPMenu.j b/AppKit/CPMenu/CPMenu.j index b5cf9a850..db5aa8e61 100644 --- a/AppKit/CPMenu/CPMenu.j +++ b/AppKit/CPMenu/CPMenu.j @@ -595,8 +595,12 @@ var _CPMenuBarVisible = NO, var items = [self itemArray]; for (var i = 0; i < [items count]; i++) { - var item = [items objectAtIndex:i], - validator = [CPApp targetForAction:[item action] to:[item target] from:item]; + var item = [items objectAtIndex:i]; + + if ([item hasSubmenu]) + continue; + + var validator = [CPApp targetForAction:[item action] to:[item target] from:item]; if (!validator || ![validator respondsToSelector:[item action]]) [item _setEnabled:NO]; diff --git a/Tests/AppKit/CPMenuValidatedUserInterfaceItemTest.j b/Tests/AppKit/CPMenuValidatedUserInterfaceItemTest.j index e4fbb2115..4322239fa 100644 --- a/Tests/AppKit/CPMenuValidatedUserInterfaceItemTest.j +++ b/Tests/AppKit/CPMenuValidatedUserInterfaceItemTest.j @@ -1,38 +1,72 @@ +var CPMenuValidatedUserInterfaceItemTestValidatedItems = []; + @implementation CPMenuValidatedUserInterfaceItemTest : OJTestCase { CPMenu _menu @accessors(property=menu); + + MenuTarget _menuTarget @accessors(property=menuTarget); } - (void)setUp { - [[CPApplication sharedApplication] setDelegate:[[MenuTarget alloc] init]]; + _menuTarget = [[MenuTarget alloc] init]; + [[CPApplication sharedApplication] setDelegate:_menuTarget]; _menu = [[CPMenu alloc] init]; [_menu addItem:[[CPMenuItem alloc] initWithTitle:@"implemented" action:@selector(implementedAction:) keyEquivalent:nil]]; [_menu addItem:[[CPMenuItem alloc] initWithTitle:@"disabled" action:@selector(disabledAction:) keyEquivalent:nil]]; [_menu addItem:[[CPMenuItem alloc] initWithTitle:@"unimplemented" action:@selector(unimplementedAction:) keyEquivalent:nil]]; + + var parentItem = [[CPMenuItem alloc] initWithTitle:@"parent" action:nil keyEquivalent:nil]; + [_menu addItem:parentItem]; + + [parentItem setSubmenu:[[CPMenu alloc] init]]; + [[parentItem submenu] addItem:[[CPMenuItem alloc] initWithTitle:@"Submenu 1" action:nil keyEquivalent:nil]]; + + CPMenuValidatedUserInterfaceItemTestValidatedItems = []; + + // Update is what performs the autoenabling so we need to call it manually. + // It's normally called automatically just before a menu becomes visible. + [_menu update]; } - (void)testAutoenable { - // Update is what actually performs the autoenabling so we need to call it manually. - // It's normally called automatically just before a menu becomes visible. - [[self menu] update]; - [self assertTrue:[[[self menu] itemWithTitle:@"implemented"] isEnabled] message:@"The implemented action should be enabled"]; [self assertFalse:[[[self menu] itemWithTitle:@"disabled"] isEnabled] message:@"The disbabled action should be disabled"]; [self assertFalse:[[[self menu] itemWithTitle:@"unimplemented"] isEnabled] message:@"The unimplemented action should be disabled"]; } +- (void)testThatParentMenusAreNotValidated +{ + var parentItem = [[self menu] itemWithTitle:@"parent"]; + + [self assertTrue:[parentItem isEnabled] message:@"Parent items should never be disabled"]; + [self assertFalse:[[[self menuTarget] validatedItems] containsObject:parent] message:@"Parent items should never be validated"]; +} + @end @implementation MenuTarget : CPObject { + CPArray _validatedItems @accessors(property=validatedItems, readonly); +} + +- (id)init +{ + if (self = [super init]) + { + _validatedItems = []; + } + + return self; } - (BOOL)validateMenuItem:(CPMenuItem)theMenuItem { + [_validatedItems addObject:theMenuItem]; + if ([theMenuItem action] === @selector(disabledAction:)) return NO; From 27211db0ccc9c3ff7b32a5cfd310288418656fc2 Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Thu, 21 Apr 2011 18:59:30 -0400 Subject: [PATCH 10/29] Fixed: JS objects 'randomly' failed to unarchive. The problem was that if the same JS object was decoded twice as part of a more complex object, the first decoded version was cached in its unwrapped state and not properly unwrapped the second time. This caused very hard to debug theming errors where e.g. a CPTextField's custom inset would decode fine, but a subclass of a CPTextField otherwise exactly the same would fail to decode the inset. --- Foundation/CPKeyedUnarchiver.j | 135 +++++++++++++------------ Tests/Foundation/CPKeyedArchiverTest.j | 44 ++++++++ 2 files changed, 113 insertions(+), 66 deletions(-) create mode 100644 Tests/Foundation/CPKeyedArchiverTest.j diff --git a/Foundation/CPKeyedUnarchiver.j b/Foundation/CPKeyedUnarchiver.j index 666b4f1e3..b9baf93c5 100644 --- a/Foundation/CPKeyedUnarchiver.j +++ b/Foundation/CPKeyedUnarchiver.j @@ -441,71 +441,48 @@ var _CPKeyedUnarchiverDecodeObjectAtIndex = function(self, anIndex) var object = self._objects[anIndex]; if (object) + { if (object === self._objects[0]) return nil; - else - return object; - - var object, - plistObject = self._plistObjects[anIndex], - plistObjectClass = plistObject.isa; - - if (plistObjectClass === CPDictionaryClass || plistObjectClass === CPMutableDictionaryClass) + // Don't return immediately here. The _CPKeyedArchiverValueClass unwrapper code + // hasn't executed yet. + } + else { - var plistClass = self._plistObjects[plistObject.valueForKey(_CPKeyedArchiverClassKey).valueForKey(_CPKeyedArchiverUIDKey)], - className = plistClass.valueForKey(_CPKeyedArchiverClassNameKey), - classes = plistClass.valueForKey(_CPKeyedArchiverClassesKey), - theClass = [self classForClassName:className]; + var plistObject = self._plistObjects[anIndex], + plistObjectClass = plistObject.isa; - if (!theClass) - theClass = CPClassFromString(className); - - if (!theClass && (self._delegateSelectors & CPKeyedUnarchiverDelegate_unarchiver_cannotDecodeObjectOfClassName_originalClasses_)) - theClass = [_delegate unarchiver:self cannotDecodeObjectOfClassName:className originalClasses:classes]; - - if (!theClass) - [CPException raise:CPInvalidUnarchiveOperationException reason:@"-[CPKeyedUnarchiver decodeObjectForKey:]: cannot decode object of class (" + className + @")"]; - - var savedPlistObject = self._plistObject; - - self._plistObject = plistObject; - - // Should we only call this on _CPCibClassSwapper? (currently the only class that makes use of this). - object = [theClass allocWithCoder:self]; - - // It is important to do this before calling initWithCoder so that decoding can be self referential (something = self). - self._objects[anIndex] = object; - - var processedObject = [object initWithCoder:self]; - - self._plistObject = savedPlistObject; - - if (processedObject !== object) + if (plistObjectClass === CPDictionaryClass || plistObjectClass === CPMutableDictionaryClass) { - if (self._delegateSelectors & _CPKeyedUnarchiverWillReplaceObjectWithObjectSelector) - [self._delegate unarchiver:self willReplaceObject:object withObject:processedObject]; + var plistClass = self._plistObjects[plistObject.valueForKey(_CPKeyedArchiverClassKey).valueForKey(_CPKeyedArchiverUIDKey)], + className = plistClass.valueForKey(_CPKeyedArchiverClassNameKey), + classes = plistClass.valueForKey(_CPKeyedArchiverClassesKey), + theClass = [self classForClassName:className]; - object = processedObject; - self._objects[anIndex] = processedObject; - } + if (!theClass) + theClass = CPClassFromString(className); - processedObject = [object awakeAfterUsingCoder:self]; + if (!theClass && (self._delegateSelectors & CPKeyedUnarchiverDelegate_unarchiver_cannotDecodeObjectOfClassName_originalClasses_)) + theClass = [_delegate unarchiver:self cannotDecodeObjectOfClassName:className originalClasses:classes]; - if (processedObject !== object) - { - if (self._delegateSelectors & _CPKeyedUnarchiverWillReplaceObjectWithObjectSelector) - [self._delegate unarchiver:self willReplaceObject:object withObject:processedObject]; + if (!theClass) + [CPException raise:CPInvalidUnarchiveOperationException reason:@"-[CPKeyedUnarchiver decodeObjectForKey:]: cannot decode object of class (" + className + @")"]; - object = processedObject; - self._objects[anIndex] = processedObject; - } + var savedPlistObject = self._plistObject; - if (self._delegate) - { - if (self._delegateSelectors & _CPKeyedUnarchiverDidDecodeObjectSelector) - processedObject = [self._delegate unarchiver:self didDecodeObject:object]; + self._plistObject = plistObject; - if (processedObject && processedObject != object) + // Should we only call this on _CPCibClassSwapper? (currently the only class that makes use of this). + object = [theClass allocWithCoder:self]; + + // It is important to do this before calling initWithCoder so that decoding can be self referential (something = self). + self._objects[anIndex] = object; + + var processedObject = [object initWithCoder:self]; + + self._plistObject = savedPlistObject; + + if (processedObject !== object) { if (self._delegateSelectors & _CPKeyedUnarchiverWillReplaceObjectWithObjectSelector) [self._delegate unarchiver:self willReplaceObject:object withObject:processedObject]; @@ -513,22 +490,48 @@ var _CPKeyedUnarchiverDecodeObjectAtIndex = function(self, anIndex) object = processedObject; self._objects[anIndex] = processedObject; } - } - } - else - { - self._objects[anIndex] = object = plistObject; - if ([object class] === CPStringClass) - { - if (object === _CPKeyedArchiverNullString) + processedObject = [object awakeAfterUsingCoder:self]; + + if (processedObject !== object) { - self._objects[anIndex] = self._objects[0]; + if (self._delegateSelectors & _CPKeyedUnarchiverWillReplaceObjectWithObjectSelector) + [self._delegate unarchiver:self willReplaceObject:object withObject:processedObject]; - return nil; + object = processedObject; + self._objects[anIndex] = processedObject; + } + + if (self._delegate) + { + if (self._delegateSelectors & _CPKeyedUnarchiverDidDecodeObjectSelector) + processedObject = [self._delegate unarchiver:self didDecodeObject:object]; + + if (processedObject && processedObject != object) + { + if (self._delegateSelectors & _CPKeyedUnarchiverWillReplaceObjectWithObjectSelector) + [self._delegate unarchiver:self willReplaceObject:object withObject:processedObject]; + + object = processedObject; + self._objects[anIndex] = processedObject; + } + } + } + else + { + self._objects[anIndex] = object = plistObject; + + if ([object class] === CPStringClass) + { + if (object === _CPKeyedArchiverNullString) + { + self._objects[anIndex] = self._objects[0]; + + return nil; + } + else + self._objects[anIndex] = object = plistObject; } - else - self._objects[anIndex] = object = plistObject; } } diff --git a/Tests/Foundation/CPKeyedArchiverTest.j b/Tests/Foundation/CPKeyedArchiverTest.j new file mode 100644 index 000000000..ea61e832e --- /dev/null +++ b/Tests/Foundation/CPKeyedArchiverTest.j @@ -0,0 +1,44 @@ +@import +@import + +@implementation CPKeyedArchiverTest : OJTestCase +{ + +} + +- (void)testJavaScriptObject +{ + var original = [Archivable new]; + + [original setAJsObject:{ 'top': 5 }]; + + var decoded = [CPKeyedUnarchiver unarchiveObjectWithData:[CPKeyedArchiver archivedDataWithRootObject:original]] + + [self assert:5 equals:[decoded aJsObject].top message:"JS object encoded and decoded right"]; +} + +@end + +@implementation Archivable : CPObject +{ + JSObject aJsObject @accessors; +} + +- (id)initWithCoder:(CPCoder)aCoder +{ + if (self = [super init]) + { + // Note we decode this twice to expose a bug where the cached decoded + // values did not properly unwrap JS objects. + aJsObject = [aCoder decodeObjectForKey:@"aJsObject"]; + aJsObject = [aCoder decodeObjectForKey:@"aJsObject"]; + } + return self; +} + +- (void)encodeWithCoder:(CPCoder)aCoder +{ + [aCoder encodeObject:aJsObject forKey:@"aJsObject"]; +} + +@end From dcf1d324a56304f2645052a5a8c9e9e9f12880b5 Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Thu, 21 Apr 2011 19:03:10 -0400 Subject: [PATCH 11/29] Additional test for the previous bug. --- Tests/AppKit/CPTextFieldTest.j | 41 ++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 Tests/AppKit/CPTextFieldTest.j diff --git a/Tests/AppKit/CPTextFieldTest.j b/Tests/AppKit/CPTextFieldTest.j new file mode 100644 index 000000000..65fba1642 --- /dev/null +++ b/Tests/AppKit/CPTextFieldTest.j @@ -0,0 +1,41 @@ +@import + +[CPApplication sharedApplication] + +@implementation CPTextFieldTest : OJTestCase +{ +} + +/*! + Detect regressions with JS object theme attribute encoding in subclasses. + + This test is not actually CPTextField specific at all. This just happens + to be the first place the keyed archiving bug was discovered, and there is + an additional, more specific, test in CPKeyedArchiverTest. +*/ +- (void)testArchiveThemeAttributes +{ + var view = [[CPTextFieldSubclass alloc] initWithFrame:CGRectMakeZero()], + decoded = [CPKeyedUnarchiver unarchiveObjectWithData:[CPKeyedArchiver archivedDataWithRootObject:view]]; + + [self assert:[view valueForThemeAttribute:@"content-inset"].top equals:2 message:@"content-inset should initialise correctly"]; + [self assert:[view valueForThemeAttribute:@"content-inset"].top equals:[decoded valueForThemeAttribute:@"content-inset"].top message:@"content-inset should unarchive correctly"]; +} + +@end + +@implementation CPTextFieldSubclass : CPTextField +{ + +} + +- (id)initWithFrame:aFrame +{ + if (self = [super initWithFrame:aFrame]) + { + [self setValue:CGInsetMake(2.0, 2.0, 2.0, 2.0) forThemeAttribute:"content-inset"]; + } + return self; +} + +@end \ No newline at end of file From 5315095182ff979c1642d00c8eb97f5c30003f90 Mon Sep 17 00:00:00 2001 From: Randall Luecke Date: Fri, 22 Apr 2011 12:53:49 -0400 Subject: [PATCH 12/29] When dragging horizontally we need to update the location of the header view too --- AppKit/CPDragServer.j | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/AppKit/CPDragServer.j b/AppKit/CPDragServer.j index 9d3344b09..bc67ee490 100644 --- a/AppKit/CPDragServer.j +++ b/AppKit/CPDragServer.j @@ -270,10 +270,10 @@ var CPDraggingSource_draggedImage_movedTo_ = 1 << 0, { if ([scrollView hasVerticalScroller]) { - if (eventLocation.y < CGRectGetMinY(insetBounds)) - deltaY = CGRectGetMinY(insetBounds) - eventLocation.y; - else if (eventLocation.y > CGRectGetMaxY(insetBounds)) - deltaY = CGRectGetMaxY(insetBounds) - eventLocation.y; + if (eventLocation.y < _CGRectGetMinY(insetBounds)) + deltaY = _CGRectGetMinY(insetBounds) - eventLocation.y; + else if (eventLocation.y > _CGRectGetMaxY(insetBounds)) + deltaY = _CGRectGetMaxY(insetBounds) - eventLocation.y; if (deltaY < -insetBounds.size.height) deltaY = -insetBounds.size.height; if (deltaY > insetBounds.size.height) @@ -282,17 +282,21 @@ var CPDraggingSource_draggedImage_movedTo_ = 1 << 0, if ([scrollView hasHorizontalScroller]) { - if (eventLocation.x < CGRectGetMinX(insetBounds)) - deltaX = CGRectGetMinX(insetBounds) - eventLocation.x; - else if (eventLocation.x > CGRectGetMaxX(insetBounds)) - deltaX = CGRectGetMaxX(insetBounds) - eventLocation.x; + if (eventLocation.x < _CGRectGetMinX(insetBounds)) + deltaX = _CGRectGetMinX(insetBounds) - eventLocation.x; + else if (eventLocation.x > _CGRectGetMaxX(insetBounds)) + deltaX = _CGRectGetMaxX(insetBounds) - eventLocation.x; if (deltaX < -insetBounds.size.width) deltaX = -insetBounds.size.width; if (deltaX > insetBounds.size.width) deltaX = insetBounds.size.width; } - [contentView scrollToPoint:CGPointMake(bounds.origin.x - deltaX, bounds.origin.y - deltaY)]; + var scrollPoint = _CGPointMake(bounds.origin.x - deltaX, bounds.origin.y - deltaY); + + [contentView scrollToPoint:scrollPoint]; + [[scrollView _headerView] scrollPoint:scrollPoint]; + } } } From 05b0abacd4d786cea65d253a7716db87338194f0 Mon Sep 17 00:00:00 2001 From: Randall Luecke Date: Fri, 22 Apr 2011 12:53:49 -0400 Subject: [PATCH 13/29] When dragging horizontally we need to update the location of the header view too --- AppKit/CPDragServer.j | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/AppKit/CPDragServer.j b/AppKit/CPDragServer.j index 9d3344b09..bc67ee490 100644 --- a/AppKit/CPDragServer.j +++ b/AppKit/CPDragServer.j @@ -270,10 +270,10 @@ var CPDraggingSource_draggedImage_movedTo_ = 1 << 0, { if ([scrollView hasVerticalScroller]) { - if (eventLocation.y < CGRectGetMinY(insetBounds)) - deltaY = CGRectGetMinY(insetBounds) - eventLocation.y; - else if (eventLocation.y > CGRectGetMaxY(insetBounds)) - deltaY = CGRectGetMaxY(insetBounds) - eventLocation.y; + if (eventLocation.y < _CGRectGetMinY(insetBounds)) + deltaY = _CGRectGetMinY(insetBounds) - eventLocation.y; + else if (eventLocation.y > _CGRectGetMaxY(insetBounds)) + deltaY = _CGRectGetMaxY(insetBounds) - eventLocation.y; if (deltaY < -insetBounds.size.height) deltaY = -insetBounds.size.height; if (deltaY > insetBounds.size.height) @@ -282,17 +282,21 @@ var CPDraggingSource_draggedImage_movedTo_ = 1 << 0, if ([scrollView hasHorizontalScroller]) { - if (eventLocation.x < CGRectGetMinX(insetBounds)) - deltaX = CGRectGetMinX(insetBounds) - eventLocation.x; - else if (eventLocation.x > CGRectGetMaxX(insetBounds)) - deltaX = CGRectGetMaxX(insetBounds) - eventLocation.x; + if (eventLocation.x < _CGRectGetMinX(insetBounds)) + deltaX = _CGRectGetMinX(insetBounds) - eventLocation.x; + else if (eventLocation.x > _CGRectGetMaxX(insetBounds)) + deltaX = _CGRectGetMaxX(insetBounds) - eventLocation.x; if (deltaX < -insetBounds.size.width) deltaX = -insetBounds.size.width; if (deltaX > insetBounds.size.width) deltaX = insetBounds.size.width; } - [contentView scrollToPoint:CGPointMake(bounds.origin.x - deltaX, bounds.origin.y - deltaY)]; + var scrollPoint = _CGPointMake(bounds.origin.x - deltaX, bounds.origin.y - deltaY); + + [contentView scrollToPoint:scrollPoint]; + [[scrollView _headerView] scrollPoint:scrollPoint]; + } } } From 7ee414d0b89ae9c3f12c5fd8faac066f0285254d Mon Sep 17 00:00:00 2001 From: Gerrit Riessen Date: Fri, 22 Apr 2011 19:06:41 +0200 Subject: [PATCH 14/29] remove an assumption of two colors in the gradient creation stuff --- AppKit/CoreGraphics/CGGradient.j | 11 ++++++++--- Tests/AppKit/CGGradientTest.j | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/AppKit/CoreGraphics/CGGradient.j b/AppKit/CoreGraphics/CGGradient.j index e8396aa3d..24d0c3573 100644 --- a/AppKit/CoreGraphics/CGGradient.j +++ b/AppKit/CoreGraphics/CGGradient.j @@ -29,10 +29,15 @@ kCGGradientDrawsAfterEndLocation = 1 << 1; function CGGradientCreateWithColorComponents(aColorSpace, components, locations, count) { - if (locations === NULL) - var locations = [0.0, 1.0]; + if ( locations === undefined || locations === NULL ) + { + var num_of_colors = components.length / 4, + locations = []; + for ( var idx = 0; idx < num_of_colors; idx++ ) + locations.push( idx / (num_of_colors - 1) ); + } - if (count === NULL) + if ( count === undefined || count === NULL) var count = locations.length; var colors = []; diff --git a/Tests/AppKit/CGGradientTest.j b/Tests/AppKit/CGGradientTest.j index 187dd713d..0f89d0c60 100644 --- a/Tests/AppKit/CGGradientTest.j +++ b/Tests/AppKit/CGGradientTest.j @@ -7,6 +7,22 @@ */ - (void)testCreateWithColorComponentsDefault { + + // Test one color and no location + var components = [ 0.2,0.2,0.4,0.4 ]; + + var tmp = CGGradientCreateWithColorComponents(CGColorSpaceCreateDeviceRGB(), + components); + [self assert:1 equals:tmp.colors.length]; + [self assert:1 equals:tmp.locations.length]; + + // TODO: This should not be supported since there is no gradient if only one color + // TODO: NaN not really helpful... + [self assert:NaN equals:tmp.locations[0]]; + + [self assert:components.slice(0,4) equals:tmp.colors[0].components]; + + // Test two colors and no location var components = [ 0.2,0.2,0.4,0.4, 0.6,0.6,0.8,0.8 ]; @@ -20,6 +36,24 @@ [self assert:components.slice(4,8) equals:tmp.colors[1].components]; [self assert:components.slice(0,4) equals:tmp.colors[0].components]; + + // Test three colors and no location + var components = [ 0.2,0.2,0.4,0.4, + 0.6,0.6,0.8,0.8, + 0.6,0.6,0.8,0.8 ]; + + var tmp = CGGradientCreateWithColorComponents(CGColorSpaceCreateDeviceRGB(), + components); + [self assert:3 equals:tmp.colors.length]; + [self assert:3 equals:tmp.locations.length]; + + [self assert:0 equals:tmp.locations[0]]; + [self assert:0.5 equals:tmp.locations[1]]; + [self assert:1.0 equals:tmp.locations[2]]; + + [self assert:components.slice(8,12) equals:tmp.colors[2].components]; + [self assert:components.slice(4,8) equals:tmp.colors[1].components]; + [self assert:components.slice(0,4) equals:tmp.colors[0].components]; } /* From 4ce8dcef12393948d317a7310bf43b300b4cfcbf Mon Sep 17 00:00:00 2001 From: Gerrit Riessen Date: Sat, 23 Apr 2011 15:25:11 +0200 Subject: [PATCH 15/29] testing cgcolorspace --- Tests/AppKit/CGColorSpaceTest.j | 138 ++++++++++++++++++++++++++++++++ 1 file changed, 138 insertions(+) create mode 100644 Tests/AppKit/CGColorSpaceTest.j diff --git a/Tests/AppKit/CGColorSpaceTest.j b/Tests/AppKit/CGColorSpaceTest.j new file mode 100644 index 000000000..2b17da8a6 --- /dev/null +++ b/Tests/AppKit/CGColorSpaceTest.j @@ -0,0 +1,138 @@ +@import + +@implementation CGColorSpaceTest : OJTestCase + +- (void)testColorSpaceCreateWithName +{ + var testdata = { + "CGColorSpaceGenericGray" : { + model: kCGColorSpaceModelMonochrome, + count: 1, + base: NULL + }, + "CGColorSpaceGenericRGB" : { + model: kCGColorSpaceModelRGB, + count: 3, + base: NULL + }, + "CGColorSpaceGenericCMYK" : { + model: kCGColorSpaceModelCMYK, + count: 4, + base: NULL + }, + "CGColorSpaceGenericRGBLinear" : { + model: kCGColorSpaceModelRGB, + count: 3, + base: NULL + }, + "CGColorSpaceGenericRGBHDR" : { + model: kCGColorSpaceModelRGB, + count: 3, + base: NULL + }, + "CGColorSpaceAdobeRGB1998" : { + model: kCGColorSpaceModelRGB, + count: 3, + base: NULL + }, + "CGColorSpaceSRGB" : { + model: kCGColorSpaceModelRGB, + count: 3, + base: NULL + }, + } + + for ( var key in testdata ) { + var clrsp = CGColorSpaceCreateWithName(key), + expdata = testdata[key]; + [self assert:expdata.model equals:clrsp.model message:"model for "+key]; + [self assert:expdata.count equals:clrsp.count message:"count for "+key]; + [self assert:expdata.base equals:clrsp.base message:"base for "+key]; + } + + [self assert:NULL + equals:CGColorSpaceCreateWithName("doesnotexist") + message:"should be missing"]; +} + + +- (void)testColorSpaceStandardizeComponents +{ + // the following models are all not supported. + + var testcases = { + "notsupported1" : { + testdata : { + model: kCGColorSpaceModelIndexed, + count: 5 + }, + expected: [5,4,3,2,1,NaN] // alpha value normalisation adds value at index 5 + }, + "notsupported2" : { + testdata : { + model: kCGColorSpaceModelLab, + count: 5 + }, + expected: [5,4,3,2,1,NaN] + }, + "notsupported3" : { + testdata : { + model: kCGColorSpaceModelPattern, + count: 4 + }, + expected: [5,4,3,2,1] + }, + "base override with count value" : { + testdata : { + model: kCGColorSpaceModelPattern, + base: { model: kCGColorSpaceModelMonochrome }, + count: 4 + }, + expected: [1,1,1,1,1], + }, + "base override no count value" : { + testdata : { + model: kCGColorSpaceModelPattern, + base: { model: kCGColorSpaceModelMonochrome, count: 4 }, + count: 0 + }, + expected: [1,4,3,2,1], // alpha value is assumed to be at index 0 + }, + "monochrome color space model" : { + testdata : { + model: kCGColorSpaceModelMonochrome, + count: 2 + }, + expected: [1,1,1,2,1], + }, + "rgb color space model" : { + testdata : { + model: kCGColorSpaceModelRGB, + count: 2 + }, + expected: [1,1,1,2,1], + }, + "cmyk color space model" : { + testdata : { + model: kCGColorSpaceModelCMYK, + count: 2 + }, + expected: [1,1,1,2,1], + }, + "devicen color space model" : { + testdata : { + model: kCGColorSpaceModelDeviceN, + count: 2 + }, + expected: [1,1,1,2,1], + }, + } + + for ( var key in testcases ) { + var components = [5,4,3,2,1]; + CGColorSpaceStandardizeComponents( testcases[key].testdata, components); + [self assert:testcases[key].expected equals:components message:"Failed for "+key]; + } +} + +@end From 6e6011c5ead1b89576f06504222b7009f1a793a6 Mon Sep 17 00:00:00 2001 From: Gerrit Riessen Date: Sat, 23 Apr 2011 15:50:32 +0200 Subject: [PATCH 16/29] testing rgb colorspace --- Tests/AppKit/CGColorSpaceTest.j | 36 ++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/Tests/AppKit/CGColorSpaceTest.j b/Tests/AppKit/CGColorSpaceTest.j index 2b17da8a6..3df8b4c3f 100644 --- a/Tests/AppKit/CGColorSpaceTest.j +++ b/Tests/AppKit/CGColorSpaceTest.j @@ -55,11 +55,8 @@ message:"should be missing"]; } - - (void)testColorSpaceStandardizeComponents { - // the following models are all not supported. - var testcases = { "notsupported1" : { testdata : { @@ -135,4 +132,37 @@ } } +- (void)testRgbStandardize +{ + var clrspc = CGColorSpaceCreateDeviceRGB(), + testcases = { + "negative values" : { + components: [-5,-4,-3,-2,-1,-1,-1,-1], + expected: [0,0,0,0,-1,-1,-1,-1], + }, + // TODO: this happens because first the alpha value is normalised, + // TODO: so the components array does not have length 0, rather 4. + "missing values become undefined" : { + components: [], + expected: [undefined,undefined,undefined,1], + }, + "values standarized if between 0 and 1" : { + components: [0.3, 0.4, 0.5, 0.6], + expected: [ROUND(0.3*255)/255, ROUND(0.4*255)/255, + ROUND(0.5*255)/255, ROUND(0.6*255)/255], + }, + "values 0 or 1 if zero or one" : { + components: [0, 1, 0, 1], + expected: [0,1,0,1], + } + }; + + for ( var key in testcases ) { + CGColorSpaceStandardizeComponents( clrspc, testcases[key].components); + [self assert:testcases[key].expected + equals:testcases[key].components + message:"Failed for "+key]; + } +} + @end From 63e63ce7a10a7ba9e11f08fd0320d60667d07de7 Mon Sep 17 00:00:00 2001 From: Gerrit Riessen Date: Sat, 23 Apr 2011 17:22:02 +0200 Subject: [PATCH 17/29] fix the color creation when using shortcut functions --- AppKit/CoreGraphics/CGColor.j | 7 ++-- Tests/AppKit/CGColorTest.j | 67 +++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 Tests/AppKit/CGColorTest.j diff --git a/AppKit/CoreGraphics/CGColor.j b/AppKit/CoreGraphics/CGColor.j index 321d091b6..9f141c80c 100644 --- a/AppKit/CoreGraphics/CGColor.j +++ b/AppKit/CoreGraphics/CGColor.j @@ -108,7 +108,7 @@ function CGColorCreateCopy(aColor) */ function CGColorCreateGenericGray(gray, alpha) { - return CGColorCreate(0, [gray, alpha]); + return CGColorCreate(CGColorSpaceCreateDeviceRGB(), [gray,gray,gray, alpha]); } /*! @@ -122,7 +122,7 @@ function CGColorCreateGenericGray(gray, alpha) */ function CGColorCreateGenericRGB(red, green, blue, alpha) { - return CGColorCreate(0, [red, green, blue, alpha]); + return CGColorCreate(CGColorSpaceCreateDeviceRGB(), [red, green, blue, alpha]); } /*! @@ -137,7 +137,8 @@ function CGColorCreateGenericRGB(red, green, blue, alpha) */ function CGColorCreateGenericCMYK(cyan, magenta, yellow, black, alpha) { - return CGColorCreate(0, [cyan, magenta, yellow, black, alpha]); + return CGColorCreate(CGColorSpaceCreateDeviceCMYK(), + [cyan, magenta, yellow, black, alpha]); } /*! diff --git a/Tests/AppKit/CGColorTest.j b/Tests/AppKit/CGColorTest.j new file mode 100644 index 000000000..bb4bf2be6 --- /dev/null +++ b/Tests/AppKit/CGColorTest.j @@ -0,0 +1,67 @@ +@import + +@implementation CGColorTest : OJTestCase + +- (void)testColorCreate +{ + var colorspace = CGColorSpaceCreateDeviceRGB(), + components = [2,3,4,5], + clruid = CFHashCode(colorspace) + components.join(""); + + // Can't access the colormap cache, but it would be nice to + // [self assert:NULL equals:_CGColorMap[clruid]]; + + var clr = CGColorCreate( colorspace, components ); + [self assert:colorspace equals:clr.colorspace message:"colorspace failed"]; + [self assert:[1,1,1,1] equals:clr.components message:"components failed"]; + [self assert:NULL equals:clr.pattern message:"pattern failed"]; + + // Can't access the colormap cache, but it would be nice to + // [self assert:clr equals:_CGColorMap[clruid]]; +} + +- (void)testColorCopy +{ + // it returns it's argument, a little artifical but will fail if something + // changes with the function. + [self assert:"banana" equals:CGColorCreateCopy( "banana" )]; + + var colorspace = CGColorSpaceCreateDeviceRGB(), + components = [2,3,4,5], + clr = CGColorCreate( colorspace, components ); + [self assert:clr equals:CGColorCreateCopy(clr)]; +} + +- (void)testColorCreateGenericGray +{ + var clr = CGColorCreateGenericGray(0.3, 0.5); + [self assert:CGColorSpaceCreateDeviceRGB() + equals:clr.colorspace message:"colorspace failed"]; + [self assert:[ROUND(0.3*255)/255,ROUND(0.3*255)/255,ROUND(0.3*255)/255,0.5] + equals:clr.components message:"components failed"]; + [self assert:NULL equals:clr.pattern message:"pattern failed"]; +} + +- (void)testColorCreateGenericRGB +{ + var clr = CGColorCreateGenericRGB(0.4,0.3,0.2,0.1); + [self assert:CGColorSpaceCreateDeviceRGB() + equals:clr.colorspace message:"colorspace failed"]; + [self assert:[ROUND(0.4*255)/255,ROUND(0.3*255)/255,ROUND(0.2*255)/255,0.1] + equals:clr.components message:"components failed"]; + [self assert:NULL equals:clr.pattern message:"pattern failed"]; +} + +- (void)testColorCreateGenericCMYK +{ + var clr = CGColorCreateGenericCMYK(0.2, 0.3, 0.4, 0.5, 0.6) + [self assert:CGColorSpaceCreateDeviceCMYK() + equals:clr.colorspace message:"colorspace failed"]; + [self assert:[ROUND(0.2*255)/255,ROUND(0.3*255)/255,ROUND(0.4*255)/255, + ROUND(0.5*255)/255, 0.6] + equals:clr.components message:"components failed"]; + [self assert:NULL equals:clr.pattern message:"pattern failed"]; +} + +@end + From e248433ec0b7e2d62dff77d9f9f4f81ed405358a Mon Sep 17 00:00:00 2001 From: Gerrit Riessen Date: Sat, 23 Apr 2011 20:50:42 +0200 Subject: [PATCH 18/29] fixed an issue where setting the alpha after creating the color causes all colors referencing that color in the cache to have their alpha value changed --- AppKit/CoreGraphics/CGColor.j | 18 +++-- Tests/AppKit/CGColorTest.j | 141 ++++++++++++++++++++++++++++++++++ 2 files changed, 151 insertions(+), 8 deletions(-) diff --git a/AppKit/CoreGraphics/CGColor.j b/AppKit/CoreGraphics/CGColor.j index 9f141c80c..7b8ee6088 100644 --- a/AppKit/CoreGraphics/CGColor.j +++ b/AppKit/CoreGraphics/CGColor.j @@ -150,19 +150,21 @@ function CGColorCreateGenericCMYK(cyan, magenta, yellow, black, alpha) */ function CGColorCreateCopyWithAlpha(aColor, anAlpha) { - var components = aColor.components; + if ( !aColor ) return aColor; // Avoid error null pointer in next line - if (!aColor || anAlpha == components[components.length - 1]) + var components = aColor.components.slice(); + + if (anAlpha == components[components.length - 1]) return aColor; + // set new alpha value now so that a potentially a new cache entry is made and + // not that an existing cache entry is mutated. + components[components.length - 1] = anAlpha; + if (aColor.pattern) - var copy = CGColorCreateWithPattern(aColor.colorspace, aColor.pattern, components); + return CGColorCreateWithPattern(aColor.colorspace, aColor.pattern, components); else - var copy = CGColorCreate(aColor.colorspace, components); - - copy.components[components.length - 1] = anAlpha; - - return copy; + return CGColorCreate(aColor.colorspace, components); } /*! diff --git a/Tests/AppKit/CGColorTest.j b/Tests/AppKit/CGColorTest.j index bb4bf2be6..f437b1d46 100644 --- a/Tests/AppKit/CGColorTest.j +++ b/Tests/AppKit/CGColorTest.j @@ -2,6 +2,33 @@ @implementation CGColorTest : OJTestCase +- (void)testColorCreateReturnNullIfNull +{ + [self assert:NULL equals:CGColorCreate( NULL, [1,2,3] ) message:"colorspace null failed"]; + [self assert:NULL + equals:CGColorCreate( CGColorSpaceCreateDeviceRGB(), NULL ) + message:"components null failed"]; +} + +- (void)testColorCreateEnsureComponentsAreCopied +{ + var colorspace = CGColorSpaceCreateDeviceRGB(), + components = [2,3,4,5], + clr = CGColorCreate(colorspace, components); + + components[0] = components[1] = components[2] = components[3] = 0; + [self assert:[1,1,1,1] equals:clr.components]; + [self assert:[0,0,0,0] equals:components]; +} + +- (void)testColorCreateCachesColorsAlphaChangeIsPropagated +{ + var clr = CGColorCreate(CGColorSpaceCreateDeviceRGB(), [0.4, 0.3, 0.2, 0.2]), + newclr = CGColorCreate(CGColorSpaceCreateDeviceRGB(), [0.4, 0.3, 0.2, 0.2]); + newclr.components[newclr.components.length - 1] = 0.6; + [self assert:0.6 equals:clr.components[clr.components.length-1]]; +} + - (void)testColorCreate { var colorspace = CGColorSpaceCreateDeviceRGB(), @@ -63,5 +90,119 @@ [self assert:NULL equals:clr.pattern message:"pattern failed"]; } +- (void)testColorCreateWithPattern +{ + var clrspc = CGColorSpaceCreateDeviceRGB(), + pattern = "a new pattern", + components = [0.2, 0.4, 0.6, 0.8], + clr = CGColorCreateWithPattern(clrspc, pattern, components); + + [self assert:clrspc equals:clr.colorspace]; + [self assert:pattern equals:clr.pattern]; + [self assert:components equals:clr.components]; + + // ensure that there was a copy made of the components + components[0] = components[1] = components[2] = components[3] = 0.0; + [self assert:[0.2, 0.4, 0.6, 0.8] equals:clr.components]; + + // ensure that null is returned + [self assert:NULL equals:CGColorCreateWithPattern(NULL, "a new pattern", [])]; + [self assert:NULL equals:CGColorCreateWithPattern(CGColorSpaceCreateDeviceRGB(), + NULL, [])]; + [self assert:NULL equals:CGColorCreateWithPattern(CGColorSpaceCreateDeviceRGB(), + "a new pattern", NULL)]; + [self assert:NULL equals:CGColorCreateWithPattern(NULL, NULL, NULL)]; +} + +- (void)testColorGetAlpha +{ + var clr = CGColorCreateGenericCMYK(0.2, 0.3, 0.4, 0.5, 0.6); + [self assert:0.6 equals:CGColorGetAlpha(clr)]; +} + +- (void)testColorGetColorSpace +{ + var clr = CGColorCreateGenericCMYK(0.2, 0.3, 0.4, 0.5, 0.6); + [self assert:CGColorSpaceCreateDeviceCMYK() + equals:CGColorGetColorSpace(clr)]; +} + +- (void)testColorGetComponents +{ + var clr = CGColorCreateGenericCMYK(0.2, 0.3, 0.4, 0.5, 0.6); + [self assert:[ROUND(0.2*255)/255, ROUND(0.3*255)/255, ROUND(0.4*255)/255, + ROUND(0.5*255)/255, ROUND(0.6*255)/255] + equals:CGColorGetComponents(clr)]; +} + +- (void)testColorGetNumberOfComponents +{ + var clr = CGColorCreateGenericCMYK(0.2, 0.3, 0.4, 0.5, 0.6); + [self assert:5 equals:CGColorGetNumberOfComponents(clr)]; +} + +- (void)testColorGetPattern +{ + var clr = CGColorCreateWithPattern(CGColorSpaceCreateDeviceRGB(), "a new pattern", + [1,1,1,1]); + [self assert:"a new pattern" equals:CGColorGetPattern(clr)]; +} + +- (void)testColorCreateCopyWithAlphaWithPattern +{ + var clr = CGColorCreateWithPattern(CGColorSpaceCreateDeviceRGB(), "a new pattern", + [1,1,1,1]); + [self assert:1 equals:CGColorGetAlpha(clr) message:"initial alpha"]; + + var newclr = CGColorCreateCopyWithAlpha(clr, 0.6); + [self assert:0.6 equals:CGColorGetAlpha(newclr) message:"new color alpha value"]; + [self assert:1 equals:CGColorGetAlpha(clr) message:"alpha after copy"]; + + [self assert:clr.colorspace equals:newclr.colorspace message:"newclr colorspace failed"]; + [self assert:clr.pattern equals:newclr.pattern message:"newclr pattern failed"]; +} + +- (void)testColorCreateCopyWithAlphaWithComponents +{ + var clr = CGColorCreateGenericRGB(0.4, 0.3, 0.2, 0.3); + [self assert:0.3 equals:CGColorGetAlpha(clr) message:"initial alpha"]; + + var newclr = CGColorCreateCopyWithAlpha(clr, 0.6); + [self assert:0.6 equals:CGColorGetAlpha(newclr) message:"new color alpha value"]; + [self assert:0.3 equals:CGColorGetAlpha(clr) message:"alpha after copy"]; + + [self assert:CGColorSpaceCreateDeviceRGB() + equals:newclr.colorspace message:"newclr colorspace failed"]; + [self assert:[ROUND(0.4*255)/255,ROUND(0.3*255)/255,ROUND(0.2*255)/255,0.6] + equals:newclr.components message:"newclr components failed"]; + [self assert:NULL equals:newclr.pattern message:"newclr pattern failed"]; + + [self assert:CGColorSpaceCreateDeviceRGB() + equals:clr.colorspace message:"orig clr colorspace failed"]; + [self assert:[ROUND(0.4*255)/255,ROUND(0.3*255)/255,ROUND(0.2*255)/255,0.3] + equals:clr.components message:"orig clr components failed"]; + [self assert:NULL equals:clr.pattern message:"orig clr pattern failed"]; +} + +- (void)testColorCreateCopyWithAlphaWithComponentsNullColor +{ + [self assert:NULL + equals:CGColorCreateCopyWithAlpha(NULL,0.5) + message:"null color failed"]; + + var clr = CGColorCreateGenericRGB(0.4,0.3,0.2,0.1), + newclr = CGColorCreateCopyWithAlpha(clr, 0.1); + [self assert:clr equals:newclr]; +} + +- (void)testColorEqualToColor +{ + var clr1 = CGColorCreateGenericRGB(0.4, 0.3, 0.2, 0.3); + [self assert:true equals:CGColorEqualToColor(NULL,NULL)]; + [self assert:false equals:CGColorEqualToColor(NULL,clr1)]; + [self assert:true equals:CGColorEqualToColor(clr1,clr1)]; + [self assert:false equals:CGColorEqualToColor(clr1,NULL)]; +} + @end From c537541825fe82c85e6d2f0ea8b793bbdd31a792 Mon Sep 17 00:00:00 2001 From: Gerrit Riessen Date: Sat, 23 Apr 2011 20:56:23 +0200 Subject: [PATCH 19/29] added test for color equal to color --- Tests/AppKit/CGColorTest.j | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/Tests/AppKit/CGColorTest.j b/Tests/AppKit/CGColorTest.j index f437b1d46..97416ef87 100644 --- a/Tests/AppKit/CGColorTest.j +++ b/Tests/AppKit/CGColorTest.j @@ -197,11 +197,20 @@ - (void)testColorEqualToColor { - var clr1 = CGColorCreateGenericRGB(0.4, 0.3, 0.2, 0.3); [self assert:true equals:CGColorEqualToColor(NULL,NULL)]; + + var clr1 = CGColorCreateGenericRGB(0.4, 0.3, 0.2, 0.3); [self assert:false equals:CGColorEqualToColor(NULL,clr1)]; [self assert:true equals:CGColorEqualToColor(clr1,clr1)]; [self assert:false equals:CGColorEqualToColor(clr1,NULL)]; + + var clr2 = CGColorCreateGenericRGB(0.4, 0.3, 0.2, 0.3); + [self assert:true equals:CGColorEqualToColor(clr1,clr2)]; + [self assert:true equals:CGColorEqualToColor(clr2,clr1)]; + + clr2 = CGColorCreateGenericCMYK(0.2, 0.3, 0.4, 0.5, 0.6); + [self assert:false equals:CGColorEqualToColor(clr1,clr2)]; + [self assert:false equals:CGColorEqualToColor(clr2,clr1)]; } @end From 26afe7c0d84c8e448dd2628c2f98aebbb924c42b Mon Sep 17 00:00:00 2001 From: Randall Luecke Date: Sun, 24 Apr 2011 22:28:41 -0400 Subject: [PATCH 20/29] Fix return type on sortUsingDescriptors in CPMutableArray. --- Foundation/CPArray/CPMutableArray.j | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Foundation/CPArray/CPMutableArray.j b/Foundation/CPArray/CPMutableArray.j index bf0874224..e9588dd51 100644 --- a/Foundation/CPArray/CPMutableArray.j +++ b/Foundation/CPArray/CPMutableArray.j @@ -332,7 +332,7 @@ [self replaceObjectAtIndex:otherIndex withObject:temporary]; } -- (CPArray)sortUsingDescriptors:(CPArray)descriptors +- (void)sortUsingDescriptors:(CPArray)descriptors { [self sortUsingFunction:compareObjectsUsingDescriptors context:descriptors]; } From d147e9cbaa75090d017864b5aef68e490c9b65f5 Mon Sep 17 00:00:00 2001 From: Gerrit Riessen Date: Sun, 24 Apr 2011 12:41:06 +0200 Subject: [PATCH 21/29] unit test for CGAffineTransform --- Tests/AppKit/CGAffineTransformTest.j | 511 +++++++++++++++++++++++++++ Tests/AppKit/CGTestCase.j | 44 +++ 2 files changed, 555 insertions(+) create mode 100644 Tests/AppKit/CGAffineTransformTest.j create mode 100644 Tests/AppKit/CGTestCase.j diff --git a/Tests/AppKit/CGAffineTransformTest.j b/Tests/AppKit/CGAffineTransformTest.j new file mode 100644 index 000000000..f1d47a034 --- /dev/null +++ b/Tests/AppKit/CGAffineTransformTest.j @@ -0,0 +1,511 @@ +@import +@import "CGTestCase.j" + +@implementation CGAffineTransformTest : CGTestCase + +- (void)testAffineTransformMake +{ + [self compareTransform:CGAffineTransformMake(1, 2, 3, 4, 3.2, 5.4) + with:{ a: 1, b: 2, c:3, d:4, tx:3.2, ty:5.4 } + message:"transform make"]; +} + +- (void)testCGAffineTransformMakeIdentity +{ + [self compareTransform:CGAffineTransformMakeIdentity() + with:{ a: 1, b: 0, c:0, d:1, tx:0, ty:0 } + message:"transform make identity"]; +} + +- (void)testAffineTransformMakeCopy +{ + var transform = CGAffineTransformMake(1, 2, 3, 4, 3.2, 5.4), + t2 = CGAffineTransformMakeCopy(transform); + + transform.a = transform.b = transform.c = transform.d = + transform.tx = transform.ty = 0; + + [self compareTransform:t2 + with:CGAffineTransformMake( 1,2,3,4,3.2,5.4 ) + message:"copy correctly made"]; + + [self compareTransform:transform + with:CGAffineTransformMake( 0, 0, 0, 0, 0, 0 ) + message:"original transform was changed"] ; +} + +- (void)testAffineTransformCreateCopy +{ + // FIXME: CGAffineTransformCreateCopy seems to be working but code has a Fixme? + var transform = CGAffineTransformMake(1, 2, 3, 4, 3.2, 5.4), + t2 = CGAffineTransformCreateCopy(transform); + + transform.a = transform.b = transform.c = transform.d = + transform.tx = transform.ty = 0; + + [self compareTransform:t2 + with:CGAffineTransformMake( 1,2,3,4,3.2,5.4 ) + message:"copy correctly made"]; + + [self compareTransform:transform + with:CGAffineTransformMake( 0, 0, 0, 0, 0, 0 ) + message:"original transform was changed"] ; +} + +- (void)testAffineTransformMakeScale +{ + [self compareTransform:CGAffineTransformMakeScale(3,4) + with:CGAffineTransformMake(3, 0, 0, 4, 0, 0) + message:"make scale"]; +} + +- (void)testAffineTransformMakeTranslation +{ + [self compareTransform:CGAffineTransformMakeTranslation(3,4) + with:CGAffineTransformMake(1,0,0,1,3,4) + message:"make translation"]; +} + +- (void)testAffineTransformTranslate +{ + var transform = CGAffineTransformMakeTranslation(3,4); + + [self compareTransform:CGAffineTransformTranslate(transform,-3,-4) + with:CGAffineTransformMakeIdentity() + message:"translate to identity"]; + + [self compareTransform:CGAffineTransformTranslate(transform,0,0) + with:transform + message:"zero translate"]; +} + +- (void)testAffineTransformScale +{ + var transform = CGAffineTransformMakeScale(3,4); + + [self compareTransform:CGAffineTransformScale( transform, 1/3, 1/4) + with:CGAffineTransformMakeIdentity() + message:"scale to identity"]; + + transform = CGAffineTransformMake(2, -2, -3, 3, 3.2, 5.4); + [self compareTransform:CGAffineTransformScale(transform, 1, 1) + with:transform + message:"scale by 1"]; + + [self compareTransform:CGAffineTransformScale(transform, 2, 5) + with:CGAffineTransformMake(4,-4,-15,15, 3.2,5.4) + message:"random scale to somewhere"]; +} + +- (void)testAffineTransformConcat +{ + var testcases = { + "identity concat" : { + testdata: CGAffineTransformConcat( CGAffineTransformMakeIdentity(), + CGAffineTransformMakeIdentity() ), + expdata: CGAffineTransformMakeIdentity() + }, + + "translation" : { + testdata: CGAffineTransformConcat( CGAffineTransformMakeTranslation(3,4), + CGAffineTransformMakeTranslation(-3,-4) ), + expdata: CGAffineTransformMakeIdentity() + }, + + "translation (reversed)" : { + testdata: CGAffineTransformConcat( CGAffineTransformMakeTranslation(-3,-4), + CGAffineTransformMakeTranslation(3,4) ), + expdata: CGAffineTransformMakeIdentity() + }, + + "scale" : { + testdata: CGAffineTransformConcat(CGAffineTransformMakeScale(3,4), + CGAffineTransformMakeScale(1/3,1/4)), + expdata: CGAffineTransformMakeIdentity() + }, + + "scale (reversed)" : { + testdata: CGAffineTransformConcat(CGAffineTransformMakeScale(1/3,1/4), + CGAffineTransformMakeScale(3,4)), + expdata: CGAffineTransformMakeIdentity() + }, + + "rotation" : { + testdata: CGAffineTransformConcat(CGAffineTransformMakeRotation(Math.PI), + CGAffineTransformMakeRotation(-Math.PI)), + expdata: CGAffineTransformMakeIdentity() + }, + }; + + for ( var key in testcases ) + [self compareTransform:testcases[key].testdata with:testcases[key].expdata message:key]; +} + +- (void)testPointApplyAffineTransform +{ + var testcases = { + "translate to zero" : { + testdata: CGPointApplyAffineTransform( CGPointMake( 3, 4 ), + CGAffineTransformMakeTranslation(-3,-4)), + expdata: CGPointMakeZero() + }, + + "scale to 1,1" : { + testdata: CGPointApplyAffineTransform( CGPointMake( 3, 4 ), + CGAffineTransformMakeScale(1/3,1/4) ), + expdata: CGPointMake(1,1) + }, + + "scale and translate to zero" : { + testdata: CGPointApplyAffineTransform( CGPointMake( 3, 4 ), + CGAffineTransformConcat( + CGAffineTransformMakeScale(1/3,1/4), + CGAffineTransformMakeTranslation(-1,-1))), + expdata: CGPointMakeZero() + + }, + }; + + for ( var key in testcases ) + [self comparePoint:testcases[key].testdata with:testcases[key].expdata message:key]; +} + +- (void)testSizeApplyAffineTransform +{ + var testcases = { + "translation on size should do nothing" : { + testdata: CGSizeApplyAffineTransform( CGSizeMake(3, 12), + CGAffineTransformMakeTranslation(-3,-4)), + expdata: CGSizeMake(3, 12) + }, + + "scale to 1,1" : { + testdata: CGSizeApplyAffineTransform( CGSizeMake( 3, 4 ), + CGAffineTransformMakeScale(1/3,1/4) ), + expdata: CGSizeMake(1, 1) + }, + + "scale and translate combined" : { + testdata: CGSizeApplyAffineTransform( CGSizeMake( 3, 4 ), + CGAffineTransformConcat( + CGAffineTransformMakeScale(1/3,1/4), + CGAffineTransformMakeTranslation(-1,-1))), + expdata: CGSizeMake(1, 1) + + }, + }; + + for ( var key in testcases ) + [self compareSize:testcases[key].testdata with:testcases[key].expdata message:key]; +} + +- (void)testAffineTransformIsIdentityPositive +{ + var testcases = { + "identity is identity" : { + testdata: CGAffineTransformMakeIdentity() + }, + + "zero rotation is identity" : { + testdata: CGAffineTransformMakeRotation(0), + }, + + "zero translation is identity" : { + testdata: CGAffineTransformMakeTranslation(0,0) + }, + + "one scale is identity" : { + testdata: CGAffineTransformMakeScale(1,1) + }, + + "identity concat'ed" : { + testdata: CGAffineTransformConcat( CGAffineTransformMakeIdentity(), + CGAffineTransformMakeIdentity() ), + }, + + "translation" : { + testdata: CGAffineTransformConcat( CGAffineTransformMakeTranslation(3,4), + CGAffineTransformMakeTranslation(-3,-4) ), + }, + + "scale" : { + testdata: CGAffineTransformConcat(CGAffineTransformMakeScale(3,4), + CGAffineTransformMakeScale(1/3,1/4)), + }, + + "rotation" : { + testdata: CGAffineTransformConcat(CGAffineTransformMakeRotation(-Math.PI), + CGAffineTransformMakeRotation(Math.PI)), + }, + + }; + + for ( var key in testcases ) + [self assert:YES + equals:CGAffineTransformIsIdentity(testcases[key].testdata) + message:key]; +} + +- (void)testAffineTransformIsIdentityNegative +{ + var testcases = { + "some random transform" : { + testdata: CGAffineTransformMake(1,1,1,1,1,1) + }, + + "non-zero translation is not identity" : { + testdata: CGAffineTransformMakeTranslation(1,1) + }, + + "non-one scale is not identity" : { + testdata: CGAffineTransformMakeScale(2,2) + }, + + "rotation" : { + testdata: CGAffineTransformMakeRotation(Math.PI), + }, + + // TODO a two-pi rotation is actually identity + "2PI rotation is NOT identity?" : { + testdata: CGAffineTransformMakeRotation(Math.PI * 2), + }, + + }; + + for ( var key in testcases ) + [self assert:NO + equals:CGAffineTransformIsIdentity(testcases[key].testdata) + message:key]; +} + +- (void)testAffineTransformEqualToTransform +{ + var testcases = { + "identity" : { + lhs: CGAffineTransformMakeIdentity(), + rhs: CGAffineTransformMakeIdentity(), + expdata: YES + }, + + "translate" : { + lhs: CGAffineTransformMakeTranslation(1,1), + rhs: CGAffineTransformMakeTranslation(1,1), + expdata: YES + }, + + "scale" : { + lhs: CGAffineTransformMakeScale(1,1), + rhs: CGAffineTransformMakeScale(1,1), + expdata: YES + }, + + "rotation" : { + lhs: CGAffineTransformMakeRotation(Math.PI), + rhs: CGAffineTransformMakeRotation(Math.PI), + expdata: YES + }, + + "translate and scale" : { + lhs: CGAffineTransformMakeScale(1,1), + rhs: CGAffineTransformMakeTranslation(1,1), + expdata: NO + }, + }; + + for ( var key in testcases ) + [self assert:testcases[key].expdata + equals:CGAffineTransformEqualToTransform(testcases[key].lhs, testcases[key].rhs) + message:key]; +} + +- (void)testStringCreateWithCGAffineTransform +{ + // FIXME?: should there be a leading space on these strings + var testcases = { + "identity" : { + testdata: CGAffineTransformMakeIdentity(), + expdata: " [[ 1, 0, 0 ], [ 0, 1, 0 ], [ 0, 0, 1]]" + }, + + "scale" : { + testdata: CGAffineTransformMakeScale(3,4), + expdata: " [[ 3, 0, 0 ], [ 0, 4, 0 ], [ 0, 0, 1]]" + }, + + "translation" : { + testdata: CGAffineTransformMakeTranslation(3,4), + expdata: " [[ 1, 0, 0 ], [ 0, 1, 0 ], [ 3, 4, 1]]" + }, + + "scale and translation" : { + testdata: CGAffineTransformTranslate(CGAffineTransformMakeScale(3,4),5,6), + expdata: " [[ 3, 0, 0 ], [ 0, 4, 0 ], [ 15, 24, 1]]" + }, + }; + + for ( var key in testcases ) + [self assert:testcases[key].expdata + equals:CGStringCreateWithCGAffineTransform(testcases[key].testdata) + message:key]; +} + +- (void)testStringFromCGAffineTransform +{ + var testcases = { + "identity" : { + testdata: CGAffineTransformMakeIdentity(), + expdata: "{1, 0, 0, 1, 0, 0}" + }, + + "scale" : { + testdata: CGAffineTransformMakeScale(3,4), + expdata: "{3, 0, 0, 4, 0, 0}" + }, + + "translation" : { + testdata: CGAffineTransformMakeTranslation(3,4), + expdata: "{1, 0, 0, 1, 3, 4}" + }, + + "rotation - zero" : { + testdata: CGAffineTransformMakeRotation(0), + expdata: "{1, 0, 0, 1, 0, 0}" + }, + + "rotation - pi" : { + testdata: CGAffineTransformMakeRotation(Math.PI), + expdata: "{-1, 1.2246467991473532e-16, -1.2246467991473532e-16, -1, 0, 0}" + }, + + "rotation - 2pi" : { + testdata: CGAffineTransformMakeRotation(2 * Math.PI), + expdata: "{1, -2.4492935982947064e-16, 2.4492935982947064e-16, 1, 0, 0}" + }, + + "rotation - 3pi" : { + testdata: CGAffineTransformMakeRotation(3 * Math.PI), + expdata: "{-1, 3.6739403974420594e-16, -3.6739403974420594e-16, -1, 0, 0}" + }, + + "scale and translation and rotate" : { + testdata: CGAffineTransformRotate(CGAffineTransformTranslate(CGAffineTransformMakeScale(3,4),5,6),Math.PI), + expdata: "{-3, 4.898587196589413e-16, -3.6739403974420594e-16, -4, 15, 24}" + }, + }; + + for ( var key in testcases ) + [self assert:testcases[key].expdata + equals:CPStringFromCGAffineTransform(testcases[key].testdata) + message:key]; +} + +- (void)testAffineTransformRotate +{ + var ang = Math.PI + 0.5 * Math.PI, + transform = CGAffineTransformMake( 1,2,3,4, 5,6 ), + cos = COS(ang), + sin = SIN(ang); + + var testcases = { + "affine transform rotate failed" : { + testdata: CGAffineTransformRotate(transform, ang), + expdata: CGAffineTransformMake( transform.a * cos + transform.c * sin, + transform.b * cos + transform.d * sin, + transform.c * cos - transform.a * sin, + transform.d * cos - transform.b * sin, + transform.tx,transform.ty ) + }, + + "rotation negation" : { + testdata: CGAffineTransformRotate(CGAffineTransformRotate(CGAffineTransformMakeScale(3,4),Math.PI),-Math.PI), + expdata: CGAffineTransformMakeScale(3,4) + } + }; + + for ( var key in testcases ) + [self compareTransform:testcases[key].testdata with:testcases[key].expdata message:key]; +} + +- (void)testAffineTransformInvert +{ + var transform = CGAffineTransformRotate(CGAffineTransformTranslate(CGAffineTransformMakeScale(3,4),5,6),Math.PI), + determinant = 1 / (transform.a * transform.d - transform.b * transform.c), + invertedtransform = CGAffineTransformMake(determinant * transform.d, + -determinant * transform.b, + -determinant * transform.c, + determinant * transform.a, + determinant * (transform.c * transform.ty - + transform.d * transform.tx), + determinant * (transform.b * transform.tx - + transform.a * transform.ty)); + + var testcases = { + "test invert algorithm" : { + testdata: transform, + expdata: invertedtransform + }, + + "identity should be it self on inversion" : { + testdata: CGAffineTransformMakeIdentity(), + expdata: CGAffineTransformMakeIdentity() + }, + + "rotation" : { + testdata: CGAffineTransformMakeRotation(-Math.PI), + expdata: CGAffineTransformMakeRotation(Math.PI), + }, + + "translation" : { + testdata: CGAffineTransformMakeTranslation(4,5), + expdata: CGAffineTransformMakeTranslation(-4,-5) + }, + + "scale" : { + testdata: CGAffineTransformMakeScale(3,4), + expdata: CGAffineTransformMakeScale(1/3,1/4) + }, + }; + + for ( var key in testcases ) + [self compareTransform:CGAffineTransformInvert(testcases[key].testdata) + with:testcases[key].expdata + message:key]; +} + +- (void)testRectApplyAffineTransform +{ + var rect = CGRectMake( 3,4,5,6 ); + + var testcases = { + "identity does nothing" : { + testdata: CGRectApplyAffineTransform(rect, CGAffineTransformMakeIdentity() ), + expdata: rect + }, + + "rotation 90 degrees" : { + testdata: CGRectApplyAffineTransform(rect, CGAffineTransformMakeRotation(Math.PI/2)), + expdata: CGRectMake( -10,3.0000000000000004,6,5 ) + }, + + "translation" : { + testdata: CGRectApplyAffineTransform(rect, CGAffineTransformMakeTranslation(3,4)), + expdata: CGRectMake( 6,8,5,6 ) + }, + + "scale" : { + testdata: CGRectApplyAffineTransform(rect, CGAffineTransformMakeScale(1,4)), + expdata: CGRectMake( 3,16,5,24 ) + }, + + "rotate, translate and scale" : { + testdata: CGRectApplyAffineTransform(rect, CGAffineTransformRotate(CGAffineTransformTranslate(CGAffineTransformMakeScale(3,4),5,6),Math.PI)), + expdata: CGRectMake( -9.000000000000004, -16, + 15.000000000000002, 24.000000000000004 ) + }, + }; + + for ( var key in testcases ) + [self compareRect:testcases[key].testdata with:testcases[key].expdata message:key]; +} + +@end diff --git a/Tests/AppKit/CGTestCase.j b/Tests/AppKit/CGTestCase.j new file mode 100644 index 000000000..e00bb1e11 --- /dev/null +++ b/Tests/AppKit/CGTestCase.j @@ -0,0 +1,44 @@ +/* + Used for comparing geometry forms. Otherwise each unit test would repeat these + comparisons. +*/ +@implementation CGTestCase : OJTestCase + +- (void)compareRect:(CGRect)aRect + with:(id)anotherRect + message:(CPString)aMsg +{ + aMsg += " (via cmp. rect)"; + [self comparePoint:aRect.origin with:anotherRect.origin message:aMsg]; + [self compareSize:aRect.size with:anotherRect.size message:aMsg]; +} + +- (void)compareSize:(CGSize)aSize + with:(id)anotherSize + message:(CPString)aMsg +{ + [self assert:anotherSize.width equals:aSize.width message:aMsg + ": Failed for width"]; + [self assert:anotherSize.height equals:aSize.height message:aMsg + ": Failed for height"]; +} + +- (void)comparePoint:(CGPoint)aPoint + with:(id)anotherPoint + message:(CPString)aMsg +{ + [self assert:anotherPoint.x equals:aPoint.x message:aMsg + ": Failed for x"]; + [self assert:anotherPoint.y equals:aPoint.y message:aMsg + ": Failed for y"]; +} + +- (void)compareTransform:(CGAffineTransform)aTransform + with:(id)aDataSet + message:(CPString)aMsg +{ + [self assert:aDataSet.a equals:aTransform.a message:aMsg + ": Failed for a"]; + [self assert:aDataSet.b equals:aTransform.b message:aMsg + ": Failed for b"]; + [self assert:aDataSet.c equals:aTransform.c message:aMsg + ": Failed for c"]; + [self assert:aDataSet.d equals:aTransform.d message:aMsg + ": Failed for d"]; + [self assert:aDataSet.tx equals:aTransform.tx message:aMsg + ": Failed for tx"]; + [self assert:aDataSet.ty equals:aTransform.ty message:aMsg + ": Failed for ty"]; +} + +@end From 6a4d8a24be35668eb7c7f415ad0c4d93548e7c3c Mon Sep 17 00:00:00 2001 From: Gerrit Riessen Date: Mon, 25 Apr 2011 13:34:52 +0200 Subject: [PATCH 22/29] unit test for CGContext --- Tests/AppKit/CGContextTest.j | 78 ++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 Tests/AppKit/CGContextTest.j diff --git a/Tests/AppKit/CGContextTest.j b/Tests/AppKit/CGContextTest.j new file mode 100644 index 000000000..0ed6242e6 --- /dev/null +++ b/Tests/AppKit/CGContextTest.j @@ -0,0 +1,78 @@ +@import +@import "CGTestCase.j" + +@implementation CGContextTest : CGTestCase + +- (void)testEnsureTestingCanvas +{ + /* + Ensure that we have no canvas nor vml support. + */ + [self assert:YES equals:!CPFeatureIsCompatible(CPHTMLCanvasFeature)]; + [self assert:YES equals:!CPFeatureIsCompatible(CPVMLFeature)]; +} + +- (void)testGStateCreate +{ + var gstate = CGGStateCreate(), + testdata = { alpha: 1.0, + strokeStyle: "#000", + fillStyle: "#ccc", + lineWidth: 1.0, + lineJoin: kCGLineJoinMiter, + lineCap: kCGLineCapButt, + miterLimit: 10.0, + globalAlpha: 1.0, + blendMode: kCGBlendModeNormal, + shadowBlur: 0.0, + shadowColor: NULL }; + + for ( var key in testdata ) + [self assert:testdata[key] equals:gstate[key] message:"Failed for " + key]; + + [self compareSize:CGSizeMakeZero() + with:gstate.shadowOffset + message:"Failed for shadowOffset"]; + + [self compareTransform:CGAffineTransformMakeIdentity() + with:gstate.CTM + message:"Failed for CTM"]; +} + +- (void)testGStateCreateCopy +{ + var gstate = CGGStateCreate(), + gstatecopy = CGGStateCreateCopy(gstate), + testdata = { alpha: 1.0, + strokeStyle: "#000", + fillStyle: "#ccc", + lineWidth: 1.0, + lineJoin: kCGLineJoinMiter, + lineCap: kCGLineCapButt, + miterLimit: 10.0, + globalAlpha: 1.0, + blendMode: kCGBlendModeNormal, + shadowBlur: 0.0, + shadowColor: NULL }; + + for ( var key in testdata ) + [self assert:testdata[key] equals:gstatecopy[key] message:"Failed for " + key]; + + // ensure that the size is copied + var sizecopy = CGSizeMakeCopy( gstate.shadowOffset ); + gstate.shadowOffset.width += 1000; + gstate.shadowOffset.height += 2000; + [self compareSize:sizecopy with:gstatecopy.shadowOffset message:"Failed for shadowOffset"]; + + // ensure that the transform is copied + var transformcopy = CGAffineTransformMakeCopy( gstate.CTM ); + gstate.CTM.a += 1000; + gstate.CTM.b += 1000; + gstate.CTM.c += 1000; + gstate.CTM.d += 1000; + gstate.CTM.tx += 1000; + gstate.CTM.ty += 1000; + [self compareTransform:transformcopy with:gstatecopy.CTM message:"Failed with CTM"]; +} + +@end From 6c12442870f7eb0e551c841a9ab9db88274148f4 Mon Sep 17 00:00:00 2001 From: Gerrit Riessen Date: Mon, 25 Apr 2011 14:10:49 +0200 Subject: [PATCH 23/29] Fixed copy the shadowOffset, similar to the copying of the affine transform --- AppKit/CoreGraphics/CGContext.j | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AppKit/CoreGraphics/CGContext.j b/AppKit/CoreGraphics/CGContext.j index 88de992c1..a67d7b200 100644 --- a/AppKit/CoreGraphics/CGContext.j +++ b/AppKit/CoreGraphics/CGContext.j @@ -129,7 +129,7 @@ function CGGStateCreateCopy(aGState) return { alpha:aGState.alpha, strokeStyle:aGState.strokeStyle, fillStyle:aGState.fillStyle, lineWidth:aGState.lineWidth, lineJoin:aGState.lineJoin, lineCap:aGState.lineCap, miterLimit:aGState.miterLimit, globalAlpha:aGState.globalAlpha, blendMode:aGState.blendMode, - shadowOffset:aGState.shadowOffset, shadowBlur:aGState.shadowBlur, shadowColor:aGState.shadowColor, CTM:_CGAffineTransformMakeCopy(aGState.CTM) }; + shadowOffset:_CGSizeMakeCopy(aGState.shadowOffset), shadowBlur:aGState.shadowBlur, shadowColor:aGState.shadowColor, CTM:_CGAffineTransformMakeCopy(aGState.CTM) }; } /*! From 1b3792a1cc436ca9e19376d5f468ffd09febd22f Mon Sep 17 00:00:00 2001 From: Randall Luecke Date: Mon, 25 Apr 2011 17:13:43 -0400 Subject: [PATCH 24/29] Exclude build directory in xcodecapp. --- Tools/xcodecapp/XCResourceMonitor.j | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tools/xcodecapp/XCResourceMonitor.j b/Tools/xcodecapp/XCResourceMonitor.j index d0fb64191..b6241090f 100644 --- a/Tools/xcodecapp/XCResourceMonitor.j +++ b/Tools/xcodecapp/XCResourceMonitor.j @@ -40,6 +40,8 @@ var FILE = require("file"), paths.exclude(FILE.join(FILE.dirname(aPath), "**", "*")); }); + paths.exclude(FILE.join("Build", "**", "*")); + var mtimesForFilePaths = [CPMutableDictionary new]; paths.forEach(function(aPath) From cde179786cfe3647fbde646b3a5a2c26d9a5ef09 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Tue, 26 Apr 2011 11:16:36 +0200 Subject: [PATCH 25/29] fail more gracefully when attempting to remove the same observer twice Before this commit we would fail gracefully for single keys. For key paths an application would crash because we would try to get the forwarder property of an undefined object. This commit makes sure we don't attempt any removal and logs a warning (just like Cocoa) about the attempt to remove an unregistered observer. --- Foundation/CPKeyValueObserving.j | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Foundation/CPKeyValueObserving.j b/Foundation/CPKeyValueObserving.j index fbac24c09..ee72d91fc 100644 --- a/Foundation/CPKeyValueObserving.j +++ b/Foundation/CPKeyValueObserving.j @@ -678,6 +678,14 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti { var observers = _observersForKey[aPath]; + if (!observers) + { + CPLog.warn(@"Cannot remove an observer %@ for the key path \"%@\" from %@ because it is not registered as an observer.", + _targetObject, aPath, anObserver); + + return; + } + if (aPath.indexOf('.') != CPNotFound) { var forwarder = [observers objectForKey:[anObserver UID]].forwarder; From afb2352a7cb83c2875c808d66de8529f38b8eac7 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Tue, 26 Apr 2011 11:20:47 +0200 Subject: [PATCH 26/29] consolidate both CPKeyValueObservingTests into one --- Tests/AppKit/CPKeyValueObservingTest.j | 106 --------------------- Tests/Foundation/CPKeyValueObservingTest.j | 95 ++++++++++++++++++ 2 files changed, 95 insertions(+), 106 deletions(-) delete mode 100644 Tests/AppKit/CPKeyValueObservingTest.j diff --git a/Tests/AppKit/CPKeyValueObservingTest.j b/Tests/AppKit/CPKeyValueObservingTest.j deleted file mode 100644 index 67e3cd07b..000000000 --- a/Tests/AppKit/CPKeyValueObservingTest.j +++ /dev/null @@ -1,106 +0,0 @@ -@import - -@implementation CPKeyValueObservingTest : OJTestCase -{ - CPString _lastKeyPath; - id _lastObject; - CPDictionary _lastChange; - id _lastContext; -} - -- (void)setup -{ - _lastKeyPath = _lastObject = _lastChange = _lastContext = nil; -} - -- (void)testInitialObserving -{ - var tester = [ObservingTester testerWithCheese:@"CHEESE!"]; - [tester addObserver:self forKeyPath:@"cheese" options:CPKeyValueObservingOptionNew | CPKeyValueObservingOptionInitial context:nil]; - - [self assert:@"cheese" equals:_lastKeyPath]; - [self assert:tester equals:_lastObject]; - [self assert:[CPDictionary dictionaryWithObject:@"CHEESE!" forKey:CPKeyValueChangeNewKey] equals:_lastChange]; - [self assert:nil equals:_lastContext]; -} - -- (void)observeValueForKeyPath:(CPString)aKeyPath - ofObject:(id)anObject - change:(CPDictionary)aChange - context:(id)aContext -{ - _lastKeyPath = aKeyPath; - _lastObject = anObject; - _lastChange = aChange; - _lastContext = aContext; -} - -- (void)testSendNotificationsForDependantKeyPaths -{ - var observingTester = [ObservingTester testerWithCheese:@"cheese"], - dependantKeyPathTester = [DependantKeyPathsTester testerWithObservingTester:observingTester]; - - [dependantKeyPathTester addObserver:self forKeyPath:@"observedCheese" options:CPKeyValueObservingOptionNew context:nil]; - [observingTester setCheese:@"changed cheese"]; - - [self assert:@"observedCheese" equals:_lastKeyPath] - [self assert:dependantKeyPathTester equals:_lastObject]; -} - -@end - -@implementation ObservingTester : CPObject -{ - id cheese; -} - -+ (id)testerWithCheese:(id)aCheese -{ - var tester = [[self alloc] init]; - [tester setCheese:aCheese]; - return tester; -} - -- (void)setCheese:(id)aCheese -{ - cheese = aCheese; -} - -- (id)cheese -{ - return cheese; -} - -@end - -@implementation DependantKeyPathsTester: CPObject -{ - ObservingTester _observingTester @accessors(property=observingTester); -} - -+ (CPSet)keyPathsForValuesAffectingObservedCheese -{ - return [CPSet setWithObjects:@"observingTester.cheese"]; -} - -+ (id)testerWithObservingTester:(ObservingTester)theObservingTester -{ - return [[self alloc] initWithObservingTester:theObservingTester]; -} - -- (id)initWithObservingTester:(ObservingTester)theObservingTester -{ - if (self = [super init]) - { - _observingTester = theObservingTester; - } - - return self; -} - -- (CPString)observedCheese -{ - return [[self observingTester] cheese]; -} - -@end diff --git a/Tests/Foundation/CPKeyValueObservingTest.j b/Tests/Foundation/CPKeyValueObservingTest.j index 5d43fc8da..2d409618f 100644 --- a/Tests/Foundation/CPKeyValueObservingTest.j +++ b/Tests/Foundation/CPKeyValueObservingTest.j @@ -22,6 +22,45 @@ return [theClass new]; } +- (void)setup +{ + _lastKeyPath = _lastObject = _lastChange = _lastContext = nil; +} + +- (void)testInitialObserving +{ + var tester = [ObservingTester testerWithCheese:@"CHEESE!"]; + [tester addObserver:self forKeyPath:@"cheese" options:CPKeyValueObservingOptionNew | CPKeyValueObservingOptionInitial context:nil]; + + [self assert:@"cheese" equals:_lastKeyPath]; + [self assert:tester equals:_lastObject]; + [self assert:[CPDictionary dictionaryWithObject:@"CHEESE!" forKey:CPKeyValueChangeNewKey] equals:_lastChange]; + [self assert:nil equals:_lastContext]; +} + +- (void)observeValueForKeyPath:(CPString)aKeyPath + ofObject:(id)anObject + change:(CPDictionary)aChange + context:(id)aContext +{ + _lastKeyPath = aKeyPath; + _lastObject = anObject; + _lastChange = aChange; + _lastContext = aContext; +} + +- (void)testSendNotificationsForDependantKeyPaths +{ + var observingTester = [ObservingTester testerWithCheese:@"cheese"], + dependantKeyPathTester = [DependantKeyPathsTester testerWithObservingTester:observingTester]; + + [dependantKeyPathTester addObserver:self forKeyPath:@"observedCheese" options:CPKeyValueObservingOptionNew context:nil]; + [observingTester setCheese:@"changed cheese"]; + + [self assert:@"observedCheese" equals:_lastKeyPath] + [self assert:dependantKeyPathTester equals:_lastObject]; +} + - (void)testOnlyInsertObject_AtKeyIndex_Implemented { var insertSelector = @selector(insertObject:inObjectsAtIndex:), @@ -155,3 +194,59 @@ } @end + +@implementation ObservingTester : CPObject +{ + id cheese; +} + ++ (id)testerWithCheese:(id)aCheese +{ + var tester = [[self alloc] init]; + [tester setCheese:aCheese]; + return tester; +} + +- (void)setCheese:(id)aCheese +{ + cheese = aCheese; +} + +- (id)cheese +{ + return cheese; +} + +@end + +@implementation DependantKeyPathsTester: CPObject +{ + ObservingTester _observingTester @accessors(property=observingTester); +} + ++ (CPSet)keyPathsForValuesAffectingObservedCheese +{ + return [CPSet setWithObjects:@"observingTester.cheese"]; +} + ++ (id)testerWithObservingTester:(ObservingTester)theObservingTester +{ + return [[self alloc] initWithObservingTester:theObservingTester]; +} + +- (id)initWithObservingTester:(ObservingTester)theObservingTester +{ + if (self = [super init]) + { + _observingTester = theObservingTester; + } + + return self; +} + +- (CPString)observedCheese +{ + return [[self observingTester] cheese]; +} + +@end From 03e7d81ffa79887d2f45adefdadd427e9d7ec7f6 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Tue, 26 Apr 2011 11:16:36 +0200 Subject: [PATCH 27/29] fail more gracefully when attempting to remove the same observer twice Before this commit we would fail gracefully for single keys. For key paths an application would crash because we would try to get the forwarder property of an undefined object. This commit makes sure we don't attempt any removal and logs a warning (just like Cocoa) about the attempt to remove an unregistered observer. --- Foundation/CPKeyValueObserving.j | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Foundation/CPKeyValueObserving.j b/Foundation/CPKeyValueObserving.j index fbac24c09..ee72d91fc 100644 --- a/Foundation/CPKeyValueObserving.j +++ b/Foundation/CPKeyValueObserving.j @@ -678,6 +678,14 @@ var kvoNewAndOld = CPKeyValueObservingOptionNew | CPKeyValueObservingOpti { var observers = _observersForKey[aPath]; + if (!observers) + { + CPLog.warn(@"Cannot remove an observer %@ for the key path \"%@\" from %@ because it is not registered as an observer.", + _targetObject, aPath, anObserver); + + return; + } + if (aPath.indexOf('.') != CPNotFound) { var forwarder = [observers objectForKey:[anObserver UID]].forwarder; From 0ee304058d397d25502853c76abdd48ab09ad7e5 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Tue, 26 Apr 2011 11:20:47 +0200 Subject: [PATCH 28/29] consolidate both CPKeyValueObservingTests into one --- Tests/AppKit/CPKeyValueObservingTest.j | 106 --------------------- Tests/Foundation/CPKeyValueObservingTest.j | 95 ++++++++++++++++++ 2 files changed, 95 insertions(+), 106 deletions(-) delete mode 100644 Tests/AppKit/CPKeyValueObservingTest.j diff --git a/Tests/AppKit/CPKeyValueObservingTest.j b/Tests/AppKit/CPKeyValueObservingTest.j deleted file mode 100644 index 67e3cd07b..000000000 --- a/Tests/AppKit/CPKeyValueObservingTest.j +++ /dev/null @@ -1,106 +0,0 @@ -@import - -@implementation CPKeyValueObservingTest : OJTestCase -{ - CPString _lastKeyPath; - id _lastObject; - CPDictionary _lastChange; - id _lastContext; -} - -- (void)setup -{ - _lastKeyPath = _lastObject = _lastChange = _lastContext = nil; -} - -- (void)testInitialObserving -{ - var tester = [ObservingTester testerWithCheese:@"CHEESE!"]; - [tester addObserver:self forKeyPath:@"cheese" options:CPKeyValueObservingOptionNew | CPKeyValueObservingOptionInitial context:nil]; - - [self assert:@"cheese" equals:_lastKeyPath]; - [self assert:tester equals:_lastObject]; - [self assert:[CPDictionary dictionaryWithObject:@"CHEESE!" forKey:CPKeyValueChangeNewKey] equals:_lastChange]; - [self assert:nil equals:_lastContext]; -} - -- (void)observeValueForKeyPath:(CPString)aKeyPath - ofObject:(id)anObject - change:(CPDictionary)aChange - context:(id)aContext -{ - _lastKeyPath = aKeyPath; - _lastObject = anObject; - _lastChange = aChange; - _lastContext = aContext; -} - -- (void)testSendNotificationsForDependantKeyPaths -{ - var observingTester = [ObservingTester testerWithCheese:@"cheese"], - dependantKeyPathTester = [DependantKeyPathsTester testerWithObservingTester:observingTester]; - - [dependantKeyPathTester addObserver:self forKeyPath:@"observedCheese" options:CPKeyValueObservingOptionNew context:nil]; - [observingTester setCheese:@"changed cheese"]; - - [self assert:@"observedCheese" equals:_lastKeyPath] - [self assert:dependantKeyPathTester equals:_lastObject]; -} - -@end - -@implementation ObservingTester : CPObject -{ - id cheese; -} - -+ (id)testerWithCheese:(id)aCheese -{ - var tester = [[self alloc] init]; - [tester setCheese:aCheese]; - return tester; -} - -- (void)setCheese:(id)aCheese -{ - cheese = aCheese; -} - -- (id)cheese -{ - return cheese; -} - -@end - -@implementation DependantKeyPathsTester: CPObject -{ - ObservingTester _observingTester @accessors(property=observingTester); -} - -+ (CPSet)keyPathsForValuesAffectingObservedCheese -{ - return [CPSet setWithObjects:@"observingTester.cheese"]; -} - -+ (id)testerWithObservingTester:(ObservingTester)theObservingTester -{ - return [[self alloc] initWithObservingTester:theObservingTester]; -} - -- (id)initWithObservingTester:(ObservingTester)theObservingTester -{ - if (self = [super init]) - { - _observingTester = theObservingTester; - } - - return self; -} - -- (CPString)observedCheese -{ - return [[self observingTester] cheese]; -} - -@end diff --git a/Tests/Foundation/CPKeyValueObservingTest.j b/Tests/Foundation/CPKeyValueObservingTest.j index 5d43fc8da..2d409618f 100644 --- a/Tests/Foundation/CPKeyValueObservingTest.j +++ b/Tests/Foundation/CPKeyValueObservingTest.j @@ -22,6 +22,45 @@ return [theClass new]; } +- (void)setup +{ + _lastKeyPath = _lastObject = _lastChange = _lastContext = nil; +} + +- (void)testInitialObserving +{ + var tester = [ObservingTester testerWithCheese:@"CHEESE!"]; + [tester addObserver:self forKeyPath:@"cheese" options:CPKeyValueObservingOptionNew | CPKeyValueObservingOptionInitial context:nil]; + + [self assert:@"cheese" equals:_lastKeyPath]; + [self assert:tester equals:_lastObject]; + [self assert:[CPDictionary dictionaryWithObject:@"CHEESE!" forKey:CPKeyValueChangeNewKey] equals:_lastChange]; + [self assert:nil equals:_lastContext]; +} + +- (void)observeValueForKeyPath:(CPString)aKeyPath + ofObject:(id)anObject + change:(CPDictionary)aChange + context:(id)aContext +{ + _lastKeyPath = aKeyPath; + _lastObject = anObject; + _lastChange = aChange; + _lastContext = aContext; +} + +- (void)testSendNotificationsForDependantKeyPaths +{ + var observingTester = [ObservingTester testerWithCheese:@"cheese"], + dependantKeyPathTester = [DependantKeyPathsTester testerWithObservingTester:observingTester]; + + [dependantKeyPathTester addObserver:self forKeyPath:@"observedCheese" options:CPKeyValueObservingOptionNew context:nil]; + [observingTester setCheese:@"changed cheese"]; + + [self assert:@"observedCheese" equals:_lastKeyPath] + [self assert:dependantKeyPathTester equals:_lastObject]; +} + - (void)testOnlyInsertObject_AtKeyIndex_Implemented { var insertSelector = @selector(insertObject:inObjectsAtIndex:), @@ -155,3 +194,59 @@ } @end + +@implementation ObservingTester : CPObject +{ + id cheese; +} + ++ (id)testerWithCheese:(id)aCheese +{ + var tester = [[self alloc] init]; + [tester setCheese:aCheese]; + return tester; +} + +- (void)setCheese:(id)aCheese +{ + cheese = aCheese; +} + +- (id)cheese +{ + return cheese; +} + +@end + +@implementation DependantKeyPathsTester: CPObject +{ + ObservingTester _observingTester @accessors(property=observingTester); +} + ++ (CPSet)keyPathsForValuesAffectingObservedCheese +{ + return [CPSet setWithObjects:@"observingTester.cheese"]; +} + ++ (id)testerWithObservingTester:(ObservingTester)theObservingTester +{ + return [[self alloc] initWithObservingTester:theObservingTester]; +} + +- (id)initWithObservingTester:(ObservingTester)theObservingTester +{ + if (self = [super init]) + { + _observingTester = theObservingTester; + } + + return self; +} + +- (CPString)observedCheese +{ + return [[self observingTester] cheese]; +} + +@end From 10ec8b528c0d3c73c6a00291f342fa9406c125c3 Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Wed, 27 Apr 2011 13:14:44 -0400 Subject: [PATCH 29/29] Correct return type for [CPURL staticResourceData]. --- Foundation/CPURL.j | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Foundation/CPURL.j b/Foundation/CPURL.j index 1b60010a0..810677c7e 100644 --- a/Foundation/CPURL.j +++ b/Foundation/CPURL.j @@ -227,7 +227,7 @@ CPURLCustomIconKey = @"CPURLCustomIconKey"; return self.setResourcePropertyForKey(aKey, anObject); } -- (CPString)staticResourceData +- (CPData)staticResourceData { return self.staticResourceData(); }