[webkit-reviews] review denied: [Bug 33486] Upgrade 1.1.17->1.1.18 fails: GTK_WIDGET_TOPLEVEL' was not declared in this scope : [Attachment 46397] Fix deprecated symbols to allow webkitgtk+ 1.1.18 to compile against gtk+-2.19.3.

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied  review:
Bug 33486: Upgrade 1.1.17->1.1.18 fails:  GTK_WIDGET_TOPLEVEL' was not declared
in this scope
https://bugs.webkit.org/show_bug.cgi?id=33486

Attachment 46397: Fix deprecated symbols to allow webkitgtk+ 1.1.18 to compile
against gtk+-2.19.3.
https://bugs.webkit.org/attachment.cgi?id=46397&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
I'm going to comment about some particular issues in your code, but the overall
patch is wrong. You need to check the GTK+ version with GTK_CHECK_VERSION, and
only use your code if we are using 2.19.3 or newer. If you grep for that macro
you'll see examples of its usage. If you don't do this WebKitGTK+ wouldn't
compile with older GTK+ versions, and we don't want that.

>@@ -45,6 +45,7 @@
> #include <glib.h>
> #include <glib/gi18n-lib.h>
> #include <gtk/gtk.h>
>+#include <gtk/gtkwidget.h>

This is wrong, only the toplevel header should be included (I'm surprised this
works at all, I think GTK+ should complain).

> bool ChromeClient::canTakeFocus(FocusDirection)
> {
>-    return GTK_WIDGET_CAN_FOCUS(m_webView);
>+  return gtk_widget_get_can_focus((GtkWidget*)m_webView);

You can use GTK_WIDGET here, since it's safer and C-style casts are against the
style guidelines.