Bug 47249 – [GTK] Fix the build for GTK+ 3

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Carlos Garcia Campos]

API has changed again in gtk+ master, we need to port webkitgtk to the new API.

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

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.

[Carlos Garcia Campos]

Created attachment 69904 [details]
Use GdkVisual instead of GdkColormap

GdkColormap has been removed in gtk3

[WebKit Review Bot]

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.

[WebKit Review Bot]

Attachment 69900 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/4147101

[Carlos Garcia Campos]

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

[Carlos Garcia Campos]

Comment on attachment 69903 [details]
Don't use gtk_size_request_get_size()

It seems I set the wrong flag

[WebKit Review Bot]

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.

[WebKit Review Bot]

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.

[Carlos Garcia Campos]

Created attachment 69915 [details]
Bump required gtk+-3 version to 2.91.0

I've just realized we need to bump gtk+-3 requirements

[Carlos Garcia Campos]

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

[Xan Lopez]

Comment on attachment 69931 [details]
Updated patch to not use GdkDrawable deprecated API

Looks good to me.

[WebKit Commit Bot]

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>

[Carlos Garcia Campos]

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.

[Martin Robinson]

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.

[Carlos Garcia Campos]

Created attachment 70048 [details]
Updated patch to not use gtk_size_request_get_size()

Fixed changelog formatting and coding style

[Carlos Garcia Campos]

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

[Xan Lopez]

Comment on attachment 70048 [details]
Updated patch to not use gtk_size_request_get_size()

Good.

[Xan Lopez]

Comment on attachment 70054 [details]
Update gdkdrawable patch according to review

Looks good to me.

[Xan Lopez]

Comment on attachment 69915 [details]
Bump required gtk+-3 version to 2.91.0

Need to regenerate the ChangeLog.

[Carlos Garcia Campos]

Created attachment 70059 [details]
Another update for the patch to remove the use of gdkcolormap

Fixes changelog and style issues.

[WebKit Review Bot]

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.

[Carlos Garcia Campos]

Created attachment 70060 [details]
Updated patch to bump gtk3 version

Fixed changelog

[WebKit Commit Bot]

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>

[Xan Lopez]

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.

[WebKit Commit Bot]

Comment on attachment 70054 [details]
Update gdkdrawable patch according to review

Clearing flags on attachment: 70054

Committed r69293: <http://trac.webkit.org/changeset/69293>

[WebKit Commit Bot]

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>

[WebKit Commit Bot]

Comment on attachment 70060 [details]
Updated patch to bump gtk3 version

Clearing flags on attachment: 70060

Committed r69298: <http://trac.webkit.org/changeset/69298>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.