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 645921 - Drop old chat themes and make Adium theme mandatory
Drop old chat themes and make Adium theme mandatory
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Chat themes
2.33.x
Other Linux
: Normal normal
: 3.6
Assigned To: Danielle Madeley
Depends on: 595318 645920
Blocks: 586386
 
 
Reported: 2011-03-28 10:30 UTC by Guillaume Desmottes
Modified: 2012-07-02 12:13 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Guillaume Desmottes 2011-03-28 10:30:00 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.
Comment 1 Danielle Madeley 2011-10-27 01:16:38 UTC
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.
Comment 2 Guillaume Desmottes 2011-10-27 11:41:28 UTC
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 :)
Comment 3 Vanessa Gutiérrez 2011-10-30 23:14:20 UTC
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.
Comment 4 Guillaume Desmottes 2011-10-31 16:34:02 UTC
Awesome, thanks!
Comment 5 Guillaume Desmottes 2012-02-17 08:47:22 UTC
Vanessa: any progress on this?
Comment 6 Taryn Fox 2012-06-15 09:15:39 UTC
This bug appears to be blocking bug #678088 from being solved.
Comment 7 Danielle Madeley 2012-06-17 06:47:01 UTC
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.
Comment 8 drago01 2012-06-18 07:49:29 UTC
(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).
Comment 9 Guillaume Desmottes 2012-06-18 12:38:49 UTC
(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.
Comment 10 Danielle Madeley 2012-06-22 07:56:54 UTC
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.
Comment 11 Guillaume Desmottes 2012-06-22 13:25:15 UTC
(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
Comment 12 Guillaume Desmottes 2012-06-22 13:35:52 UTC
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
Comment 13 Danielle Madeley 2012-06-25 05:24:29 UTC
(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.
Comment 14 Guillaume Desmottes 2012-06-25 06:52:27 UTC
(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?
Comment 15 Danielle Madeley 2012-06-29 05:43:27 UTC
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.
Comment 16 Guillaume Desmottes 2012-06-29 06:44:36 UTC
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.
Comment 17 Guillaume Desmottes 2012-07-02 11:35:48 UTC
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.
Comment 18 Guillaume Desmottes 2012-07-02 12:13:23 UTC
Merged to master. Thanks a lot Danielle!