[Webkit-unassigned] [Bug 48202] [GTK] Port to new GtkScrollable interface in GTK+ 3.x

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #71674|review?                     |review-
               Flag|                            |




--- Comment #5 from Martin Robinson <mrobinson at webkit.org>  2010-10-24 22:17:27 PST ---
(From update of attachment 71674)
View in context: https://bugs.webkit.org/attachment.cgi?id=71674&action=review

Looks good. I have a couple nits.

> WebKit/gtk/webkit/webkitwebview.cpp:202
> +#else
> +    PROP_VIEW_MODE,

I believe it's fine to have the extra comma on PROP_VIEW_MODE for the GTK+ 2.0 version. That would simplify the enum and #ifdef.

> WebKit/gtk/webkit/webkitwebview.cpp:214
> +                        G_IMPLEMENT_INTERFACE(GTK_TYPE_SCROLLABLE, NULL))

Should this NULL be a 0?

> WebKit/gtk/webkit/webkitwebview.cpp:400
> +static void webkit_web_view_set_hadjustment(WebKitWebView* webView, GtkAdjustment* hadj)

IMO if these setters aren't public they should be named in WebKit style. I can see naming them this way for consistency though. I definitely think 'hadj' should be 'horizontalAdjustment' though. These comments apply for webkit_web_view_set_vadjustment as well.

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