Bug 14806

Summary: [gtk] [patch] Implement can_go_backward and can_go_forward in webkitgtkpage.cpp
Product: WebKit Reporter: Diego Escalante Urrelo <diegoe>
Component: WebKit APIAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: diegoe
Priority: P2 Keywords: Gtk
Version: 523.x (Safari 3)   
Hardware: PC   
OS: Linux   
Description Flags
Implements the wrappers for can_go_forward|backward
aroben: review-
Updated patch, corrects style aroben: review+

Description Diego Escalante Urrelo 2007-07-29 14:10:11 PDT
Hacking on Epiphany's webkit backend I discovered that webkitgtkpage.h announces the webkit_gtk_page_can_go_backward and webkit_gtk_page_can_go_forward functions but they are not implemented. I just dived into the code and added the corresponding wrappers. Patch attached.
Comment 1 Diego Escalante Urrelo 2007-07-29 14:11:22 PDT
Created attachment 15726 [details]
Implements the wrappers for can_go_forward|backward

Pretty simple.
Comment 2 Adam Roben (:aroben) 2007-07-29 14:31:02 PDT
Comment on attachment 15726 [details]
Implements the wrappers for can_go_forward|backward

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!
Comment 3 Diego Escalante Urrelo 2007-07-29 18:45:34 PDT
Created attachment 15735 [details]
Updated patch, corrects style

This should be enough!
Comment 4 Adam Roben (:aroben) 2007-07-29 18:50:16 PDT
Comment on attachment 15735 [details]
Updated patch, corrects style


Thanks for the cleanup!
Comment 5 Diego Escalante Urrelo 2007-07-29 19:05:12 PDT
Comment on attachment 15735 [details]
Updated patch, corrects style

>Index: gtk/Api/webkitgtkpage.cpp
>--- gtk/Api/webkitgtkpage.cpp	(revision 24766)
>+++ gtk/Api/webkitgtkpage.cpp	(working copy)
>@@ -328,6 +328,20 @@ void webkit_gtk_page_go_forward(WebKitGt
>     frame_data->frame->loader()->goBackOrForward(1);
> }
>+gboolean webkit_gtk_page_can_go_backward(WebKitGtkPage* page)
>+    WebKitGtkPagePrivate* pageData = WEBKIT_GTK_PAGE_GET_PRIVATE(page);
>+    WebKitGtkFramePrivate* frameData = WEBKIT_GTK_FRAME_GET_PRIVATE(page_data->main_frame);
Oops, should be pageData.

>+    return frame_data->frame->loader()->canGoBackOrForward(-1);
Oops, should be frameData

Same in the other function.