From 33e4fea0ec1eb1255dd090f836e3c4c643757426 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 31 May 2011 00:44:16 -0700 Subject: [PATCH 1/4] Fix forwarding in Objective-J (and many infinite recursions, such as when there is no superclass): 1. Fix infinite recursion when no superclass and method isn't implemented. Closes #469. 2. Fix infinite recursion when coercing an object without description implemented into a string. Closes #1289. 3. Add -forwardingTargetForSelector: support. Reviewed by @tolmasky. --- Foundation/CPObject.j | 47 +++------------------- Objective-J/Runtime.js | 88 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 86 insertions(+), 49 deletions(-) diff --git a/Foundation/CPObject.j b/Foundation/CPObject.j index 6b3158a15..0a9e4f02c 100644 --- a/Foundation/CPObject.j +++ b/Foundation/CPObject.j @@ -329,6 +329,11 @@ CPLog(@"Got some class: %@", inst); return objj_msgSend(self, aSelector, anObject, anotherObject); } +- (id)forwardingTargetForSelector:(SEL)aSelector +{ + return nil; +} + // Forwarding Messages /*! Subclasses can override this method to forward message to @@ -341,36 +346,6 @@ CPLog(@"Got some class: %@", inst); [self doesNotRecognizeSelector:[anInvocation selector]]; } -/*! - Used for forwarding of messages to other objects. - @ignore -*/ -// FIXME: This should be moved to the runtime? -- (void)forward:(SEL)aSelector :(marg_list)args -{ - var signature = [self methodSignatureForSelector:aSelector]; - - if (signature) - { - var invocation = [CPInvocation invocationWithMethodSignature:signature]; - - [invocation setTarget:self]; - [invocation setSelector:aSelector]; - - var index = 2, - count = args.length; - - for (; index < count; ++index) - [invocation setArgument:args[index] atIndex:index]; - - [self forwardInvocation:invocation]; - - return [invocation returnValue]; - } - - [self doesNotRecognizeSelector:aSelector]; -} - // Error Handling /*! Called by the Objective-J runtime when an object can't respond to @@ -381,7 +356,7 @@ CPLog(@"Got some class: %@", inst); { [CPException raise:CPInvalidArgumentException reason: (class_isMetaClass(isa) ? "+" : "-") + " [" + [self className] + " " + aSelector + "] unrecognized selector sent to " + - (class_isMetaClass(isa) ? "class" : "instance") + " 0x" + [CPString stringWithHash:[self UID]]]; + (class_isMetaClass(isa) ? "class " + class_getName(isa) : "instance 0x" + [CPString stringWithHash:[self UID]])]; } // Archiving @@ -542,13 +517,3 @@ CPLog(@"Got some class: %@", inst); } @end - -// override toString on Objective-J objects so we get the actual description of the object -// when coerced to a string, instead of "[Object object]" -objj_class.prototype.toString = objj_object.prototype.toString = function() -{ - if (this.isa && class_getInstanceMethod(this.isa, "description") != NULL) - return [this description]; - else - return String(this) + " (-description not implemented)"; -} diff --git a/Objective-J/Runtime.js b/Objective-J/Runtime.js index 0d1145d0f..20db83f3c 100644 --- a/Objective-J/Runtime.js +++ b/Objective-J/Runtime.js @@ -71,7 +71,7 @@ GLOBAL(objj_class) = function(displayName) this.method_dtable = this.method_store.prototype; #if DEBUG - // naming the allocator allows the WebKit heap snapshot tool to display object class names correctly + // Naming the allocator allows the WebKit heap snapshot tool to display object class names correctly // HACK: displayName property is not respected so we must eval a function to name it eval("this.allocator = function " + (displayName || "OBJJ_OBJECT").replace(/\W/g, "_") + "() { }"); #else @@ -342,10 +342,61 @@ var _class_initialize = function(/*Class*/ aClass) } } -var _objj_forward = new objj_method("forward", function(self, _cmd) +var _objj_forward = function(self, _cmd) { - return objj_msgSend(self, "forward::", _cmd, arguments); -}); + var isa = self.isa, + implementation = isa.method_dtable[SEL_forwardingTargetForSelector_]; + + if (implementation) + { + var target = implementation.method_imp.call(this, self, SEL_forwardingTargetForSelector_, _cmd); + + if (target && target !== self) + { + arguments[0] = target; + + return objj_msgSend.apply(this, arguments); + } + } + + implementation = isa.method_dtable[SEL_methodSignatureForSelector_]; + + if (implementation) + { + var forwardInvocationImplementation = isa.method_dtable[SEL_forwardInvocation_]; + + if (forwardInvocationImplementation) + { + var signature = implementation.method_imp.call(this, self, SEL_methodSignatureForSelector_, _cmd); + + if (signature) + { + var invocationClass = objj_lookUpClass("CPInvocation"); + + if (invocationClass) + { + var invocation = objj_msgSend(invocationClass, SEL_invocationWithMethodSignature_, signature), + index = 0, + count = arguments.length; + + for (; index < count; ++index) + objj_msgSend(invocation, SEL_setArgument_atIndex_, arguments[index], index); + + forwardInvocationImplementation.method_imp.call(this, self, SEL_forwardInvocation_, invocation); + + return objj_msgSend(invocation, SEL_returnValue); + } + } + } + } + + implementation = isa.method_dtable[SEL_doesNotRecognizeSelector_]; + + if (implementation) + return implementation.method_imp.call(this, self, SEL_doesNotRecognizeSelector_, _cmd); + + throw class_getName(isa) + " does not implement doesNotRecognizeSelector:. Did you forget a superclass for " + class_getName(isa) + "?"; +}; // I think this forward:: may need to be a common method, instead of defined in CPObject. #define CLASS_GET_METHOD_IMPLEMENTATION(aMethodImplementation, aClass, aSelector)\ @@ -354,10 +405,7 @@ var _objj_forward = new objj_method("forward", function(self, _cmd) \ var method = aClass.method_dtable[aSelector];\ \ - if (!method)\ - method = _objj_forward;\ - \ - aMethodImplementation = method.method_imp; + aMethodImplementation = method ? method.method_imp : _objj_forward; GLOBAL(class_getMethodImplementation) = function(/*Class*/ aClass, /*SEL*/ aSelector) { @@ -657,3 +705,27 @@ GLOBAL(sel_registerName) = function(/*String*/ aName) } DISPLAY_NAME(sel_registerName); + +objj_class.prototype.toString = objj_object.prototype.toString = function() +{ + var isa = this.isa; + + if (class_getInstanceMethod(isa, SEL_description)) + return objj_msgSend(this, description); + + if (class_isMetaClass(isa)) + return this.name; + + return "[" + isa.name + " Object](-description not implemented)"; +} + +var SEL_description = sel_getUid("description"), + SEL_forwardingTargetForSelector_ = sel_getUid("forwardingTargetForSelector:"), + SEL_methodSignatureForSelector_ = sel_getUid("methodSignatureForSelector:"), + SEL_forwardInvocation_ = sel_getUid("forwardInvocation:"), + SEL_doesNotRecognizeSelector_ = sel_getUid("doesNotRecognizeSelector:"), + SEL_invocationWithMethodSignature_ = sel_getUid("invocationWithMethodSignature:"), + SEL_setTarget_ = sel_getUid("setTarget:"), + SEL_setSelector_ = sel_getUid("setSelector:"), + SEL_setArgument_atIndex_ = sel_getUid("setArgument:atIndex:"), + SEL_returnValue = sel_getUid("returnValue"); From 671666e8424541c19104ed4dccb152299b3eed58 Mon Sep 17 00:00:00 2001 From: Francisco Ryan Tolmasky I Date: Tue, 31 May 2011 01:27:41 -0700 Subject: [PATCH 2/4] Fix selector typo. Reviewed by me. --- Objective-J/Runtime.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Objective-J/Runtime.js b/Objective-J/Runtime.js index 20db83f3c..413649cad 100644 --- a/Objective-J/Runtime.js +++ b/Objective-J/Runtime.js @@ -711,7 +711,7 @@ objj_class.prototype.toString = objj_object.prototype.toString = function() var isa = this.isa; if (class_getInstanceMethod(isa, SEL_description)) - return objj_msgSend(this, description); + return objj_msgSend(this, SEL_description); if (class_isMetaClass(isa)) return this.name; From 753257a65044a2da994837a11730e9097fd927e8 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Tue, 31 May 2011 14:37:29 +0200 Subject: [PATCH 3/4] Always tile the collectionview A collectionview should always tile because it needs to adjusts it's size to properly work with an enclosing scrollview --- AppKit/CPCollectionView.j | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/AppKit/CPCollectionView.j b/AppKit/CPCollectionView.j index 99d3db162..ec57e1b69 100644 --- a/AppKit/CPCollectionView.j +++ b/AppKit/CPCollectionView.j @@ -386,7 +386,7 @@ _items = []; - if (!_itemPrototype || !_content) + if (!_itemPrototype) return; var index = 0; @@ -412,7 +412,7 @@ { var width = CGRectGetWidth([self bounds]); - if (![_content count] || width == _tileWidth) + if (width == _tileWidth) return; // We try to fit as many views per row as possible. Any remaining space is then From 16183cca0991bc6b7728b61260f363716e277263 Mon Sep 17 00:00:00 2001 From: Klaas Pieter Annema Date: Wed, 1 Jun 2011 12:09:49 +0200 Subject: [PATCH 4/4] fix nib2cibbed controls enabled state --- Tools/nib2cib/NSButton.j | 2 ++ Tools/nib2cib/NSControl.j | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Tools/nib2cib/NSButton.j b/Tools/nib2cib/NSButton.j index 1cbda554e..79e3a5572 100644 --- a/Tools/nib2cib/NSButton.j +++ b/Tools/nib2cib/NSButton.j @@ -183,6 +183,8 @@ var NSButtonIsBorderedMask = 0x00800000, [self setImagePosition:[cell imagePosition]]; } + [self setEnabled:[cell isEnabled]]; + return self; } diff --git a/Tools/nib2cib/NSControl.j b/Tools/nib2cib/NSControl.j index abe978ec9..06afe13cd 100644 --- a/Tools/nib2cib/NSControl.j +++ b/Tools/nib2cib/NSControl.j @@ -44,9 +44,10 @@ [self setFont:[cell font]]; [self setAlignment:[cell alignment]]; - // The NSEnabled flag is never changed when changing the enabled state of a control - // Enabled state should is derived from the NSCellFlags decoded by NSCell - [self setEnabled:[cell isEnabled]]; + // Enabled state is derived from the NSEnabled flag or the control's cell. + // For example NSTableView uses the NSEnabled flag, but NSButton uses it's cell isEnabled state. + // We use the NSEnabled flag here and override the behavior in controls using different logic (NSButton). + [self setEnabled:[aCoder decodeBoolForKey:@"NSEnabled"]]; [self setContinuous:[cell isContinuous]]; [self setTarget:[aCoder decodeObjectForKey:@"NSTarget"]];