Merge pull request #2077 from mockingbird/fix-cpcheckbutton-theme-bug

Fix bug with unsetThemeState + more tests.

AppKit/CPTheme.j

@@ -358,13 +358,13 @@ ThemeState.prototype.hasThemeState = function(aState)
 {
     if (!aState || !aState._stateNames)
         return false;
-        
+
     // We can do this in O(n) because both states have their stateNames already sorted.
     for (var stateName in aState._stateNames)
     {
         if (!aState._stateNames.hasOwnProperty(stateName))
             continue;
-            
+
         if (!this._stateNames[stateName])
             return false;
     }
@@ -380,7 +380,7 @@ ThemeState.prototype.isSubsetOf = function(aState)
     {
         if (!this._stateNames.hasOwnProperty(key))
             continue;
-            
+
         if (!aState._stateNames[key])
             return false;
     }
@@ -390,7 +390,7 @@ ThemeState.prototype.isSubsetOf = function(aState)
 ThemeState.prototype.without = function(aState)
 {
     if (!aState || aState === [CPNull null])
-        return aState;
+        return this;
 
     var newStates = {};
     for (var stateName in this._stateNames)

AppKit/CPView.j

@@ -2897,7 +2897,7 @@ setBoundsOrigin:
 
 - (BOOL)setThemeState:(ThemeState)aState
 {
-    if (aState.isa && [aState isKindOfClass:CPArray])
+    if (aState && aState.isa && [aState isKindOfClass:CPArray])
         aState = CPThemeState.apply(null, aState);
 
     if (_themeState.hasThemeState(aState))
@@ -2913,14 +2913,15 @@ setBoundsOrigin:
 
 - (BOOL)unsetThemeState:(ThemeState)aState
 {
-     if (aState.isa && [aState isKindOfClass:CPArray])
+     if (aState && aState.isa && [aState isKindOfClass:CPArray])
         aState = CPThemeState.apply(null, aState);
 
-    if (!_themeState.hasThemeState(aState))
-        return NO;
-
+    var oldThemeState = _themeState
     _themeState = _themeState.without(aState);
 
+    if (oldThemeState === _themeState)
+        return NO;
+
     [self setNeedsLayout];
     [self setNeedsDisplay:YES];
 

Tests/AppKit/CPButtonTest.j

@@ -132,6 +132,16 @@
     [self assert:CPMixedState equals:[button state] message:@"Mixed state is allowed, state should be CPMixedState"];
 }
 
+- (void)testThemeStateWhenSettingObjectValue
+{
+    [button unsetThemeState:[button themeState]];
+    [button setObjectValue:CPOnState];
+    [self assert:String(CPThemeStateSelected) equals:String([button themeState]) message:@"object should be in the selected themestate"];
+
+    [button setObjectValue:CPOffState];
+    [self assert:String(CPThemeStateNormal) equals:String([button themeState]) message:@"object should be in the normal themestate"];
+}
+
 - (void)testThemeAttributes
 {
     var attributes = [CPButton themeAttributes];

Tests/AppKit/CPThemeTest.j

@@ -42,6 +42,10 @@
 
     var themeState5 = CPThemeState('a', 'theme', 'state');
     [self assertTrue:(String(themeState5) === 'a+state+theme') message:"Creating a new ThemeState using multiple strings works."];
+
+    var themeState6 = CPThemeState('a')
+    var themeState7 = CPThemeState('a', 'b')
+    [self assertTrue:(String(CPThemeState(themeState6, themeState7)) === 'a+b') message:"Creating a new ThemeState where the same state repeats itself does not result in the state being in the new themestate twice"];
 }
 
 - (void)testThemeAttributeValueForState

Tests/AppKit/CPViewTest.j

@@ -54,48 +54,70 @@
     [self assertFalse:[view hasThemeState:CPThemeState(CPThemeStateNormal)] message:@"CPView should not be in CPThemeStateNormal"];
 }
 
-- (void)testThemeState
+- (void)testSetThemeState
 {
-    [self assertTrue:[view hasThemeState:CPThemeStateNormal] message:@"CPView should initialy have CPThemeStateNormal"];
-    [self assertFalse:[view hasThemeState:CPThemeStateDisabled] message:@"view should not be disabled"];
+    [self assert:String(CPThemeStateNormal) equals:String([view themeState]) message:@"CPView should initialy have CPThemeStateNormal"];
 
     [view setThemeState:CPThemeStateDisabled];
-    [self assertTrue:[view hasThemeState:CPThemeStateDisabled] message:@"The view should be CPThemeStateDisabled"];
+    [self assert:String(CPThemeStateDisabled) equals:String([view themeState]) message:@"The view should be CPThemeStateDisabled"];
 
     [view setThemeState:CPThemeStateHighlighted];
-    [self assertTrue:[view hasThemeState:CPThemeStateHighlighted] message:@"Theme state should be CPThemeStateHighlighted"];
+    [self assert:String(CPThemeState(CPThemeStateDisabled, CPThemeStateHighlighted)) equals:String([view themeState]) message:@"Theme state should be CPThemeStateHighlighted and CPThemeStateDisabled"];
 
+    [view unsetThemeState:[view themeState]];
     [view setThemeState:CPThemeState(CPThemeStateNormal, CPThemeStateHighlighted)];
     [self assertFalse:[view hasThemeState:CPThemeStateNormal] message:@"CPThemeStateNormal cannot exist as part of a compound state"];
-    [self assertTrue:[view hasThemeState:CPThemeStateHighlighted] message:@"The view should be CPThemeStateHighlighted"];
+    [self assert:String([view themeState]) equals:String(CPThemeStateHighlighted) message:@"The view should be CPThemeStateHighlighted"];
 
     [view setThemeState:CPThemeState(CPThemeStateHighlighted, CPThemeStateDisabled)];
     [self assertTrue:[view hasThemeState:CPThemeStateHighlighted] message:@"The view should be CPThemeStateHighlighted"];
     [self assertTrue:[view hasThemeState:CPThemeStateDisabled] message:@"The view should be CPThemeStateDisabled"];
-    [self assertTrue:[view hasThemeState:CPThemeState(CPThemeStateDisabled, CPThemeStateHighlighted)] message:@"The view should be in the combined state of CPThemeStateDisabled and CPThemeStateHighlighted"];
+    [self assert:String(CPThemeState(CPThemeStateDisabled, CPThemeStateHighlighted)) equals:String([view themeState]) message:@"The view should be in the combined state of CPThemeStateDisabled and CPThemeStateHighlighted"];
 
-    [view setThemeState:[CPThemeStateHighlighted, CPThemeStateDisabled]];
-    [self assertTrue:[view hasThemeState:CPThemeState(CPThemeStateDisabled, CPThemeStateHighlighted)] message:@"setThemeState works with array argument"];
+    [view setThemeState:[CPThemeStateSelected, CPThemeStateDisabled]];
+    [self assert:String(CPThemeState(CPThemeStateDisabled, CPThemeStateSelected)) equals:String([view themeState]) message:@"setThemeState works with array argument"];
 }
 
 - (void)testUnsetThemeState
 {
-    [self assertTrue:[view hasThemeState:CPThemeStateNormal] message:@"CPView should initialy have CPThemeStateNormal"];
+    [self assert:String(CPThemeStateNormal) equals:String([view themeState]) message:@"CPView should initialy have CPThemeStateNormal"];
     [view unsetThemeState:CPThemeStateNormal];
-    [self assertTrue:[view hasThemeState:CPThemeStateNormal] message:@"CPView always be in CPThemeStateNormal even if you try to unset it"];
+    [self assert:String(CPThemeStateNormal) equals:String([view themeState]) message:@"CPView always be in CPThemeStateNormal even if you try to unset it"];
+
     [view setThemeState:CPThemeStateDisabled];
     [view unsetThemeState:CPThemeStateDisabled];
-    [self assertTrue:[view hasThemeState:CPThemeStateNormal] message:@"CPView should be in CPThemeStateNormal when all other theme states have been unset from it"];
+    [self assert:String(CPThemeStateNormal) equals:String([view themeState]) message:@"CPView should be in CPThemeStateNormal when all other theme states have been unset from it"];
+
     [view setThemeState:CPThemeState(CPThemeStateDisabled, CPThemeStateHighlighted)];
     [view unsetThemeState:CPThemeStateDisabled];
-    [self assertTrue:[view hasThemeState:CPThemeStateHighlighted] message:"@CPView should have the remaining state when one of its combined states is unset"];
+    [self assert:String(CPThemeStateHighlighted) equals:String([view themeState]) message:"@CPView should have the remaining state when one of its combined states is unset"];
+
     [view setThemeState:CPThemeState(CPThemeStateDisabled, CPThemeStateHighlighted, CPThemeStateBordered)];
     [view unsetThemeState:CPThemeState(CPThemeStateBordered, CPThemeStateHighlighted, CPThemeStateDisabled)];
-    [self assertTrue:[view hasThemeState:CPThemeStateNormal] message:@"CPView should be able to unset a combined theme state"];
+    [self assert:String(CPThemeStateNormal) equals:String([view themeState]) message:@"CPView should be able to unset a combined theme state"];
 
     [view setThemeState:CPThemeState(CPThemeStateDisabled, CPThemeStateHighlighted, CPThemeStateBordered)];
     [view unsetThemeState:[CPThemeStateBordered, CPThemeStateHighlighted]];
-    [self assertTrue:[view hasThemeState:CPThemeStateDisabled] message:@"unsetThemeState works with array argument"];
+    [self assert:String(CPThemeStateDisabled) equals:String([view themeState]) message:@"unsetThemeState works with array argument"];
+
+    [view setThemeState:CPThemeStateDisabled];
+    [view unsetThemeState:[CPThemeStateDisabled, CPThemeStateHighlighted]];
+    [self assert:String(CPThemeStateNormal) equals:String([view themeState]) message:@"CPView should be able to unset a combined theme state that has more theme states than the view currently has"];
+
+    [view setThemeState:CPThemeState(CPThemeStateDisabled, CPThemeStateBordered)];
+    var returnValue = [view unsetThemeState:[CPThemeStateDisabled, CPThemeStateHighlighted]];
+    [self assert:String(CPThemeStateBordered) equals:String([view themeState]) message:@"CPView should be able to unset a combined theme state that has not entirely overlapping themestates"];
+    [self assertTrue:returnValue message:@"When unsetThemeState successfully unsets anything, it return YES"];
+
+    [view setThemeState:CPThemeState(CPThemeStateDisabled, CPThemeStateBordered)];
+    var returnValue = [view unsetThemeState:[CPThemeStateSelected, CPThemeStateHighlighted]];
+    [self assert:String(CPThemeState(CPThemeStateDisabled, CPThemeStateBordered)) equals:String([view themeState]) message:@"CPView not unset any theme states it does not have"];
+    [self assertFalse:returnValue message:@"When unsetThemeState doesn't unset anything, it returns NO"];
+
+    [view setThemeState:CPThemeState(CPThemeStateDisabled, CPThemeStateBordered)];
+    var returnValue = [view unsetThemeState:null];
+    [self assert:String(CPThemeState(CPThemeStateDisabled, CPThemeStateBordered)) equals:String([view themeState]) message:@"Trying to unset a null themestate does not change the current themestate of the view"];
+    [self assertFalse:returnValue message:@"Trying to unset a null themestate returns false"];
 }
 
 - (void)testThemeAttributes