GNOME Bugzilla – Bug 739737
Don't use properties of deprecated widgets
Last modified: 2014-11-25 11:06:48 UTC
Created attachment 290105 [details] [review] Don't use properties of deprecated widgets Several widgets used in the Printers panel of gnome-control-center use properties of widgets which are deprecated now. This can lead to assertion fail. Attached patch replaces xalign, yalign and xpad properties by halign, valign and margin-start wherever needed. It adds custom GtkLabel into GtkButton so we can adjust alignment of the text on the button which changes printer driver. It also sets padding of "printer-model-setting" widget at runtime.
Review of attachment 290105 [details] [review]: ::: panels/printers/cc-printers-panel.c @@ +859,1 @@ model_button = (GtkWidget*) You'll need to rename it. @@ +1002,1 @@ model_button = (GtkWidget*) Ditto. @@ +2837,3 @@ + gtk_builder_get_object (priv->builder, "printer-model-setting"); + + gtk_misc_get_padding (GTK_MISC (label), &pad, NULL); GtkMisc is deprecated as well. Best get rid of it before touching that code.
Created attachment 290187 [details] [review] Don't update alignment padding for dialogs The update of alignment padding is not needed since gtk+ 3.14. It was needed for proper alignment of widgets of action area with those from content area.
Created attachment 290188 [details] [review] Don't use methods of deprecated GtkMisc Use GtkWidget's methods for settings of halign, valign and margin-start instead of GtkMisc's methods for settings of xalign, yalign and xpad.
Created attachment 290189 [details] [review] Add GtkLabel with printer model name Place a custom GtkLabel with name of printer model into "printer-model-button". This is needed for us to be able to align the text and keep the button filling all available horizontal space.
Created attachment 290190 [details] [review] Fix padding of a progress text Set padding of the text "Setting new driver" at runtime so that it is aligned with other fields properly.
Created attachment 290191 [details] [review] Remove usage of deprecated GtkAlignment Use halign and valign of children instead of adding GtkAlignment and setting of xalign and yalign.
Created attachment 290192 [details] [review] Don't use properties of deprecated widgets Hi, thank you for the review. I've divided the patch into several smaller + added few fixes. (In reply to comment #1) > Review of attachment 290105 [details] [review]: > > ::: panels/printers/cc-printers-panel.c > @@ +859,1 @@ > model_button = (GtkWidget*) > > You'll need to rename it. Done. (in "Add GtkLabel with printer model name") > @@ +1002,1 @@ > model_button = (GtkWidget*) > > Ditto. Done. (in "Add GtkLabel with printer model name") > @@ +2837,3 @@ > + gtk_builder_get_object (priv->builder, "printer-model-setting"); > + > + gtk_misc_get_padding (GTK_MISC (label), &pad, NULL); > > GtkMisc is deprecated as well. Best get rid of it before touching that code. Done. (in "Don't use methods of deprecated GtkMisc") This patch replaces xalign and yalign properties by halign and valign wherever needed.
Created attachment 290193 [details] [review] Fix spacing in Jobs dialog Increase spacing in the VBox to 10 as in other dialogs. I've spot this during fixing of the bug.
Review of attachment 290187 [details] [review]: If this requires GTK+ 3.14, please update configure.ac as well.
Review of attachment 290188 [details] [review]: Sure.
Review of attachment 290190 [details] [review]: ::: panels/printers/cc-printers-panel.c @@ +2834,3 @@ gtk_widget_set_margin_start (label, offset); + + label = (GtkWidget*) please cast properly with GTK_WIDGET.
Review of attachment 290189 [details] [review]: ::: panels/printers/cc-printers-panel.c @@ +859,1 @@ + model_button_label = (GtkWidget*) Ditto about the casting.
Review of attachment 290191 [details] [review]: Looks fine.
Review of attachment 290192 [details] [review]: Looks fine.
Review of attachment 290193 [details] [review]: Sure.
Created attachment 291407 [details] [review] Use macros for casting of GObject (In reply to comment #11) > Review of attachment 290190 [details] [review]: > > ::: panels/printers/cc-printers-panel.c > @@ +2834,3 @@ > gtk_widget_set_margin_start (label, offset); > + > + label = (GtkWidget*) > > please cast properly with GTK_WIDGET. I went ahead and prepared this patch which fixes the casting for all Glib/GTK+ object types for whole Printers panel. I'll modify all the patches accordingly before push if you'll accept it.
Review of attachment 291407 [details] [review]: There's no need to do this now, this would just be churn. Correcting the patches is enough for me.
Comment on attachment 290190 [details] [review] Fix padding of a progress text (In reply to comment #11) > Review of attachment 290190 [details] [review]: > > ::: panels/printers/cc-printers-panel.c > @@ +2834,3 @@ > gtk_widget_set_margin_start (label, offset); > + > + label = (GtkWidget*) > > please cast properly with GTK_WIDGET. I've changed the patch to cast using GTK_WIDGET and pushed it master.
Comment on attachment 290189 [details] [review] Add GtkLabel with printer model name (In reply to comment #12) > Review of attachment 290189 [details] [review]: > > ::: panels/printers/cc-printers-panel.c > @@ +859,1 @@ > + model_button_label = (GtkWidget*) > > Ditto about the casting. This one too.
(In reply to comment #17) > Review of attachment 291407 [details] [review]: > > There's no need to do this now, this would just be churn. Correcting the > patches is enough for me. Ok, I've modified the casting in the 2 patches and pushed them to master together with the other patches. I'm closing this bug as resolved. Thank you for all the reviews. Marek