GNOME Bugzilla – Bug 645921
Drop old chat themes and make Adium theme mandatory
Last modified: 2012-07-02 12:13:23 UTC
Once we'll be able able to choose a specific theme for group chat (bug #595318) and have nice default themes (bug #645920) we may drop support for old themes and rely only on Adium themes.
Potentially we could develop two Adium themes, one called Classic, and one called Boxes (with variants simple, clean and blue) to replace the old themes.
Yeah that's a good idea; would be faster than blocking on a new design. Any chance you could take a look on this? I suck at HTML :)
As I spoke to Danielle through IRC, I'll be starting to work on porting these themes to Adium. I'm very happy to help you guys with this! I'm sure I'll be making questions/comments here or on IRC, mu nick is titacgs in case you want to get in touch with me.
Awesome, thanks!
Vanessa: any progress on this?
This bug appears to be blocking bug #678088 from being solved.
Here's an implementation of the Boxes themes http://cgit.collabora.com/git/user/danni/empathy.git/commit/?h=adium-themes&id=9e8475bf547db986bdb300d186639db18497f6c0 -- symlink it into ~/.local/share/adium/message-styles Need to add the classic theme, install the themes, remove the old code and add some code to migrate from the old themes to the new ones.
(In reply to comment #0) > Once we'll be able able to choose a specific theme for group chat (bug #595318) > and have nice default themes (bug #645920) we may drop support for old themes > and rely only on Adium themes. Unfortunately the Adium themes suck if you intend to copy & paste a conversation (the resulting text ends up unreadable).
(In reply to comment #8) > (In reply to comment #0) > > Once we'll be able able to choose a specific theme for group chat (bug #595318) > > and have nice default themes (bug #645920) we may drop support for old themes > > and rely only on Adium themes. > > Unfortunately the Adium themes suck if you intend to copy & paste a > conversation (the resulting text ends up unreadable). I just tried Danni's branch and copy/pasting works just fine using the Webkit re-implementation of the Classic theme. But I agree, it would be good to solve this with all the themes: bug #678313 Danielle: Great work! Let me know when your branch is ready for review.
http://cgit.collabora.com/git/user/danni/empathy.git/log/?h=adium-themes Review it! I wonder if we can improve the cut-n-paste-ability of our themes somehow. I can't imagine these themes are any worse than the TextView ones though.
(In reply to comment #10) > I wonder if we can improve the cut-n-paste-ability of our themes somehow. I > can't imagine these themes are any worse than the TextView ones though. We can, see Kov's comment: https://bugzilla.gnome.org/show_bug.cgi?id=678313#c1
Does the branch pass "make distcheck"? http://cgit.collabora.com/git/user/danni/empathy.git/commit/?h=adium-themes&id=c6d3e9b62d0105ddc8e7ffe57a9c754275575726 is uninstall working now? http://cgit.collabora.com/git/user/danni/empathy.git/commit/?h=adium-themes&id=fa1966f8cd9641defbfb83440ef78326a4883604 I wrote some code for this kind of migration task that need to be done only once; see ./src/empathy-sanity-cleaning.c I got this when testing your branch (the adium themes were not installed): ** (empathy:4743): CRITICAL **: file empathy-theme-manager.c: line 221 (empathy_theme_manager_create_view): should not be reached (empathy:4743): Gtk-CRITICAL **: gtk_container_add: assertion `GTK_IS_WIDGET (widget)' failed (empathy:4743): Gtk-CRITICAL **: gtk_widget_show: assertion `GTK_IS_WIDGET (widget)' failed ** (empathy:4743): CRITICAL **: empathy_chat_view_append_message: assertion `EMPATHY_IS_CHAT_VIEW (view)' failed ** (empathy:4743): CRITICAL **: empathy_chat_view_append_message: assertion `EMPATHY_IS_CHAT_VIEW (view)' failed ** (empathy:4743): CRITICAL **: empathy_chat_view_append_message: assertion `EMPATHY_IS_CHAT_VIEW (view)' failed ** (empathy:4743): CRITICAL **: empathy_chat_view_append_message: assertion `EMPATHY_IS_CHAT_VIEW (view)' failed ** (empathy:4743): CRITICAL **: empathy_chat_view_append_message: assertion `EMPATHY_IS_CHAT_VIEW (view)' failed ** (empathy:4743): CRITICAL **: empathy_chat_view_append_event: assertion `EMPATHY_IS_CHAT_VIEW (view)' failed
(In reply to comment #12) > Does the branch pass "make distcheck"? Yes, with one small fix. > http://cgit.collabora.com/git/user/danni/empathy.git/commit/?h=adium-themes&id=c6d3e9b62d0105ddc8e7ffe57a9c754275575726 > is uninstall working now? It's there, I just wonder if it could be better. > http://cgit.collabora.com/git/user/danni/empathy.git/commit/?h=adium-themes&id=fa1966f8cd9641defbfb83440ef78326a4883604 > > I wrote some code for this kind of migration task that need to be done only > once; see ./src/empathy-sanity-cleaning.c I think it's probably worth doing it separately here, unless we also wanted to migrate the meaning of keys, i.e. looking for themes in the search path, rather than hardcoding a path in the settings. > I got this when testing your branch (the adium themes were not installed): Yeah, that will happen, because it can't locate a theme to use. I'm not sure how to resolve this TBH unless we start looking up themes in the path instead (could possible do this in a backwards compatible way, where themes beginning with / are paths, but otherwise looked up). Then we could also search EMPATHY_SRCDIR.
(In reply to comment #13) > > I got this when testing your branch (the adium themes were not installed): > > Yeah, that will happen, because it can't locate a theme to use. I'm not sure > how to resolve this TBH unless we start looking up themes in the path instead > (could possible do this in a backwards compatible way, where themes beginning > with / are paths, but otherwise looked up). Then we could also search > EMPATHY_SRCDIR. I think being able to run it from sources (EMPATHY_SRCDIR) is important: much easier when testing/hacking on themes. Could you add that please?
Updated. Migration will now use a theme by name, which will look in EMPATHY_SRCDIR etc. Preferences still sets the hard theme path, this can be migrated easily enough though.
Your patches look good but I think empathy_theme_manager_get_adium_themes() should be modified as well to load themes from EMPATHY_SRCDIR. Atm the themes don't appear in the preferences dialog when running from source which kinda defeat the point. (In reply to comment #15) > Preferences still sets the hard theme path, this can be migrated easily enough > though. I guess we'll have to do that then, we don't want to save the EMPATHY_SRCDIR path.
I rebased your branch on top of master and fixed the last bits: http://cgit.collabora.com/git/user/cassidy/empathy/log/?h=adium-themes-645921 Only my patches need to be reviewed as I already reviewed yours.
Merged to master. Thanks a lot Danielle!