1288702 - |GtkVBox| is deprecated in Gtk+3

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Thomas Zimmermann [:tzimmermann] [:tdz]]

When building the FF updater with Gtk+3, the compiler generates the following warning.

> 0:06.39 /home/mozilla/Projects/mozilla/src/hg.mozilla.org/mozilla-central/toolkit/mozapps/update/updater/progressui_gtk.cpp: In function ‘int ShowProgressUI()’:
> 0:06.39 Warning: -Wdeprecated-declarations in /home/mozilla/Projects/mozilla/src/hg.mozilla.org/mozilla-central/toolkit/mozapps/update/updater/progressui_gtk.cpp: ‘GtkWidget* gtk_vbox_new(gboolean, gint)’ is deprecated (declared at /usr/include/gtk-3.0/gtk/deprecated/gtkvbox.h:61): Use 'gtk_box_new' instead
> 0:06.39 /home/mozilla/Projects/mozilla/src/hg.mozilla.org/mozilla-central/toolkit/mozapps/update/updater/progressui_gtk.cpp:105:21: warning: ‘GtkWidget* gtk_vbox_new(gboolean, gint)’ is deprecated (declared at /usr/include/gtk-3.0/gtk/deprecated/gtkvbox.h:61): Use 'gtk_box_new' instead [-Wdeprecated-declarations]
> 0:06.39    GtkWidget *vbox = gtk_vbox_new(TRUE, 6);
> 0:06.39                      ^

Comment 1

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: Bug 1288702: Firefox updater: Replace |GtkVBox| by |GtkBox| on Gtk+3

Review commit: https://reviewboard.mozilla.org/r/66452/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/66452/

Comment 2

[Thomas Zimmermann [:tzimmermann] [:tdz]]

I wasn't sure if Gtk+2 is still supported so I left the old code in place.

Comment 3

[Karl Tomlinson (:karlt)]

Comment on attachment 8773720 [details]
Bug 1288702: Firefox updater: Replace |GtkVBox| by |GtkBox| on Gtk+3

https://reviewboard.mozilla.org/r/66452/#review64612

I'm all for getting rid of warnings that are in the way, thanks, but I'd
prefer to share the same code with GTK2 and GTK3 builds where practical.

The "right" way to do things with GTK changes so often that it is not worth
trying to stay up to date just for the sake of it.
"GtkBox is going to go away eventually", for example.

This warning, and some others like it, could be suppressed by changing
GDK_VERSION_MIN_REQUIRED to 3_0.  However warnings for symbols deprecated in
3.0 can't be suppressed by that method, so I'm going to propose another
solution and see what glandium thinks of it.

Comment 4

[Karl Tomlinson (:karlt)]

Here's a list of similar warnings, the version in which the symbol was
deprecated, and the version in which the replacement was made available.

gdk_pointer_ungrab 3.0/3.0
gtk_key_snooper_install 3.4/-
gtk_key_snooper_remove 3.4/-
gtk_hbox_new 3.2/3.2
gtk_vbox_new 3.2/3.2
gtk_hbutton_box_new 3.2/3.0
gtk_icon_set_render_icon 3.0/2.4
gtk_style_lookup_icon_set 3.0/3.0
gtk_table_new 3.4/3.2?
gtk_table_attach 3.4/3.2?
gtk_widget_ensure_style 3.0/3.0
gtk_widget_get_style 3.0/3.0
gtk_widget_modify_text 3.0/?

Some of these don't really have replacements that can be written in terms of
GTK2 functions as was done for gdk_window_foreign_new_for_display() in
https://hg.mozilla.org/mozilla-central/diff/1bb9ee25eb2c/widget/gtk/compat/gdk/gdkx.h

Comment 5

[Karl Tomlinson (:karlt)]

Attachment: bug 1288702 suppress all GTK3 deprecated warnings

Functions deprecated in GTK3 are often used for compatibility with GTK2
builds.

Review commit: https://reviewboard.mozilla.org/r/67484/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/67484/

Comment 6

[Karl Tomlinson (:karlt)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9db43f1f9fb5

Comment 7

[Mike Hommey [:glandium]]

Comment on attachment 8775313 [details]
bug 1288702 suppress all GTK3 deprecated warnings

https://reviewboard.mozilla.org/r/67484/#review65532

::: widget/gtk/compat-gtk3/gdk/gdkversionmacros.h:22
(Diff revision 1)
> +#define GDK_VERSION_MIN_REQUIRED GDK_VERSION_3_0
> +
> +#include_next <gdk/gdkversionmacros.h>
> +
> +#undef GDK_DEPRECATED
> +#define GDK_DEPRECATED

You should make the macros expand to _GDK_EXTERN,for both macros, like when GDK_DISABLE_DEPRECATION_WARNINGS is set.

Comment 8

[Karl Tomlinson (:karlt)]

Comment on attachment 8775313 [details]
bug 1288702 suppress all GTK3 deprecated warnings

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/67484/diff/1-2/

Comment 9

[Karl Tomlinson (:karlt)]

https://reviewboard.mozilla.org/r/67484/#review65532

> You should make the macros expand to _GDK_EXTERN,for both macros, like when GDK_DISABLE_DEPRECATION_WARNINGS is set.

_GDK_EXTERN was introduced in 3.10 and its naming suggests that it may not be
stable.  GDK_AVAILABLE_IN_ALL was introduced in the same version and is
intended for use in the same situations as GDK_DEPRECATED*.  WDYT?
https://git.gnome.org/browse/gtk+/log/gdk/gdkversionmacros.h.in?h=gtk-3-10

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 8775313 [details]
bug 1288702 suppress all GTK3 deprecated warnings

https://reviewboard.mozilla.org/r/67484/#review66550

::: widget/gtk/compat-gtk3/gdk/gdkversionmacros.h:21
(Diff revisions 1 - 2)
>  
>  #define GDK_VERSION_MIN_REQUIRED GDK_VERSION_3_0
>  
>  #include_next <gdk/gdkversionmacros.h>
>  
> +/* GDK_AVAILABLE_IN_ALL was introduced in 3.10 */

... which we don't build against on automation.

::: widget/gtk/compat-gtk3/gdk/gdkversionmacros.h:23
(Diff revisions 1 - 2)
>  
>  #include_next <gdk/gdkversionmacros.h>
>  
> +/* GDK_AVAILABLE_IN_ALL was introduced in 3.10 */
> +#ifndef GDK_AVAILABLE_IN_ALL
> +#define GDK_AVAILABLE_IN_ALL

You still need a _GDK_EXTERN

Comment 11

[Mike Hommey [:glandium]]

(In reply to Karl Tomlinson (:karlt) from comment #9)
> https://reviewboard.mozilla.org/r/67484/#review65532
> 
> > You should make the macros expand to _GDK_EXTERN,for both macros, like when GDK_DISABLE_DEPRECATION_WARNINGS is set.
> 
> _GDK_EXTERN was introduced in 3.10 and its naming suggests that it may not be
> stable.  GDK_AVAILABLE_IN_ALL was introduced in the same version and is
> intended for use in the same situations as GDK_DEPRECATED*.  WDYT?
> https://git.gnome.org/browse/gtk+/log/gdk/gdkversionmacros.h.in?h=gtk-3-10

Missed this comment. Then you should define _GDK_EXTERN to extern if it's not defined.

Comment 12

[Karl Tomlinson (:karlt)]

Comment on attachment 8775313 [details]
bug 1288702 suppress all GTK3 deprecated warnings

(In reply to Mike Hommey [:glandium] from comment #11)
> Missed this comment. Then you should define _GDK_EXTERN to extern if it's
> not defined.

When building against GTK versions < 3.10, GDK_DEPRECATED* do not include
"extern", and so we should not add it.

https://git.gnome.org/browse/gtk+/tree/gdk/gdkversionmacros.h.in?h=gtk-3-8#n46

There is no reason to define _GDK_EXTERN.

Comment 13

[Mike Hommey [:glandium]]

Comment on attachment 8775313 [details]
bug 1288702 suppress all GTK3 deprecated warnings

https://reviewboard.mozilla.org/r/67484/#review66658

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/513d42e7a166
suppress all GTK3 deprecated warnings r=glandium

Comment 15

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/513d42e7a166