[webkit-reviews] review denied: [Bug 14806] [gtk] [patch] Implement
can_go_backward and can_go_forward in webkitgtkpage.cpp :
[Attachment 15726] Implements the wrappers for
can_go_forward|backward
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Jul 29 14:31:02 PDT 2007
Adam Roben <aroben at apple.com> has denied 's request for review:
Bug 14806: [gtk] [patch] Implement can_go_backward and can_go_forward in
webkitgtkpage.cpp
http://bugs.webkit.org/show_bug.cgi?id=14806
Attachment 15726: Implements the wrappers for can_go_forward|backward
http://bugs.webkit.org/attachment.cgi?id=15726&action=edit
------- Additional Comments from Adam Roben <aroben at apple.com>
The implementation looks good. Just a few comments to bring this patch in line
with <http://webkit.org/coding/coding-style.html>
}
+gboolean webkit_gtk_page_can_go_backward (WebKitGtkPage* page)
+{
There should be an empty line before/after all functions. Please remove the
space before the open parenthesis.
+ WebKitGtkPagePrivate* page_data = WEBKIT_GTK_PAGE_GET_PRIVATE(page);
+ WebKitGtkFramePrivate* frame_data =
WEBKIT_GTK_FRAME_GET_PRIVATE(page_data->main_frame);
After talking with Holger, I think we're going to move towards maintaining the
WebKit camelCase style for variable names within the WebKit/gtk implementation
files. So these variables should be called pageData and frameData. I know it's
inconsistent with the rest of the file, but that will be cleaned up later.
You also need a ChangeLog entry to go along with your patch. See
<http://webkit.org/coding/contributing.html> for instructions on how to
generate one.
Once these are addressed, I think we can get this landed!
More information about the webkit-reviews
mailing list