Summary: | Upgrade 1.1.17->1.1.18 fails: GTK_WIDGET_TOPLEVEL' was not declared in this scope | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Ronis <David.Ronis> | ||||||||
Component: | New Bugs | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Critical | CC: | commit-queue, xan.lopez | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | Linux | ||||||||||
Attachments: |
|
Description
David Ronis
2010-01-11 12:39:54 PST
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! 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.
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. 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. Created attachment 46662 [details]
Proposed patch
Based on David patch.
Created attachment 46663 [details]
Proposed patch
Changelog not saved :)
Comment on attachment 46663 [details]
Proposed patch
r=me
Comment on attachment 46663 [details] Proposed patch Clearing flags on attachment: 46663 Committed r53351: <http://trac.webkit.org/changeset/53351> All reviewed patches have been landed. Closing bug. |