Bug 8389 – support for Cocoa bindings - binding an NSTreeController to the WebView's DOM

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[James G. Speth]

I have some small changes that make it possible to bind an NSTreeController to the DOM of a WebView, adding some additional support for Cocoa bindings to WebKit.  Basically, there are a couple of places in WebKit that needed to send out KVO messages, and in JavaScriptCore I made a category that adds the array accessors (count/objectAtIndex:) to WebScriptObject.

I also added a controller class, WebController, that is a subclass of NSTreeController that has a new outlet/binding for the WebView.  It is a very minimal controller that could use some more work, but it does enough to make it possible to connect things up in Interface Builder.

I'll also attach the Interface Builder palette and a sample NIB file for using the bindings.

I'd be happy to make changes to this, as there are several areas I can see that might need to be modified.  I wanted to put this here for review, instead of going too far on my own.

[James G. Speth]

Created attachment 7709 [details]
patch for additional support of cocoa bindings in webkit

[James G. Speth]

Created attachment 7711 [details]
Interface Builder palette for controller and example NIB file

after building with the patch, you should run Interface Builder with the built frameworks.  next, you open the palette in Interface Builder, then the simple.nib file after that.  you can test it by choosing "Test Interface" from the File menu.

[James G. Speth]

Created attachment 7712 [details]
screenshot of example nib file in action

[Eric Seidel (no email)]

Comment on attachment 7709 [details]
patch for additional support of cocoa bindings in webkit

The patch looks fine.  

I'm a bit surprised by this:

+@implementation WebUndefined (WebKitCocoaBindings)
+
+- (NSString *)description
+{
+    return @"";
+}

but honestly I don't know what WebUndefined does.  I would have expected that to return either "nil" or @"undefined".

[Timothy Hatcher]

Comment on attachment 7709 [details]
patch for additional support of cocoa bindings in webkit

Looks good, here are few things that should be cleaned up.

+@interface WebUndefined (WebKitCocoaBindings)
+
+- (NSString *)description;
+
+@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?

[Timothy Hatcher]

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

Also our ObjC coding style does not include a space after colons in the selector.

[Timothy Hatcher]

We should consider adding did/will change binding updates for nodes by hooking listening for DOM mutation events. This would let the outline view be a live tree representation for example if some JavaScript adds or deletes nodes on the fly.

[James G. Speth]

Created attachment 7720 [details]
new patch for review

i snuck one new thing in here which is that WebScriptObject valueForKey: now calls super if the javascript returns undefined.  this is actually what i really wanted with that WebUndefined description business, but i was wimping out for some reason.

if super valueForKey: fails it will call valueForUndefinedKey, which is important because it causes the right behavior to happen with bindings using the "Raises for Not Applicable Keys" flag and the Not Applicable Placeholder setting.

this lets you set an empty string as the placeholder, and get rid of all the "undefined"s in the outline view for nodes that don't have innerHTML (for instance).

[Timothy Hatcher]

Comment on attachment 7720 [details]
new patch for review

Looks great! Thanks for the code comments.

+    if ([length respondsToSelector:@selector(intValue)])
+        return [length intValue];
+    else
+        return 0;

One minor style thing for the future; no need for "else" here.

[Timothy Hatcher]

Landed in r13877.

[Mark Rowe (bdash)]

The attached IB palette does not open for me, with IB logging a message to the console saying: 
Interface Builder Tiger[90541:d03] -[__IBDocument readFromFile:/Volumes/Data/Downloads/palette/WebController.palette/DP.nib ofType:Interface Builder Palette] failed with exception "*** -[NSKeyedUnarchiver decodeObjectForKey:]: cannot decode object of class (WebController)"

What do I need to do to make this work?

[Alexey Proskuryakov]

Mark, will the palette work for you if you run IB under Rosetta?