[Webkit-unassigned] [Bug 16401] [GTK] GObject/C DOM binding

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=16401





------- Comment #181 from xan.lopez at gmail.com  2009-03-05 04:18 PDT -------
(In reply to comment #179)
> Created an attachment (id=27762)
 --> (https://bugs.webkit.org/attachment.cgi?id=27762&action=view) [review]

Just a few more comments:

 #include "ScriptController.h"
+#include "TextDocument.h"
 #include "SubstituteData.h"

Would be good to keep this in alphabetical order.

 /**
+ * webkit_web_frame_get_xml_http_request:
+ * @frame: a #WebKitWebFrame
+ *
+ * returns a Gdom wrapper around an XMLHttpRequest
+ *
+ * Return value: a GdomXMLHttpRequest
+ */

You need to add "Since: 1.1.2" to all new public functions doc string. (If we
don't make it for 1.1.2, we'll change it to 1.1.3, and so on).

+    PassRefPtr<DOMWindow> win = coreFrame->domWindow();
+    return GDOM_DOM_WINDOW(WebKit::toGDOM(win.get()));

What's the point of using a PassRefPtr here? I think it adds unnecessary
refcount churning, and that this is enough:

return GDOM_DOM_WINDOW(WebKit::toGDOM(coreFrame->domWindow()));

+WEBKIT_API GdomXMLHttpRequest *
+webkit_web_frame_get_xml_http_request       (WebKitWebFrame       *frame);
+
+WEBKIT_API GdomDOMWindow *
+webkit_web_frame_get_dom_window       (WebKitWebFrame       *frame);

We try to keep this aligned in headers.

+2009-02-18  Luke Kenneth Casson Leighton <lkcl at lkcl.net>
+
+        Implemented GTK DOM binding.
+
+        * gtk/webkit/webkitwebframe.cpp:
+        * gtk/webkit/webkitwebframe.h:

Here add the bug URL and title too (and same in all changelogs).

webcore_cppflags += \
+    -I$(srcdir)/WebKit/gtk \
+    -I$(srcdir)/WebKit/gtk/WebCoreSupport \
+    -I$(srcdir)/WebKit/gtk/webkit \
        -I$(srcdir)/WebCore \
+       -I$(srcdir)/WebCore/bindings/gdom \
        -I$(srcdir)/WebCore/bindings/js \

We could fix indentation here while we are at it :)

+#include "gdom/GdomHTMLElementPrivate.h"
+#include "gdom/GdomHTMLAnchorElementPrivate.h"

Alphabetical order again.

+gpointer GDOMObjectCache::getDOMObject(void* objectHandle)
+{
+    gpointer ret = domObjects().get(objectHandle);
+    return ret;
+}

No need for variable.


Also, I agree the prefix WebKit seems to make the most sense here, so +1 from
me on that.


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