GNOME Bugzilla – Bug 102216
icon themes install into ~/.themes instead of ~/.icons
Last modified: 2005-08-04 22:21:10 UTC
Package: control-center Severity: normal Version: GNOME2.1.5 cvs Synopsis: icon themes install into ~/.themes instead of ~/.icons Bugzilla-Product: control-center Bugzilla-Component: theme-manager Description: Icon themes should be installed to ~/.icons but they are installed into ~/.themes when you use the "Install theme" button. I've written a patch that fixes the problem. It is very simple in that it just duplicates the code used to install into ~/.themes and changes it to install into ~/.icons. It would be more ideal to just put in 3 or 4 if statements but my coding skills are too limited to work that out. Anyway, I'm going to post the patch. If no-one can come up with a better one it's better than nothing for 2.2. From the users' point of view it works :) ------- Bug moved to this database by unknown@bugzilla.gnome.org 2002-12-30 08:26 ------- Reassigning to the default owner of the component, control-center-maint@bugzilla.gnome.org.
Created attachment 13279 [details] [review] simple patch
Note: drag and drop still installs to .themes
I think that it would be a much better solution to pass a variable to gnome_theme_installer_run to tell it whether it is installing into .themes or .icons Hopefully someone with better coding skill than me will get to looking at this. I cannot remember how to pass variables. I was looking at fixing the drag and drop problem and it make much more sense to do it all properly from the beginning, otherwise the code is going to get really dirty.
pri=urgent, we need to do this right for 2.2.0
To Quote Seth: "Making sure jrb is aware there is a patch for this..."
Presumably this patch is still applicable, Mark?
I'll check it against HEAD to see if it still applies. But as I said, it only fixes half the problem and isn't really very nicely done.
Apparently it does still apply
*** Bug 107189 has been marked as a duplicate of this bug. ***
This should be retargetted for next gnome stable release. It's too bad this one got through to redhat 9, we are getting a lot of questions on IRC and linuxquestions.org about why this feature does't seem to work.
*** Bug 113781 has been marked as a duplicate of this bug. ***
Created attachment 17042 [details] [review] Shiny new patch
Ok, In the last 6 months my coding skills have improved a bit. That patch does it the "right way" AFAICT. The only problem is that since I made my first patch one thing has changed: When you dnd a file you are dragging it onto the notebook widget instead of onto the list inside the notebook. This makes it difficult if not impossible to determine if you dnd'd an icon theme. So that patch only fixes the install button on the icon tab, but even that is worth doing.
I haven't had time to look at it, but if there is a way to find out what tab a notebook is showing then it should be possible to fix dnd too. Will look into it.
*** Bug 116176 has been marked as a duplicate of this bug. ***
*** Bug 116392 has been marked as a duplicate of this bug. ***
This needs to be looked at soon.
By Now my patch is pretty much useless so I'm removing the PATCH keyword.
Can we *please* get this fixed by 2.6?
*** Bug 129220 has been marked as a duplicate of this bug. ***
I'm working on this one. I'm also working on an intelligently detecting what type of theme is in a tarball and installing it to the correct place. I'm also going to try to add some checks to report an error if it's a gtk-engine or some other unknown tarball the user is trying to install.
Is anyone still working on this? Or otherwise I'm going to try and write a patch which does the following things which checks what kind of theme it is by looking into the index.theme file. (Are there already parsers for these kind of files?)
*** Bug 139459 has been marked as a duplicate of this bug. ***
Created attachment 29144 [details] [review] Initial proposed patch This patch try to solve some bugs ( #97177, #121427 and others). The idea is to have a more intelligent and user friendly theme installer. Inform to the user if the file is not in a supported format, if it not a theme and if the theme have installed corrected. This patch it's a initial work. Needs to cleanup and support more theme formats, but I'd like some feedback about it.
Created attachment 29187 [details] [review] New install procedure More work in the patch. Support gtk, metacity, metatheme and icons themes. Informs the user about the success or failure of the installation. Btw, the icon_theme var passed to the gnome_theme_installer_run doesn't do anything but I keep it in this patch.
Adding the PATCH keyword
*** Bug 145083 has been marked as a duplicate of this bug. ***
*** Bug 147842 has been marked as a duplicate of this bug. ***
Why don't all themes (wm, icon, gtk) just install in .themes in the first place?
Did you mean that install in .themes in the first extract and if it a icon theme move to .icons and if it's not a theme remove it? maybe it's better so there are more possibilities that the theme goes to .themes finally but I think it's more clear to install (extract) to a temp dir and find what type of theme it is, then move to the correct place.
I think they should just go into ~/.themes/Name/icons/. This would definately solve this issue, and make it easier for artists to distribute desktop themes. But that's just me. I have a patch to the theme spec that changes this among other things that I need to clean up and re-submit.
If the theme spec change to .themes, the problem is solved. Will be this change in time for 2.8 ? I plan to do more work on the patch and I will be great to known if it will be useful :)
Of course, you can't just change the theme spec; you'd have to add the directory as being somewhere that's OK to keep icon themes but older systems will still have them installed in .icons.
Created attachment 29852 [details] [review] Cleaning up the patch This patch clean some unneeded vars and callbacks: gnome_theme_installer_run: unneeded icon_theme var. theme-properties.glade: Do not needed "Install" button in the details dialog tabs ( See bug #122970) gnome-theme-details.c: Removed callbacks for the three "Install" button in the details dialog tabs.
Opps, the bug reference in the attachmet comment it's #107286. Sorry. Also the patch fix this bug's also: #121427 : Error control it's performed now and informs the user. #97177 : The patch correct this also, informing the user if the file it's not supported #114553 : Recommend syntax in this patch to extract files.
Some comments on the patch: 1) We really need to escape the filename we pass to the command line. Do that with g_shell_quote() 2) we should pass in absolute paths for commands ('/usr/bin/tar') and test for their existence. 3) untar the tarball in a temporary dot directory in ~/themes/ rather than /tmp/. This saves us from having it be on a different filesytem (or a ram disk, or a place with limited disk space...). Also, we don't have to worry about file permissions then. 4) Check to see if its a compilable module (eg, configure{,.in,.ac} exists). Give a special warning dialog in that case.
Created attachment 30115 [details] [review] Update patch Update patch with the comment of jrb and some fixed. The absolute path are /bin/gzip /bin/tar (in my distro there are not /usr/bin/tar) and /usr/bin/bzip2.
Created attachment 30161 [details] [review] Free the memory from g_shell_quote
Created attachment 33207 [details] [review] Update patch to current cvs
*** Bug 97177 has been marked as a duplicate of this bug. ***
Jonathan, Jody, can we get this patch in, please?! Putting on the 2.10 milestone.
Fixing gnome version. Just got bitten by this again, too.
Comment on attachment 33207 [details] [review] Update patch to current cvs I don't usually comment on theme manager bugs, but this has been sitting here for a long time. Let's get it in for 2.10.
Tried applying this but got rejects. Attaching a new try. Could someone take a look at that one and see if it still does what the original tried to do? I'm a bit afraid that we changed stuff around since this was made.
Created attachment 36830 [details] [review] updated to CVS
Comment on attachment 33207 [details] [review] Update patch to current cvs Please take another look at the newer patch.
Forgot to mention that just gnome-theme-installer.c had rejects and thus needs most attention.
Created attachment 36846 [details] [review] Some little updates Only 2 updates: - Support tgz extension - Inform the user if the file it's not a tar.gz (or tgz) or tar.bz2. - Add a Changelog entry Ok to commit please ? :)
Comment on attachment 36846 [details] [review] Some little updates Ok, go ahead.
Comment on attachment 36846 [details] [review] Some little updates Committed into HEAD. Thanks you all.
Just a quick note to say that I had some problems when testing this patch - installed a new theme - created a custom theme based on this - it didn't show up in the list after closing the dialog Could be a different bug though.
I can't reproduce the bug. Also, I think it can be another bug, because the patch doesn't touch the part of save a custom theme.
I don't know if its related, but gnome-theme-manager hangs immediately after installing a theme. $ gdb gnome-theme-manager GNU gdb Red Hat Linux (6.1post-1.20040607.43rh) Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "i386-redhat-linux-gnu"...Using host libthread_db lib rary "/lib/tls/libthread_db.so.1". (gdb) run Starting program: /gnome/install/bin/gnome-theme-manager [Thread debugging using libthread_db enabled] [New Thread -1208096544 (LWP 22195)] Detaching after fork from child process 22198. (gnome-theme-manager:22198): Gtk-WARNING **: Theme directory scalable/emblems of theme Flat-Blue has no size field [New Thread 13470640 (LWP 22199)] Detaching after fork from child process 22200. Detaching after fork from child process 22203. *** glibc detected *** double free or corruption (fasttop): 0x08da3628 *** Program received signal SIGABRT, Aborted.
+ Trace 55194
Thread NaN (LWP 22195)
I'm starting to think that the whole hash table memory management part of gnome-theme-info.c is broken. The docs say that both keys and values have to be there for the lifetime of the hash table, and therefore keys and values should be copied into the hash table. - result of g_hash_table_lookup() is freed in several places - add_data_to_hash_by_name() g_strdup()s when it does g_hash_table_insert() but still the key is g_strdup()ed again when calling add_data_to_hash_by_name() in some cases - it just feels very wrong? :)
Dennis, I guess you could try to remove the g_free (monitor_data); call at line 954 in gnome-theme-info.c and see if that helps. You are using CVS, right? I'll commit what I have locally so you can test that first maybe.
kmaraas : Does valgrind indicate any problems ? There's nothing that mandates the lifecycle of the value in a hashtable. The data structure does not actually reference it, only the application. The key needs to exist as long as the entry is there. It does look like add_data_to_hash_by_name is leaking the name
*** Bug 165807 has been marked as a duplicate of this bug. ***
Jody, valgrind doesn't show up anything special, but I was sufficiently confused by the inconsitencies in memory management in there that I wasn't sure what was up. I'm thinking of going back to square one and adding small incremental "known to be right" fixes until we get to the bottom of this. So, removing the strdup from the calls to add_data_to_hash_by_name is one thing. The other is to not free data that is already freed by the GDestroyNotify function when those are specified. Let's see if this makes the other things more obvious.
Kjartan, I am using CVS. I updated gnome-control-center, today. It has your latest change to gnome-theme-info.c. It fixed the crash that I reported above.
I'm still seeing this crash with latest cvs. I've been seeing this for a few months now. [aldug@edison ~]$ gdb gnome-theme-manager GNU gdb Red Hat Linux (6.1post-1.20040607.43rh) Copyright 2004 Free Software Foundation, Inc. GDB is free software, covered by the GNU General Public License, and you are welcome to change it and/or distribute copies of it under certain conditions. Type "show copying" to see the conditions. There is absolutely no warranty for GDB. Type "show warranty" for details. This GDB was configured as "i386-redhat-linux-gnu"...Using host libthread_db library "/lib/tls/libthread_db.so.1". (gdb) run Starting program: /opt/gnome28/bin/gnome-theme-manager [Thread debugging using libthread_db enabled] [New Thread -1208097088 (LWP 7943)] Detaching after fork from child process 7958. Program received signal SIGSEGV, Segmentation fault.
+ Trace 55283
Thread NaN (LWP 7943)
It seems to die if a theme in $prefix/share/themes does not have an index.theme, file, a metacity-1, etc
Alex, do you have gnome-vfs from CVS too? The most critical fix for the theme manager was fixing a call there to not return NULL ever.
So, this should all be working now with this patch, and the cleanup I did after this was commited. Please test and see if the following works: - installing new themes - having the newly installed themes show up in the details dialog immediately - customizing themes - changing between themes It would be nice if we could close this one now.
Sorry, but it still does not work for me. I tried to install a metacity theme by dragging the package on the theme manager (on the "details" page). There was definitely some reshuffling of the theme list, but the theme did not appear in the list before a restart.
This seems to work reliably for me with 2.9.91. Vincent, can you retest? I'm going to close for now...
*** Bug 312579 has been marked as a duplicate of this bug. ***