[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]

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
http://bugzilla.opendarwin.org/show_bug.cgi?id=4624

Attachment 10240: better patch of DOMCore changes (ZOMG, it's even smaller)
http://bugzilla.opendarwin.org/attachment.cgi?id=10240&action=edit

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

Some more comments:

DOMCSS.h:
+// Interface that used to be in this file:
+//
+
+// Now included in DOMElement.h
+//@interface DOMElement (DOMElementCSSInlineStyle)
+//- (DOMCSSStyleDeclaration *)style;
+//@end
+
+// 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.

DOMCSS.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
NSString.

+    # 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
DOMObject.

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

offsetTop and friends are still ObjCPrivate.

DOMElementExtensionsFIX should be removed or changed.  

Very close to an r+ from me!