[webkit-reviews] review denied: [Bug 48202] [GTK] Port to new GtkScrollable interface in GTK+ 3.x : [Attachment 71674] scrollable.diff

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Xan Lopez
<xan.lopez at gmail.com>'s request for review:
Bug 48202: [GTK] Port to new GtkScrollable interface in GTK+ 3.x
https://bugs.webkit.org/show_bug.cgi?id=48202

Attachment 71674: scrollable.diff
https://bugs.webkit.org/attachment.cgi?id=71674&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
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.