From 20ae47971c88f24940091783c35d60bfa119accb Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Tue, 6 Dec 2011 22:43:00 +0000 Subject: [PATCH 1/7] Missing semicolons. --- Objective-J/CFDictionary.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/Objective-J/CFDictionary.js b/Objective-J/CFDictionary.js index 899ea595e..99fd02fb4 100644 --- a/Objective-J/CFDictionary.js +++ b/Objective-J/CFDictionary.js @@ -35,7 +35,7 @@ CFDictionary.prototype.copy = function() { // Immutable, so no need to actually copy. return this; -} +}; CFDictionary.prototype.mutableCopy = function() { @@ -58,12 +58,12 @@ CFDictionary.prototype.mutableCopy = function() } return newDictionary; -} +}; CFDictionary.prototype.containsKey = function(/*String*/ aKey) { return hasOwnProperty.apply(this._buckets, [aKey]); -} +}; DISPLAY_NAME(CFDictionary.prototype.containsKey); @@ -79,21 +79,21 @@ CFDictionary.prototype.containsValue = function(/*id*/ anObject) return YES; return NO; -} +}; DISPLAY_NAME(CFDictionary.prototype.containsValue); CFDictionary.prototype.count = function() { return this._count; -} +}; DISPLAY_NAME(CFDictionary.prototype.count); CFDictionary.prototype.countOfKey = function(/*String*/ aKey) { return this.containsKey(aKey) ? 1 : 0; -} +}; DISPLAY_NAME(CFDictionary.prototype.countOfKey); @@ -110,14 +110,14 @@ CFDictionary.prototype.countOfValue = function(/*id*/ anObject) ++countOfValue; return countOfValue; -} +}; DISPLAY_NAME(CFDictionary.prototype.countOfValue); CFDictionary.prototype.keys = function() { return this._keys.slice(); -} +}; DISPLAY_NAME(CFDictionary.prototype.keys); @@ -129,7 +129,7 @@ CFDictionary.prototype.valueForKey = function(/*String*/ aKey) return nil; return buckets[aKey]; -} +}; DISPLAY_NAME(CFDictionary.prototype.valueForKey); @@ -148,7 +148,7 @@ CFDictionary.prototype.toString = function() } return string + "}"; -} +}; DISPLAY_NAME(CFDictionary.prototype.toString); @@ -162,7 +162,7 @@ CFMutableDictionary.prototype = new CFDictionary(); CFMutableDictionary.prototype.copy = function() { return this.mutableCopy(); -} +}; CFMutableDictionary.prototype.addValueForKey = function(/*String*/ aKey, /*Object*/ aValue) { @@ -173,7 +173,7 @@ CFMutableDictionary.prototype.addValueForKey = function(/*String*/ aKey, /*Objec this._keys.push(aKey); this._buckets[aKey] = aValue; -} +}; DISPLAY_NAME(CFMutableDictionary.prototype.addValueForKey); @@ -204,7 +204,7 @@ CFMutableDictionary.prototype.removeValueForKey = function(/*String*/ aKey) this._keys.splice(indexOfKey, 1); delete this._buckets[aKey]; -} +}; DISPLAY_NAME(CFMutableDictionary.prototype.removeValueForKey); @@ -213,7 +213,7 @@ CFMutableDictionary.prototype.removeAllValues = function() this._count = 0; this._keys = []; this._buckets = { }; -} +}; DISPLAY_NAME(CFMutableDictionary.prototype.removeAllValues); @@ -223,7 +223,7 @@ CFMutableDictionary.prototype.replaceValueForKey = function(/*String*/ aKey, /*O return; this._buckets[aKey] = aValue; -} +}; DISPLAY_NAME(CFMutableDictionary.prototype.replaceValueForKey); @@ -237,6 +237,6 @@ CFMutableDictionary.prototype.setValueForKey = function(/*String*/ aKey, /*Objec else this.addValueForKey(aKey, aValue); -} +}; DISPLAY_NAME(CFMutableDictionary.prototype.setValueForKey); From 107103bcbb7ee15fde033fdaf68d45d66d904f8b Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Tue, 6 Dec 2011 22:45:26 +0000 Subject: [PATCH 2/7] Fixes #1413: Internet Explorer colorWithCSSString crash. Syntax such as [[CPColor colorWithCSSString:@"rgba(0,0,0,0.5)"] cssString] lead to a CSS string incompatible with browsers without the CSS rgba feature. --- AppKit/CPColor.j | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/AppKit/CPColor.j b/AppKit/CPColor.j index ca9e7996a..88bdbcffe 100644 --- a/AppKit/CPColor.j +++ b/AppKit/CPColor.j @@ -480,7 +480,9 @@ function CPColorWithImages() parts[3] ? parseFloat(parts[3], 10) : 1.0 ]; - _cssString = aString; + // We can't reuse aString as _cssString because the browser might not support the `rgba` syntax, and aString might + // use it (issue #1413.) + [self _initCSSStringFromComponents]; return self; } @@ -494,18 +496,23 @@ function CPColorWithImages() { _components = components; - var hasAlpha = CPFeatureIsCompatible(CPCSSRGBAFeature) && _components[3] != 1.0; - - _cssString = (hasAlpha ? "rgba(" : "rgb(") + - parseInt(_components[0] * 255.0) + ", " + - parseInt(_components[1] * 255.0) + ", " + - parseInt(_components[2] * 255.0) + - (hasAlpha ? (", " + _components[3]) : "") + ")"; + [self _initCSSStringFromComponents]; } return self; } +- (void)_initCSSStringFromComponents +{ + var hasAlpha = CPFeatureIsCompatible(CPCSSRGBAFeature) && _components[3] != 1.0; + + _cssString = (hasAlpha ? "rgba(" : "rgb(") + + parseInt(_components[0] * 255.0) + ", " + + parseInt(_components[1] * 255.0) + ", " + + parseInt(_components[2] * 255.0) + + (hasAlpha ? (", " + _components[3]) : "") + ")"; +} + /* @ignore */ - (id)_initWithPatternImage:(CPImage)anImage { From 9367566e61f171c5240d018d68132dd3230fa9ef Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Tue, 6 Dec 2011 23:01:26 +0000 Subject: [PATCH 3/7] Fixes #1412. IE rendered clearColor shadows in black. --- AppKit/_CPImageAndTextView.j | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/AppKit/_CPImageAndTextView.j b/AppKit/_CPImageAndTextView.j index 275b0e051..49190f27d 100644 --- a/AppKit/_CPImageAndTextView.j +++ b/AppKit/_CPImageAndTextView.j @@ -390,7 +390,10 @@ var _CPimageAndTextViewFrameSizeChangedFlag = 1 << 0, var textStyle = hasDOMTextElement ? _DOMTextElement.style : nil; // Create or destroy the DOM Text Shadow element as necessary. - var needsDOMTextShadowElement = hasDOMTextElement && !!_textShadowColor, + // If _textShadowColor's alphaComponent is 0, don't bother drawing anything (issue #1412). + // This improves performance as we get rid of an invisible element, and makes IE <9.0 capable + // of correctly 'rendering' shadows with [CPColor clearColor]. + var needsDOMTextShadowElement = hasDOMTextElement && [_textShadowColor alphaComponent] > 0.0, hasDOMTextShadowElement = !!_DOMTextShadowElement; if (needsDOMTextShadowElement !== hasDOMTextShadowElement) From 052c07f3e380874eead1e35ff579aa96d014d5a6 Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Tue, 6 Dec 2011 23:26:13 +0000 Subject: [PATCH 4/7] Refs #1412, #1413. Manual test for the last two fixes. --- Tests/Manual/CPTextField/AppController.j | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Tests/Manual/CPTextField/AppController.j b/Tests/Manual/CPTextField/AppController.j index d30fafd22..1e5639509 100644 --- a/Tests/Manual/CPTextField/AppController.j +++ b/Tests/Manual/CPTextField/AppController.j @@ -35,6 +35,23 @@ [contentView addSubview:textField]; + var shadowLabel = [CPTextField labelWithTitle:@"This text should have a shadow."], + championOfLightLabel = [CPTextField labelWithTitle:@"This text should have no shadow."]; + + [shadowLabel setTextColor:[CPColor blackColor]]; + [shadowLabel setTextShadowOffset:CGSizeMake(0, 1)]; + [shadowLabel setTextShadowColor:[CPColor colorWithCSSString:@"rgba(0, 0, 0, 0.5)"]]; + + [championOfLightLabel setTextColor:[CPColor blackColor]]; + [championOfLightLabel setTextShadowOffset:CGSizeMake(0, 1)]; + [championOfLightLabel setTextShadowColor:[CPColor clearColor]]; + + [shadowLabel setFrame:CGRectMake(15, CGRectGetMaxY([textField frame]) + 10, 300, 18)]; + [championOfLightLabel setFrame:CGRectMake(15, CGRectGetMaxY([shadowLabel frame]) + 2, 300, 18)]; + + [contentView addSubview:shadowLabel]; + [contentView addSubview:championOfLightLabel]; + [theWindow orderFront:self]; aWindow = [[CPWindow alloc] initWithContentRect:CGRectMake(150, 300, 400, 150) styleMask:CPTitledWindowMask | CPClosableWindowMask | CPDocModalWindowMask]; From a5854f129d7afc6b835a336820e0178c2f22b9d6 Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Tue, 6 Dec 2011 23:29:57 +0000 Subject: [PATCH 5/7] Fixes #1411. In IE, no table/outline view selection highlights would render for certain CIBs. This fix also enables `setSelectionHighlightStyle: CPTableViewSelectionHighlightStyleSourceList` in Internet Explorer. Even that the gradient effect can't be rendered and the fallback regular rendering will be used, the proper field value might still be useful for subclasses or saving out to CIBs etc. --- AppKit/CPTableView.j | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/AppKit/CPTableView.j b/AppKit/CPTableView.j index 025a6cccd..cb28fb249 100644 --- a/AppKit/CPTableView.j +++ b/AppKit/CPTableView.j @@ -768,10 +768,6 @@ NOT YET IMPLEMENTED */ - (void)setSelectionHighlightStyle:(unsigned)aSelectionHighlightStyle { - //early return for IE. - if (aSelectionHighlightStyle == CPTableViewSelectionHighlightStyleSourceList && !CPFeatureIsCompatible(CPHTMLCanvasFeature)) - return; - _selectionHighlightStyle = aSelectionHighlightStyle; [self setNeedsDisplay:YES]; @@ -3690,7 +3686,7 @@ Your delegate can implement this method to avoid subclassing the tableview to ad if (!count) return; - var drawGradient = (_selectionHighlightStyle === CPTableViewSelectionHighlightStyleSourceList && [_selectedRowIndexes count] >= 1), + var drawGradient = (CPFeatureIsCompatible(CPHTMLCanvasFeature) && _selectionHighlightStyle === CPTableViewSelectionHighlightStyleSourceList && [_selectedRowIndexes count] >= 1), deltaHeight = 0.5 * (_gridStyleMask & CPTableViewSolidHorizontalGridLineMask); CGContextBeginPath(context); @@ -3817,7 +3813,7 @@ Your delegate can implement this method to avoid subclassing the tableview to ad */ - (void)_drawGroupRowsForRects:(CPArray)rects { - if (_selectionHighlightStyle === CPTableViewSelectionHighlightStyleSourceList || !rects.length) + if ((CPFeatureIsCompatible(CPHTMLCanvasFeature) && _selectionHighlightStyle === CPTableViewSelectionHighlightStyleSourceList) || !rects.length) return; var context = [[CPGraphicsContext currentContext] graphicsPort], From 24326883ca716dab40ad5621c1babf57d5c3a701 Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Wed, 7 Dec 2011 01:38:41 +0000 Subject: [PATCH 6/7] More extensive Notification Center tests. --- Tests/Foundation/CPNotificationCenterTest.j | 83 ++++++++++++++++++++- 1 file changed, 81 insertions(+), 2 deletions(-) diff --git a/Tests/Foundation/CPNotificationCenterTest.j b/Tests/Foundation/CPNotificationCenterTest.j index b342cf84b..b1f3b8bf5 100644 --- a/Tests/Foundation/CPNotificationCenterTest.j +++ b/Tests/Foundation/CPNotificationCenterTest.j @@ -1,14 +1,77 @@ @import +var TestNotification = @"TestNotification"; + @implementation CPNotificationCenterTest : OJTestCase { int notificationCount; } +/*! + Test various filter parameters. +*/ +- (void)testNotify +{ + var center = [CPNotificationCenter defaultCenter]; + + notificationCount = 0; + + [center addObserver:self selector:@selector(countNotification:) name:TestNotification object:2]; + [center addObserver:self selector:@selector(countNotification:) name:TestNotification object:25]; + [center postNotificationName:TestNotification object:self]; + [self assert:0 equals:notificationCount message:@"observer should only be notified for object '2'"]; + + [center postNotificationName:TestNotification object:2]; + [self assert:1 equals:notificationCount message:@"observer should be notified for object '2'"]; + + [center addObserver:self selector:@selector(countNotification:) name:TestNotification object:nil]; + [center postNotificationName:TestNotification object:2]; + [self assert:3 equals:notificationCount message:@"observer should be notified for object '2' and for any object"]; + + [center removeObserver:self name:TestNotification object:2]; + [center postNotificationName:TestNotification object:2]; + [self assert:4 equals:notificationCount message:@"observer should be notified only for any object"]; + + // At this point we have TestNofication:nil observer and a TestNotification:25 observer. + [center addObserver:self selector:@selector(countNotification:) name:nil object:2]; + [center postNotificationName:TestNotification object:2]; + [self assert:6 equals:notificationCount message:@"observer should be notified for TestNofication and for object '2' (TestNotification)"]; + [center postNotificationName:@"RandomNotification" object:2]; + [self assert:7 equals:notificationCount message:@"observer should be notified for object '2' (RandomNotification)"]; + + [center removeObserver:self name:TestNotification object:nil]; + [center postNotificationName:TestNotification object:nil]; + [center postNotificationName:TestNotification object:2]; + [center postNotificationName:TestNotification object:25]; + [self assert:8 equals:notificationCount message:@"observer should be notified only for object '2'"]; + + [center removeObserver:self]; + [center postNotificationName:TestNotification object:nil]; + [center postNotificationName:TestNotification object:2]; + [center postNotificationName:TestNotification object:25]; + [self assert:8 equals:notificationCount message:@"observer should not be notified"]; +} + +- (void)testAddObserversDuringNotification +{ + var center = [CPNotificationCenter defaultCenter]; + + notificationCount = 0; + + [center addObserver:self selector:@selector(addObserversNotification:) name:TestNotification object:nil]; + + [center postNotificationName:TestNotification object:self]; + + [self assert:1 equals:notificationCount message:@"the new observers in addObserversNotification: should not be notified"]; + + [center postNotificationName:TestNotification object:self]; + + [self assert:5 equals:notificationCount message:@"the new observers from the first addObserversNotification should now be active"]; +} + - (void)testRemoveObserversDuringNotification { - var center = [CPNotificationCenter defaultCenter], - TestNotification = @"TestNotification"; + var center = [CPNotificationCenter defaultCenter]; notificationCount = 0; @@ -29,4 +92,20 @@ [center removeObserver:self]; } +- (void)addObserversNotification:(CPNotification)aNotification +{ + notificationCount += 1; + + var center = [CPNotificationCenter defaultCenter]; + // These should not be notified. + [center addObserver:self selector:@selector(countNotification:) name:TestNotification object:nil]; + [center addObserver:self selector:@selector(countNotification:) name:TestNotification object:nil]; + [center addObserver:self selector:@selector(countNotification:) name:TestNotification object:nil]; +} + +- (void)countNotification:(CPNotification)aNotification +{ + notificationCount += 1; +} + @end From 3dff67e0b1359421dae0de37a87d68a2fe0b1804 Mon Sep 17 00:00:00 2001 From: Alexander Ljungberg Date: Wed, 7 Dec 2011 01:48:16 +0000 Subject: [PATCH 7/7] Faster CPNotificationCenter removeObserver. Removing observers is now faster, significantly so when there are very many observers and one or more is removed while a notification is being posted. --- Foundation/CPNotificationCenter.j | 71 ++++++++++++------------------- 1 file changed, 27 insertions(+), 44 deletions(-) diff --git a/Foundation/CPNotificationCenter.j b/Foundation/CPNotificationCenter.j index 981573eea..da0b71605 100644 --- a/Foundation/CPNotificationCenter.j +++ b/Foundation/CPNotificationCenter.j @@ -85,7 +85,6 @@ var CPNotificationDefaultCenter = nil; if (aNotificationName == nil) registry = _unnamedRegistry; - else if (!(registry = [_namedRegistries objectForKey:aNotificationName])) { registry = [[_CPNotificationRegistry alloc] init]; @@ -181,7 +180,6 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */ @implementation _CPNotificationRegistry : CPObject { CPDictionary _objectObservers; - BOOL _observerRemovalCount; } - (id)init @@ -190,7 +188,6 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */ if (self) { - _observerRemovalCount = 0; _objectObservers = [CPDictionary dictionary]; } @@ -209,12 +206,12 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */ if (!observers) { - observers = []; + observers = [CPMutableSet set]; [_objectObservers setObject:observers forKey:[anObject UID]]; } // Add this observer. - observers.push(anObserver); + [observers addObject:anObserver]; } - (void)removeObserver:(id)anObserver object:(id)anObject @@ -231,16 +228,14 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */ while (key = [keys nextObject]) { var observers = [_objectObservers objectForKey:key], - count = observers ? observers.length : 0; + observer = nil, + observersEnumerator = [observers objectEnumerator]; - while (count--) - if ([observers[count] observer] == anObserver) - { - ++_observerRemovalCount; - observers.splice(count, 1); - } + while ((observer = [observersEnumerator nextObject]) !== nil) + if ([observer observer] == anObserver) + [observers removeObject:observer]; - if (!observers || observers.length == 0) + if (![observers count]) removedKeys.push(key); } } @@ -248,16 +243,14 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */ { var key = [anObject UID], observers = [_objectObservers objectForKey:key], - count = observers ? observers.length : 0; + observer = nil, + observersEnumerator = [observers objectEnumerator]; - while (count--) - if ([observers[count] observer] == anObserver) - { - ++_observerRemovalCount; - observers.splice(count, 1) - } + while ((observer = [observersEnumerator nextObject]) !== nil) + if ([observer observer] == anObserver) + [observers removeObject:observer]; - if (!observers || observers.length == 0) + if (![observers count]) removedKeys.push(key); } @@ -273,26 +266,20 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */ // during the posting of this notification, nor observers that get added. The // best way to do this is to make a copy of the current observers (this avoids // new observers from being notified) and double checking every observer against - // the current array (this avoids removed observers from receiving notifications). - // However, this is a very expensive operation (O(N) => O(N^2)), so to avoid it, - // we keep track of whether observers are added or removed, and only do our - // rigorous testing in those cases. - var observerRemovalCount = _observerRemovalCount, - object = [aNotification object], - observers = nil; + // the current set (this avoids removed observers from receiving notifications). + var object = [aNotification object], + currentObservers = nil; if (object != nil && (currentObservers = [_objectObservers objectForKey:[object UID]])) { var observers = [currentObservers copy], - count = observers.length; + observer = nil, + observersEnumerator = [observers objectEnumerator]; - while (count--) + while ((observer = [observersEnumerator nextObject]) !== nil) { - var observer = observers[count]; - - // if there wasn't removal of an observer during this posting, or there - // was but we are still in the observer list... - if ((observerRemovalCount === _observerRemovalCount) || [currentObservers indexOfObjectIdenticalTo:observer] !== CPNotFound) + // CPSet containsObject is N(1) so this is a fast check. + if ([currentObservers containsObject:observer]) [observer postNotification:aNotification]; } } @@ -303,17 +290,13 @@ var _CPNotificationCenterPostNotification = function(/* CPNotificationCenter */ if (!currentObservers) return; - var observerRemovalCount = _observerRemovalCount, - observers = [currentObservers copy], - count = observers.length; + var observers = [currentObservers copy], + observersEnumerator = [observers objectEnumerator]; - while (count--) + while ((observer = [observersEnumerator nextObject]) !== nil) { - var observer = observers[count]; - - // if there wasn't removal of an observer during this posting, or there - // was but we are still in the observer list... - if ((observerRemovalCount === _observerRemovalCount) || [currentObservers indexOfObjectIdenticalTo:observer] !== CPNotFound) + // CPSet containsObject is N(1) so this is a fast check. + if ([currentObservers containsObject:observer]) [observer postNotification:aNotification]; } }