GNOME Bugzilla – Bug 157452
remove call to gnome_theme_init() from the keyboard prefs dialog
Last modified: 2007-01-31 22:36:49 UTC
Please take a look at this patch. Not sure about the GtkMessageDialog changes in gnome-theme-save.c since I still get a warning from GCC: gnome-theme-save.c: In function `setup_directory_structure': gnome-theme-save.c:105: warning: assignment from incompatible pointer type And this line is actually the error string itself, so I don't know why it's causing the compiler to spit warnings... Should the string be declared like const gchar *error_string = "Error message"; and error_string be passed to gtk_message_dialog_new() maybe?
Created attachment 33470 [details] [review] patch to clean up warnings
Paolo Borelli pointed me to the correct fix for the dialog. Attaching an updated patch that should be ok for both head and stable.
Created attachment 33472 [details] [review] new patch
Note that the patch removes gnome_theme_init() from gnome-keybindings-properties.c which looked bogus, someone has to check that this is ok before commiting I guess.
I've reviewed the patch, it seems to be fine. Some comments: - capplets/common/activate-settings-daemon.c: the change is already in the CVS - capplets/display/main.c: why not removing this line ? - capplets/keybindings/gnome-keybinding-properties.c: what's bugged with the gnome_theme_init() call ? - capplets/theme-switcher/gnome-theme-save.c: a part of the changes is already in the CVS. Why changing the GTK_DIALOG_MODAL, that's not coherent with all the other dialogs used ...
- I'll drop the two first commented on above - keybindings properties shouldn't need to do gnome_theme_init() at all, and we've seen crash reports from this prefs dialog with theme related functions in the backtraces... - gnome-theme-save.c: I just followed the gtk_message_dialog_new() example to create a modal dialog in the api reference... GTK_DIALOG_MODAL wasn't mentioned there AFAIR...
- gnome_theme_init() has been added on purpose, so there is probably a reason ... Mon Jan 13 15:14:22 2003 Jonathan Blandford <jrb@redhat.com> * gnome-keybinding-properties.c (main): gnome_theme_init() added. - Perhaps the GTK_DIALOG_MODAL change is right, but it should be done on the other dialogs too so.
gnome_theme_init() was probably added because the capplet had a keyboard theme (emacs vs normal) at one point, but that was removed again AFAICS. I've commited most of the other stuff, but I've left the dialog as GTK_DIALOG_MODAL. Attaching a new patch with cleanups I've got left.
Created attachment 35198 [details] [review] cleanups
the keyboard theme is still here: /desktop/gnome/interface/gtk_key_theme I was supposed that to but I'm not sure of the details ... have you removed it ? Better to get jrb's advice on it first since he has probably added it for a reason ...
Jonathan? Comments?
We're planning on removing the keyboard theme stuff from the keyboard capplet, and move it into a powertools-type setup. So I'd like to keep that in the gnome-theme-* code, but remove it from the keyboard dialog.
So, removing gnome_theme_init() from the keyboard capplet is ok to do?
Jonathan, do you know if gnome_theme_init () is needed here or not ? So we can get this patch in ...
A part of the fixes from this patches are in the CVS now (bug #166267)
As Kjartan said, gnome_theme_init was originally required for the code dealing with key themes. This code has been dumped back in 2004 (revision 4972, to be exact). The keybindings capplet does not need it any more, so I've removed the call. I believe this was the last missing piece the wasn't applied, so I'm closing this bug. Please reopen if anything else is still open.