Bug 47249 - [GTK] Fix the build for GTK+ 3
Summary: [GTK] Fix the build for GTK+ 3
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 47279
Blocks:
  Show dependency treegraph
 
Reported: 2010-10-06 01:45 PDT by Carlos Garcia Campos
Modified: 2010-10-07 05:32 PDT (History)
5 users (show)

See Also:


Attachments
Do not use GdkDrawable deprecated API (4.63 KB, patch)
2010-10-06 01:50 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Don't use gtk_size_request_get_size() (2.98 KB, patch)
2010-10-06 01:58 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Use GdkVisual instead of GdkColormap (3.41 KB, patch)
2010-10-06 02:07 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch to remove use of GdkColormap (4.68 KB, patch)
2010-10-06 02:25 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Bump required gtk+-3 version to 2.91.0 (890 bytes, patch)
2010-10-06 03:42 PDT, Carlos Garcia Campos
xan.lopez: review-
Details | Formatted Diff | Diff
Updated patch to not use GdkDrawable deprecated API (5.31 KB, patch)
2010-10-06 06:46 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Another update for gdkdrawable deprecated api (5.38 KB, patch)
2010-10-07 00:19 PDT, Carlos Garcia Campos
mrobinson: review-
Details | Formatted Diff | Diff
Updated patch to not use gtk_size_request_get_size() (2.94 KB, patch)
2010-10-07 01:23 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Update gdkdrawable patch according to review (5.43 KB, patch)
2010-10-07 02:05 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Another update for the patch to remove the use of gdkcolormap (4.98 KB, patch)
2010-10-07 02:44 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch to bump gtk3 version (898 bytes, patch)
2010-10-07 02:49 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2010-10-06 01:45:04 PDT
API has changed again in gtk+ master, we need to port webkitgtk to the new API.
Comment 1 Carlos Garcia Campos 2010-10-06 01:50:25 PDT
Created attachment 69900 [details]
Do not use GdkDrawable deprecated API

Some methods of GdkDrawable are deprecated in gtk2 and have been removed in gtk3. Equivalent API has been added to GdkWindow.
Comment 2 Carlos Garcia Campos 2010-10-06 01:58:52 PDT
Created attachment 69903 [details]
Don't use gtk_size_request_get_size()

It has been removed, gtk_widget_get_preferred_size() should be used instead.
Comment 3 Carlos Garcia Campos 2010-10-06 02:07:09 PDT
Created attachment 69904 [details]
Use GdkVisual instead of GdkColormap

GdkColormap has been removed in gtk3
Comment 4 WebKit Review Bot 2010-10-06 02:11:08 PDT
Attachment 69904 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit/gtk/WebCoreSupport/DragClientGtk.cpp:109:  Tab found; better to use spaces  [whitespace/tab] [1]
WebKit/gtk/WebCoreSupport/DragClientGtk.cpp:109:  Declaration has space between type name and * in GdkVisual *visual  [whitespace/declaration] [3]
WebKit/gtk/WebCoreSupport/DragClientGtk.cpp:109:  Extra space before ( in function call  [whitespace/parens] [4]
WebKit/gtk/WebCoreSupport/DragClientGtk.cpp:111:  Tab found; better to use spaces  [whitespace/tab] [1]
WebKit/gtk/WebCoreSupport/DragClientGtk.cpp:111:  Extra space before ( in function call  [whitespace/parens] [4]
WebKit/gtk/WebCoreSupport/DragClientGtk.cpp:112:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/plugins/gtk/gtk2xtbin.c:329:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:331:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:345:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:346:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:347:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 11 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 WebKit Review Bot 2010-10-06 02:15:48 PDT
Attachment 69900 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/4147101
Comment 6 Carlos Garcia Campos 2010-10-06 02:25:53 PDT
Created attachment 69905 [details]
Updated patch to remove use of GdkColormap

Updated patch, it fixes the coding style issues and includes changes to WebCore/plugins/gtk/PluginViewGtk.cpp
Comment 7 Carlos Garcia Campos 2010-10-06 02:26:38 PDT
Comment on attachment 69903 [details]
Don't use gtk_size_request_get_size()

It seems I set the wrong flag
Comment 8 WebKit Review Bot 2010-10-06 02:28:23 PDT
Attachment 69905 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/plugins/gtk/gtk2xtbin.c:329:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:331:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:345:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:346:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:347:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 5 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 WebKit Review Bot 2010-10-06 02:28:52 PDT
Attachment 69903 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/platform/gtk/PopupMenuGtk.cpp:93:  Use 0 instead of NULL.  [readability/null] [5]
WebCore/platform/gtk/PopupMenuGtk.cpp:110:  Tab found; better to use spaces  [whitespace/tab] [1]
WebCore/platform/gtk/PopupMenuGtk.cpp:110:  Use 0 instead of NULL.  [readability/null] [5]
Total errors found: 3 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Carlos Garcia Campos 2010-10-06 03:42:08 PDT
Created attachment 69915 [details]
Bump required gtk+-3 version to 2.91.0

I've just realized we need to bump gtk+-3 requirements
Comment 11 Carlos Garcia Campos 2010-10-06 06:46:53 PDT
Created attachment 69931 [details]
Updated patch to not use GdkDrawable deprecated API

It fixes the syntax of the CangeLog file and the build issues with gtk2 < 2.24
Comment 12 Xan Lopez 2010-10-06 07:02:07 PDT
Comment on attachment 69931 [details]
Updated patch to not use GdkDrawable deprecated API

Looks good to me.
Comment 13 WebKit Commit Bot 2010-10-06 09:40:54 PDT
Comment on attachment 69931 [details]
Updated patch to not use GdkDrawable deprecated API

Clearing flags on attachment: 69931

Committed r69201: <http://trac.webkit.org/changeset/69201>
Comment 14 Carlos Garcia Campos 2010-10-07 00:19:53 PDT
Created attachment 70043 [details]
Another update for gdkdrawable deprecated api

Previous patch removed deprecated api in gtk3 but added gtk2 deprecated method, since it has been added to gtk3 with the same name.
Comment 15 Martin Robinson 2010-10-07 00:41:45 PDT
Comment on attachment 70043 [details]
Another update for gdkdrawable deprecated api

View in context: https://bugs.webkit.org/attachment.cgi?id=70043&action=review

Looks good, but I have a couple suggestions.

> WebKit/gtk/ChangeLog:11
> +        Some methods of GdkDrawable are deprecated in gtk2 and have been
> +        remmoved in gtk3. Equivalent API has been added to GdkWindow.

Should be "removed." Might want to tighten up the ChangeLog text a little to make it one continuous paragraph. It's good form to fill in the ChangeLog method sections too.

> WebCore/platform/gtk/PlatformScreenGtk.cpp:70
> +#ifdef GTK_API_VERSION_2
>      return gdk_drawable_get_visual(GDK_DRAWABLE(gtk_widget_get_window(container)));
> +#else
> +    return gdk_window_get_visual(gtk_widget_get_window(container));
> +#endif

I'd rather see gdk_window_get_visual defined in GtkVersioning.h, instead of having the #ifdef here.
Comment 16 Carlos Garcia Campos 2010-10-07 01:23:31 PDT
Created attachment 70048 [details]
Updated patch to not use gtk_size_request_get_size()

Fixed changelog formatting and coding style
Comment 17 Carlos Garcia Campos 2010-10-07 02:05:43 PDT
Created attachment 70054 [details]
Update gdkdrawable patch according to review

Moved gdk_window_get_visual to GtkVersioning defining it only when deprecated flags are not used and fixed typo in changelog
Comment 18 Xan Lopez 2010-10-07 02:13:30 PDT
Comment on attachment 70048 [details]
Updated patch to not use gtk_size_request_get_size()

Good.
Comment 19 Xan Lopez 2010-10-07 02:21:00 PDT
Comment on attachment 70054 [details]
Update gdkdrawable patch according to review

Looks good to me.
Comment 20 Xan Lopez 2010-10-07 02:33:06 PDT
Comment on attachment 69915 [details]
Bump required gtk+-3 version to 2.91.0

Need to regenerate the ChangeLog.
Comment 21 Carlos Garcia Campos 2010-10-07 02:44:22 PDT
Created attachment 70059 [details]
Another update for the patch to remove the use of gdkcolormap

Fixes changelog and style issues.
Comment 22 WebKit Review Bot 2010-10-07 02:46:45 PDT
Attachment 70059 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebCore/plugins/gtk/gtk2xtbin.c:329:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:331:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:345:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:346:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WebCore/plugins/gtk/gtk2xtbin.c:347:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 5 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 23 Carlos Garcia Campos 2010-10-07 02:49:45 PDT
Created attachment 70060 [details]
Updated patch to bump gtk3 version

Fixed changelog
Comment 24 WebKit Commit Bot 2010-10-07 02:55:57 PDT
Comment on attachment 70048 [details]
Updated patch to not use gtk_size_request_get_size()

Clearing flags on attachment: 70048

Committed r69285: <http://trac.webkit.org/changeset/69285>
Comment 25 Xan Lopez 2010-10-07 03:00:46 PDT
Comment on attachment 70059 [details]
Another update for the patch to remove the use of gdkcolormap

Looks reasonable to me. Needs the previous patch with the trickery for gdk_window_get_visual, so we need to be careful with that.
Comment 26 WebKit Commit Bot 2010-10-07 04:46:48 PDT
Comment on attachment 70054 [details]
Update gdkdrawable patch according to review

Clearing flags on attachment: 70054

Committed r69293: <http://trac.webkit.org/changeset/69293>
Comment 27 WebKit Commit Bot 2010-10-07 05:20:21 PDT
Comment on attachment 70059 [details]
Another update for the patch to remove the use of gdkcolormap

Clearing flags on attachment: 70059

Committed r69296: <http://trac.webkit.org/changeset/69296>
Comment 28 WebKit Commit Bot 2010-10-07 05:32:03 PDT
Comment on attachment 70060 [details]
Updated patch to bump gtk3 version

Clearing flags on attachment: 70060

Committed r69298: <http://trac.webkit.org/changeset/69298>
Comment 29 WebKit Commit Bot 2010-10-07 05:32:10 PDT
All reviewed patches have been landed.  Closing bug.