[Webkit-unassigned] [Bug 8389] support for Cocoa bindings - binding an NSTreeController to the WebView's DOM

[bugzilla-daemon at opendarwin.org]

http://bugzilla.opendarwin.org/show_bug.cgi?id=8389


timothy at hatcher.name changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #7709|review?                     |review-
               Flag|                            |




------- Comment #5 from timothy at hatcher.name  2006-04-14 13:30 PDT -------
(From update of attachment 7709)
Looks good, here are few things that should be cleaned up.

+ at interface WebUndefined (WebKitCocoaBindings)
+
+- (NSString *)description;
+
+ at end

No need for this interface, NSObject exposes description. I agree with Eric,
the WebUndefined's description should be "undefined". No need for this to be in
a category, just add description to the main implementation of WebUndefined.

+- (unsigned int)count
+{
+    return [[self valueForKey:@"length"] intValue];
+}

You should not assume "length" will always have a intValue method. You should
check for isKindOfClass: NSString or NSNumber, or respondsToSelector:.

+- (DOMDocument *)mainFrameDOM;

This should be called "mainFrameDocument".

+    [self setChildrenKeyPath:@"none"];

Should that be [self setChildrenKeyPath:nil]?

+- (void)setContent:(id)newContent
+{
+    id content = [[self content] retain];
+    NSArray *paths = [[self selectionIndexPaths] retain];
+    NSString *childrenKeyPath = [[self childrenKeyPath] retain];
+    
+    [self setSelectionIndexPaths:nil];
+    [super setContent:nil];
+    [self setChildrenKeyPath:@"none"];
+    
+    [super setContent:newContent];
+    [self setChildrenKeyPath:childrenKeyPath];
+    [self setSelectionIndexPaths:paths];
+    
+    [content release];
+    [paths release];
+    [childrenKeyPath release];
+}

What is the reason we can't just use super's setContent:?

+    [webView release];
+    webView = [newWebView retain];

I recommend you do this like follows. 

+    id old = webView;
+    webView = [newWebView retain];
+    [old release];


+    [wv _didChangeValueForKey: _WebMainFrameDOMKey];
+    [wv _willChangeValueForKey: _WebMainFrameDOMKey];

Should these be called in the reverse order?


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.