Bug 33486 – Upgrade 1.1.17->1.1.18 fails: GTK_WIDGET_TOPLEVEL' was not declared in this scope

• Component: New Bugs
• Status: RESOLVED FIXED

[David Ronis]

I'm trying to upgrade as mentioned in the summary.   The build fails with errors like:

GTK_WIDGET_TOPLEVEL' was not declared in this scope
WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp: In member function 'virtual void WebKit::ChromeClient::unfocus()':
WebKit/gtk/WebCoreSupport/ChromeClientGtk.cpp:109: error: 'GTK_WIDGET_TOPLEVEL' was not declared in this scope

I'm running with gtk+-2.19.3 (1.1.17 was built against 2.19.2).

[David Ronis]

The problem is gtk+ related.  I tried rebuilding 1.1.17.  It now fails with similar errors as well as some triggered by GTK_WIDGET_CAN_FOCUS no longer being defined.

So, I'm now in a state where I can't upgrade or downgrade.   HELP!

[David Ronis]

Created attachment 46397 [details]
Fix deprecated symbols to allow webkitgtk+ 1.1.18 to compile against gtk+-2.19.3.

It turns out there were only a few places that the deprecated symbols triggered a compile error.  The patch fixes there.  Note that one of the changes required me to cast as:

-    return GTK_WIDGET_CAN_FOCUS(m_webView);
+  return gtk_widget_get_can_focus((GtkWidget*)m_webView);

It compiles but I'm not sure it's right.

[Xan Lopez]

Comment on attachment 46397 [details]
Fix deprecated symbols to allow webkitgtk+ 1.1.18 to compile against gtk+-2.19.3.

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.

[David Ronis]

Hi xan,

I suspect you're right about the include.  I'd included it explicitly in the hope that I could simply modify the build to support deprecated symbols.   I wasn't able to figure that out and had forgotten to remove the include; you'll notice that the other patched files don't have the include.

As to protecting the patch's against GTK+ version and using GTK_WIDGET both suggestions make sense; however, I'm neither a webkit or GTK developer and would prefer someone who knows those codes make the change.

[Alejandro G. Castro]

Created attachment 46662 [details]
Proposed patch

Based on David patch.

[Alejandro G. Castro]

Created attachment 46663 [details]
Proposed patch

Changelog not saved :)

[Xan Lopez]

Comment on attachment 46663 [details]
Proposed patch

r=me

[WebKit Commit Bot]

Comment on attachment 46663 [details]
Proposed patch

Clearing flags on attachment: 46663

Committed r53351: <http://trac.webkit.org/changeset/53351>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.