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 640732 - Remove copy pasted lock button
Remove copy pasted lock button
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
: 640733 641028 (view as bug list)
Depends on: 626457
Blocks:
 
 
Reported: 2011-01-27 15:52 UTC by William Jon McCann
Modified: 2011-05-07 16:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (25.14 KB, patch)
2011-01-31 12:57 UTC, Thomas Wood
committed Details | Review

Description William Jon McCann 2011-01-27 15:52:40 UTC
Currently it is pretty annoying and unexpected when trying to change something and a password dialog pops up.  Probably better to do what the other panels do and unlock it all at once.

(and we should make sure PolicyKit is robust against time changes and not expire the authorizations)
Comment 1 Matthias Clasen 2011-01-31 01:45:41 UTC
The polkit bug about time changes is
https://bugs.freedesktop.org//show_bug.cgi?id=29712
Comment 2 Thomas Wood 2011-01-31 12:57:04 UTC
*** Bug 641028 has been marked as a duplicate of this bug. ***
Comment 3 Thomas Wood 2011-01-31 12:57:49 UTC
Created attachment 179715 [details] [review]
Patch
Comment 4 Bastien Nocera 2011-01-31 13:02:34 UTC
Review of attachment 179715 [details] [review]:

::: panels/datetime/cc-datetime-panel.c
@@ +873,3 @@
+  g_signal_connect (permission, "notify",
+                    G_CALLBACK (on_permission_changed), self);
+  lockbutton = dt_lock_button_new (permission);

The permission needs to be finalised when the panel goes away. Otherwise you'll crash when the permission expires and the panel is switched to another one.

::: panels/datetime/dt-lockbutton.c
@@ +1,3 @@
+/*
+ * Copyright (C) 2010 Red Hat, Inc.
+ * Author: Matthias Clasen

Is this button the exact same that lives in the user-accounts panel? If so, I'd rather it be made a bit more flexible and moved to common/, into its own small library.

That would ensure that behaviour and look are consistent.
Comment 5 Thomas Wood 2011-01-31 13:20:31 UTC
Review of attachment 179715 [details] [review]:

::: panels/datetime/cc-datetime-panel.c
@@ +873,3 @@
+  g_signal_connect (permission, "notify",
+                    G_CALLBACK (on_permission_changed), self);
+  on_permission_changed (permission, NULL, self);

As noted in the comment in the code, the lock button assumes ownership of the permission. Therefore, there shouldn't be any issue about the signal handler being called after the panel is destroyed since the button and the panel should be destroyed at the same time.

::: panels/datetime/dt-lockbutton.c
@@ +1,3 @@
+/*
+ * Copyright (C) 2010 Red Hat, Inc.
+ * Author: Matthias Clasen

This is the same, but Matthias suggested not putting into it into a shared library since he was hoping it would be available in GTK+ 3 very shortly. If the code is not in a shared library, then it needs a new type name because it's not possible register two types with the same name (which would occur if the user-accounts panel was opened, and then the date/time panel was opened).
Comment 6 Thomas Wood 2011-01-31 16:07:18 UTC
Comment on attachment 179715 [details] [review]
Patch

Patch committed, but Bastien asked that this bug is left open until the lock button class is available in GTK+.
Comment 7 Bastien Nocera 2011-02-23 15:02:51 UTC
I moved the lock button to libgnome-control-center, which has no API requirements, as CcLockButton. We'll remove it when GTK+ has its own version.
Comment 8 Bastien Nocera 2011-02-23 15:03:08 UTC
*** Bug 640733 has been marked as a duplicate of this bug. ***
Comment 9 Bastien Nocera 2011-05-07 16:48:31 UTC
commit da829ad5554821eedb9da384b237da60adc7e54a
Author: Bastien Nocera <hadess@hadess.net>
Date:   Sat May 7 17:47:27 2011 +0100

    lib: Remove CcLockbutton
    
    We can now use the lock button in GTK+ itself.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=640732

commit 8c6221a567c11ecd8e7ce17c86246f7bc59cd522
Author: Bastien Nocera <hadess@hadess.net>
Date:   Sat May 7 17:47:13 2011 +0100

    user-accounts: Use GTK+ lock button

commit b28923cc903fa966e5b64fa74ceedad7efe10eee
Author: Bastien Nocera <hadess@hadess.net>
Date:   Sat May 7 17:46:55 2011 +0100

    datetime: Use GTK+ lock button

commit 8dea9d125b9cd21176aa41d5ab712ab4d8995ade
Author: Bastien Nocera <hadess@hadess.net>
Date:   Sat May 7 17:46:43 2011 +0100

    printers: Use GTK+ lock button

commit 00bfe342bbf3a5cff27fd10e15e1d903f9397e74
Author: Bastien Nocera <hadess@hadess.net>
Date:   Sat May 7 17:46:25 2011 +0100

    build: Require a newer GTK+ for the lock button