[webkit-reviews] review denied: [Bug 4624] WebCore needs autogenerated Obj-C DOM bindings : [Attachment 10240] better patch of DOMCore changes (ZOMG, it's even smaller)

bugzilla-request-daemon at opendarwin.org bugzilla-request-daemon at opendarwin.org
Sat Aug 26 11:55:46 PDT 2006

Timothy Hatcher <timothy at hatcher.name> has denied Timothy Hatcher
<timothy at hatcher.name>'s request for review:
Bug 4624: WebCore needs autogenerated Obj-C DOM bindings

Attachment 10240: better patch of DOMCore changes (ZOMG, it's even smaller)

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
Gret work!

Some more comments:

+// Interface that used to be in this file:
+// Now included in DOMElement.h
+//@interface DOMElement (DOMElementCSSInlineStyle)
+//- (DOMCSSStyleDeclaration *)style;
+// Now included in DOMDocument.h

We can remove this, since we include the right headers we don't need to tell
people where the interfaces moved. Same applies to DOMEvents.h,
DOMExtensions.h, DOMRange.h, DOMStylesheets.h, DOMViews.h, DOMXPath.h.

+// FIXME: this should go into DOMDocument.h
+// REASON: not in IDL yet.

This is a public header, we can't have a FIXME in there. Same applies to
DOMEvents.h, DOMExtensions.h and DOMTraversal.h.

+             or $type eq "AtomicString") {

AtomicString should not be exposed in ObjC, they should be converted to

+    # Default, assume objective-c type is a pointer with the same type name as

+    # idl type prefixed with "DOM".
+    return "DOM$type *";

Shouldn't this use GetClassName() to special case DOMImplementation?

+            # Only generate 'dealloc' and 'finalize' methods for subclasses of

Comment should say "...for direct subclasses of DOMObject." or "immediate

offsetTop and friends are still ObjCPrivate.

DOMElementExtensionsFIX should be removed or changed.  

Very close to an r+ from me!

More information about the webkit-reviews mailing list