GNOME Bugzilla – Bug 534515
Add a tray icon + other cleanups
Last modified: 2009-01-10 13:31:37 UTC
Please describe the problem: Colin asked me to put these into Bugzilla: When working with a laptop a tray icon indicating the current state of the kerberos ticket is convenient (since we can't acquire one on login, etc.). The attached patches implement this. The tray icon shows the remaining time in the tooltip and indicates if we have a valid ticket via the tray icon itself. The context menu allows to drop the credentials cache entirely or to quit and has an about box (of course!). The patches in detail are: 0001-set-icon-to-gtk-dialog-authentication.patch: Set an icon that shows up in the window list when the dialog is displayed 0002-use-gtk-instead-of-gnome-headers.patch: we don't need any gnome specific stuff, so we can drop this dependency 0003-use-gboolean-instead-of-int.patch: minor cleanup 0004-check-for-dbus-development-files.patch: fixup autoconf, we don't need to look for gnome anymore but should look for dbus 0005-add-a-tray-icon.patch: this adds the actual tray icon 0006-install-the-icons.patch: these are the icons for the tray icon, these can certainly be improved Can these be applied? As you can see the changes to the actual krb5-auth-dialog-code are quiet minimal. Future versions might indicate the remaining time via a progress bar or list the available service tickets. Steps to reproduce: 1. 2. 3. Actual results: Expected results: Does this happen every time? Other information:
Created attachment 111418 [details] [review] 0001-set-icon-to-gtk-dialog-authentication Set an icon that shows up in the window list when the dialog is displayed
Created attachment 111419 [details] [review] 0002-use-gtk-instead-of-gnome-headers.patch we don't need any gnome specific stuff, so we can drop this dependency
Created attachment 111420 [details] [review] 0003-use-gboolean-instead-of-int.patch minor cleanup
Created attachment 111421 [details] [review] 0004-check-for-dbus-development-files.patch fixup autoconf, we don't need to look for gnome anymore but should look for dbus
Created attachment 111422 [details] [review] 0005-add-a-tray-icon.patch add the actual tray icon
Created attachment 111423 [details] [review] 0006-install-the-icons.patch these are the icons for the tray icon, these can certainly be improved
The tray icon is a pretty big change. It has some advantages and disadvantages. Off the top of my head: * Advantages: Would give us cleaner integration with the notification system; we could use libnotify notification pointing to the icon instead of a sudden dialog for renewal. Gives user visual indication that they're signed in. * Disadvantages: Another tray icon =/ On the code: There's a fair amount of inconsistent spacing, e.g. between function names and the paren, indentation level. + err = _("Failure to setup icons"); We should just g_error () on this stuff. It's a non-recoverable deployment/system error, there's no point in showing the user a dialog they can't do anything to fix. Otherwise looks OK at a high level. I should note I don't personally use Kerberos anymore so my testing isn't very good.
One other thought is to integrate this into the NetworkManager applet.
*** Bug 331656 has been marked as a duplicate of this bug. ***
Created attachment 112500 [details] [review] add trayicon
I've reworked the patch fixing up (hopefully) most of the indentation issues. The patch has more of my follow up patches included (like libnotify and gconf support). This allows to disable the trayicon via a gconf setting (which hopefully adresses your "yet another tray icon concerns"). The patch is quiet large but the updates to existing code aren't very intrusive so I decided not to split up things further. Can this be applied?
Created attachment 112502 [details] [review] desktop file cleanup
Created attachment 112503 [details] [review] minor manpage fixup
Except for the trayicon everything in this patch is applied now. If nobody objects I'll apply that one too. Adding this to NM doesn't make that much sense - I think - since the kerberos part will grow quit complex in the future. We might want to handle several credential caches as well as pkinit, etc so an extra applet is probaly justified. Just as a side note: I already pkinit support here: http://honk.sigxcpu.org/con/krb5_auth_dialog_pkinit_support.html I'll merge this after the applet stuff.
Current git with these patches is at: http://honk.sigxcpu.org/git/krb5-auth-dialog.git/ I'll push this onto a branch in SVN soonish.
I've merged things onto: http://svn.gnome.org/svn/krb5-auth-dialog/branches/pkinit now. Christopher, Collin: I'll wait a couple of days and merge to trunk if I hear no objections, o.k.?
All committed to trunk now. O.k. to tag a 0.8 release? Could someone give me the necessary permissions to close the bugzilla bugs?
Fixed on trunk.