GNOME Bugzilla – Bug 740986
Cleanup deprecated gtk+ API compilation warnings
Last modified: 2015-01-20 13:10:44 UTC
In preparation to merge g-i-s into g-c-c, I figured it would be a good idea to cleanup our compilation warnings. After this patch set there's still a few warnings left about deprecations in gnome-desktop, goa and libnm. There's also a couple of minor bug fixes here that I noticed while working on this.
Created attachment 291893 [details] [review] background: Fix deprecated API usage
Created attachment 291894 [details] [review] power: Fix deprecated API usage
Created attachment 291895 [details] [review] color: Fix deprecated API usage
Created attachment 291896 [details] [review] display: Drop deprecated API usage on FooScrollArea painting This achieves a very similar result (darkening the background color) without having to peek into the style context. Note that the stroke is removed because it isn't really visible anyway.
Created attachment 291897 [details] [review] display: Fix deprecated API usage
Created attachment 291898 [details] [review] online-accounts: Fix deprecated API usage
Created attachment 291899 [details] [review] region: Fix deprecated API usage
Created attachment 291900 [details] [review] region: Don't show IBus sources in the system-wide input chooser Since we can't export IBus sources through the localed API we shouldn't even allow users to choose them. This allows us to stop having to show an apologising dialog which makes us look a bit bad and was using deprecated gtk+ API that we're trying to get rid of.
Created attachment 291901 [details] [review] sound/level-bar: Port to GtkStyleContext The result isn't exactly the same but very close and should handle theme changes better going forward.
Created attachment 291902 [details] [review] sound/speaker-test: Port to GtkGrid
Created attachment 291903 [details] [review] sound: Fix deprecated API usage
Created attachment 291904 [details] [review] user-accounts/crop-area: Fix background rendering around the picture Instead of trying to fill the displayed pixbuf with the background color (and failing at that), just make the pixbuf be an aspect correct scaled size of the original picture and draw it at the correct offset on the draw vfunc. This allows us to get rid of deprecated gtk+ API usage and fixes the background around the picture ending up black despite de code's intent.
Created attachment 291905 [details] [review] user-accounts/photo-dialog: Remove needless frame Now that the crop area isn't all black around the picture, this frame is just visual noise.
Created attachment 291906 [details] [review] user-accounts/crop-area: Enforce a minimum size Otherwise, since the dialog is resizable, users could resize us so small that we'd crash inside gdk_pixbuf_scale() .
Created attachment 291907 [details] [review] user-accounts: Fix deprecated API usage
Created attachment 291908 [details] [review] user-account: Don't stroke the down arrow with the border color Besides using deprecated gtk+ API this color is actually fully transparent in the current GNOME theme, meaning that the arrow ended up looking smaller than intended.
Created attachment 291909 [details] [review] search: Drop deprecated API usage
Created attachment 291910 [details] [review] privacy: Drop deprecated API usage
Created attachment 291911 [details] [review] sharing: Drop deprecated API usage
Created attachment 291912 [details] [review] wacom: Drop deprecated API usage
Created attachment 291913 [details] [review] printers: Drop remaining deprecated API usage
Created attachment 291914 [details] [review] shell: Drop deprecated API usage
Created attachment 291915 [details] [review] network: Drop deprecated API usage
Review of attachment 291910 [details] [review]: Looks good to me.
Created attachment 291939 [details] [review] network: Use GdNotification instead of GtkDialog for NM version warning This is more in line with current GNOME UI practices and allows us to drop deprecated gtk+ API usage.
Review of attachment 291893 [details] [review]: ++
Review of attachment 291894 [details] [review]: ++
Review of attachment 291895 [details] [review]: ::: panels/color/cc-color-device.c @@ +79,3 @@ gtk_widget_set_visible (priv->widget_button, profiles->len > 0); + gtk_image_set_from_icon_name (GTK_IMAGE (priv->widget_arrow), + priv->expanded ? "pan-down-symbolic" : "pan-end-symbolic", Please make sure to check this work correctly in RTL (should do, but...).
Review of attachment 291896 [details] [review]: Sure.
Review of attachment 291897 [details] [review]: Sure.
Review of attachment 291898 [details] [review]: Sure.
Review of attachment 291899 [details] [review]: ++
Review of attachment 291900 [details] [review]: I think this needs ui-review, we probably need to show that information in some other way, or find another way to export the settings to gdm.
Review of attachment 291901 [details] [review]: There's a bug opened about using GtkLevelBar instead of a custom widget, I'd rather go that way.
Review of attachment 291902 [details] [review]: Sure.
Review of attachment 291903 [details] [review]: Looks good otherwise. ::: panels/sound/gvc-channel-bar.c @@ +638,3 @@ } else { + gtk_widget_set_valign (bar->priv->mute_switch, GTK_ALIGN_CENTER); +G_GNUC_BEGIN_IGNORE_DEPRECATIONS probably needs an ifdef. #ifdef G_GNUC_END_IGNORE_DEPRECATIONS ....
Review of attachment 291904 [details] [review]: The same code needs to go in cheese, please make sure the code is reviewed there.
Review of attachment 291905 [details] [review]: Sure.
Review of attachment 291906 [details] [review]: Ditto, needs reviewing in cheese.
Review of attachment 291907 [details] [review]: Needs reviewing in cheese.
Review of attachment 291908 [details] [review]: Sure.
Review of attachment 291909 [details] [review]: ++
Review of attachment 291911 [details] [review]: ++
Review of attachment 291912 [details] [review]: Sure.
Review of attachment 291913 [details] [review]: Leaving that one to Marek.
Review of attachment 291914 [details] [review]: ::: shell/cc-application.c @@ -331,3 @@ G_MENU_MODEL (menu)); - gtk_application_add_accelerator (GTK_APPLICATION (application), Why not use gtk_application_set_accels_for_action()?
Review of attachment 291915 [details] [review]: ++
Review of attachment 291939 [details] [review]: Got a screenshot for ui-review? Code looks fine.
(In reply to comment #28) > Please make sure to check this work correctly in RTL (should do, but...). Yes, this works fine. (In reply to comment #34) > Review of attachment 291901 [details] [review]: > > There's a bug opened about using GtkLevelBar instead of a custom widget, I'd > rather go that way. Sure, attached a patch to bug 691559 and will drop this one. (In reply to comment #36) > probably needs an ifdef. > > #ifdef G_GNUC_END_IGNORE_DEPRECATIONS These have been in glib since 2.32 and we already require glib 2.39.91 (which should probably be bumped along with the gtk+ requirement for this patch set - will add the bump on top) I will address the other comments tomorrow.
Created attachment 292139 [details] before (In reply to comment #48) > Got a screenshot for ui-review? Code looks fine. Sure, here they go
Created attachment 292140 [details] after
(In reply to comment #51) > Created an attachment (id=292140) [details] > after I'd change the label to be more meaningful. Something like: "Oops, something has gone wrong. Please contact your software vendor." And then below, possibly in monospace, some technical details: "NetworkManager x.xx required, but version x.xx is installed." Also, it might be better to show a blank panel with this label in the middle, although I'd understand if that's too much effort.
Attachment 291893 [details] pushed as 6b9e4ec - background: Fix deprecated API usage Attachment 291894 [details] pushed as 7e7dbd1 - power: Fix deprecated API usage Attachment 291895 [details] pushed as 111f93d - color: Fix deprecated API usage Attachment 291896 [details] pushed as 3059006 - display: Drop deprecated API usage on FooScrollArea painting Attachment 291897 [details] pushed as dd05918 - display: Fix deprecated API usage Attachment 291898 [details] pushed as 54227dc - online-accounts: Fix deprecated API usage Attachment 291899 [details] pushed as 6686a79 - region: Fix deprecated API usage Attachment 291902 [details] pushed as 3d2576b - sound/speaker-test: Port to GtkGrid Attachment 291905 [details] pushed as 33404f1 - user-accounts/photo-dialog: Remove needless frame Attachment 291908 [details] pushed as a880013 - user-account: Don't stroke the down arrow with the border color Attachment 291909 [details] pushed as d3b6add - search: Drop deprecated API usage Attachment 291910 [details] pushed as 2a1455f - privacy: Drop deprecated API usage Attachment 291911 [details] pushed as 7d1a8db - sharing: Drop deprecated API usage Attachment 291912 [details] pushed as 205a0cd - wacom: Drop deprecated API usage Attachment 291915 [details] pushed as cd5316b - network: Drop deprecated API usage
Created attachment 292194 [details] [review] region: Don't show IBus sources in the system-wide input chooser Since we can't export IBus sources through the localed API we shouldn't even allow users to choose them. This allows us to stop having to show an apologising dialog which makes us look a bit bad and was using deprecated gtk+ API that we're trying to get rid of. Instead, we show the same information up front, in the input chooser. -- (In reply to comment #33) > I think this needs ui-review, we probably need to show that information in some > other way, or find another way to export the settings to gdm. Allan agreed on IRC with droping the dialog and instead show an informational string in the chooser itself. Further work on this can happen with the planed region panel re-design.
Comment on attachment 291903 [details] [review] sound: Fix deprecated API usage Attachment 291903 [details] pushed as 70c33f4 - sound: Fix deprecated API usage This one should also be OK to push since the macros exist on the glib version we already require
Created attachment 292196 [details] [review] shell: Drop deprecated API usage (In reply to comment #46) > Review of attachment 291914 [details] [review]: > Why not use gtk_application_set_accels_for_action()? Yeah, this is better
Created attachment 292211 [details] [review] network: Display an error instead of the panel on NM version mismatch This is more in line with current GNOME UI and allows us to drop deprecated gtk+ API usage. -- (In reply to comment #52) > I'd change the label to be more meaningful. Something like: > > "Oops, something has gone wrong. Please contact your software vendor." > > And then below, possibly in monospace, some technical details: > > "NetworkManager x.xx required, but version x.xx is installed." > > Also, it might be better to show a blank panel with this label in the middle, > although I'd understand if that's too much effort. Ok, here it is.
Created attachment 292212 [details] network panel - after
Comment on attachment 292212 [details] network panel - after Note that the version string here is bogus (0.9 == 0.9) because I cheated to make it appear.
Review of attachment 292194 [details] [review]: This is fine for now, but we need to start looking into a way to export the settings to gdm at least, so that IBus is usable system-wide.
Review of attachment 292196 [details] [review]: Sure.
Review of attachment 292211 [details] [review]: Better, thanks!
Attachment 292194 [details] pushed as 1029d0c - region: Don't show IBus sources in the system-wide input chooser Attachment 292196 [details] pushed as 52c27cd - shell: Drop deprecated API usage
Cheese patches reviewd and pushed, see bug 742530. Attachment 291904 [details] pushed as a566932 - user-accounts/crop-area: Fix background rendering around the picture Attachment 291906 [details] pushed as ca7421e - user-accounts/crop-area: Enforce a minimum size Attachment 291907 [details] pushed as 283f6b4 - user-accounts: Fix deprecated API usage
Review of attachment 291913 [details] [review]: Hi, thank you for the changes. I have just one request with which it is good for me to commit. ::: panels/printers/cc-printers-panel.c @@ -1601,3 @@ - widget = (GtkWidget*) - gtk_builder_get_object (priv->builder, "supply-drawing-area"); - Thank you for removing this, I overlooked it somehow when I was writing it. @@ +1657,3 @@ + gtk_render_frame (context, cr, 1, 1, width - 2, SUPPLY_BAR_HEIGHT - 2); + gtk_style_context_restore (context); Move the gtk_style_context_restore() to the same level as the gtk_style_context_save() please.
Comment on attachment 291913 [details] [review] printers: Drop remaining deprecated API usage (In reply to comment #65) > Move the gtk_style_context_restore() to the same level as the > gtk_style_context_save() please. Of course, thanks Attachment 291913 [details] pushed as 1df796a - printers: Drop remaining deprecated API usage
Rebased and pushed the network panel patch after IRC chat