After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 780864 - Replace deprecated GTK functions to build with -Werror
Replace deprecated GTK functions to build with -Werror
Status: RESOLVED FIXED
Product: gnome-disk-utility
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Kai Lüke
gnome-disk-utility-maint
Depends on:
Blocks:
 
 
Reported: 2017-04-03 09:59 UTC by Kai Lüke
Modified: 2017-04-05 12:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
remove deprecated GTK functions (8.81 KB, patch)
2017-04-03 09:59 UTC, Kai Lüke
none Details | Review
Remove deprecated GTK functions (cleaned up) (9.91 KB, patch)
2017-04-03 18:20 UTC, Kai Lüke
none Details | Review
Remove deprecated GTK functions (10.63 KB, patch)
2017-04-04 17:42 UTC, Kai Lüke
committed Details | Review

Description Kai Lüke 2017-04-03 09:59:55 UTC
Created attachment 349168 [details] [review]
remove deprecated GTK functions

Also some functions are not needed anymore and therefore removed.
Comment 1 Ondrej Holy 2017-04-03 14:36:14 UTC
Review of attachment 349168 [details] [review]:

Thanks for your patch!

Please be sure that "Patch" checkbox is checked and MIME type is "text/plain" when attaching patches.

Please add bugzilla links into the commit message :-)

::: src/disks/gduapplication.c
@@ +336,3 @@
 {
   //GduApplication *app = GDU_APPLICATION (user_data);
   //gtk_widget_destroy (GTK_WIDGET (app->window));

It would be nice to propose another patch to remove this redundant commented code...

@@ +337,3 @@
   //GduApplication *app = GDU_APPLICATION (user_data);
   //gtk_widget_destroy (GTK_WIDGET (app->window));
+  gtk_show_uri_on_window (NULL, /* GdkScreen */

The first argument is not GdkScreen, but GtkWindow, which would be nice to set if possible to show the help on a correct screen...

::: src/disks/gducreatediskimagedialog.c
@@ +496,3 @@
                                       _("_Delete Disk Image File"),
                                       GTK_RESPONSE_NO);
+      gtk_button_box_set_child_secondary (GTK_BUTTON_BOX (gtk_widget_get_parent (button)),

Yes, this fixes the warning, however, the fix does more-or-less the same as the deprecated function, but in a worse manner... :-P

I think that there are two possible ways to fix it properly:
- use gtk_message_dialog_new_with_markup without GTK_BUTTONS_CLOSE and add the close manually in preferred order...
- or, use gtk_dialog_new_with_buttons instead gtk_message_dialog_new_with_markup and specify both buttons as parameters (this would require probably more work to make it look as gtk_message_dialog)

::: src/disks/gducreatefilesystemwidget.c
@@ +547,3 @@
+  gtk_container_remove (GTK_CONTAINER (dummy_window), widget->grid);
+  gtk_container_add (GTK_CONTAINER (widget), widget->grid);
+  g_object_unref (widget->grid);

Looks good, but, I wonder why we need the dummy_window here, can't we simply remove the dummy window?

::: src/disks/gduvolumegrid.c
@@ -467,3 @@
-
-  context = gtk_widget_get_style_context (widget);
-  gtk_style_context_set_background (context, window);

Ah, the toolbar used to be white probably [1], but it isn't with current GTK+ versions, so that's the reason, why you can't see any change... I think it is ok, but maybe worth to discuss with aday.

[1] https://en.wikipedia.org/wiki/GNOME_Disks#/media/File:GNOME_Disks_3.12.1.png

::: src/disks/gduwindow.c
@@ +3432,3 @@
+                            GDK_GRAVITY_SOUTH_WEST,
+                            GDK_GRAVITY_NORTH_WEST,
+                            NULL);

Would be nice to use GtkPopover instead of those menus in the future, but that's another story :-P

@@ +3433,3 @@
+                            GDK_GRAVITY_NORTH_WEST,
+                            NULL);
+  }

Please remove this unwanted whitespace change.
Comment 2 Kai Lüke 2017-04-03 18:20:43 UTC
Created attachment 349196 [details] [review]
Remove deprecated GTK functions (cleaned up)

Good to know all this, I hope it is better now.
Yes, I would like to keep the appearance question of the volume grid as well as the popover idea in my mind but not for this patch.

I guess the dummy window is for convenience with Glade editing?

Regards
Comment 3 Ondrej Holy 2017-04-04 10:41:40 UTC
Review of attachment 349196 [details] [review]:

Great work, almost ready...

I would personally remove the TODOs from the commit message and file new bug reports instead, or add comments into the code...

::: src/disks/gducreatediskimagedialog.c
@@ -497,3 @@
                                       GTK_RESPONSE_NO);
-      gtk_button_box_set_child_secondary (GTK_BUTTON_BOX (gtk_dialog_get_action_area (GTK_DIALOG (dialog))),
-                                          button, TRUE);

Please remove also button declaration:
gducreatediskimagedialog.c:469:27: warning: variable ‘button’ set but not used
Comment 4 Ondrej Holy 2017-04-04 10:42:29 UTC
(In reply to Kai Lüke from comment #2)
> Created attachment 349196 [details] [review] [review]
> Remove deprecated GTK functions (cleaned up)
> 
> Good to know all this, I hope it is better now.
> Yes, I would like to keep the appearance question of the volume grid as well
> as the popover idea in my mind but not for this patch.^
>
> I guess the dummy window is for convenience with Glade editing?

Ah, you are right :-)
Comment 5 Kai Lüke 2017-04-04 17:42:11 UTC
Created attachment 349257 [details] [review]
Remove deprecated GTK functions

Oh, good that you've spotted it, thanks.
I've updated it :)
Comment 6 Ondrej Holy 2017-04-05 12:32:42 UTC
Review of attachment 349257 [details] [review]:

Looks good to me, thanks!
Comment 7 Ondrej Holy 2017-04-05 12:33:29 UTC
Attachment 349257 [details] pushed as c08ae34 - Remove deprecated GTK functions