Version: (using KDE Devel) Installed from: Compiled sources When fixing #65326 I made the animated icons not work in KDE versions earlier than 3.2; this will be fixed soon (tomorrow).
Subject: Re: [patch] animated icons do not work in KDE < 3.2 Please can someone review the attached patch? A fix for this regression in KDE3.1: Look for the movies in crystal svg theme only (if KDE_VERSION < 3.1.90). This will not fix the problem for people who have kdelibs 3.1.90 onwards up until last friday or saturday, unfortunately, since compat/ doesn't build for those versions. Thanks, lilac Created an attachment (id=3205) fix-kde3.1-movies.patch
I don't see a problem with it, but you might want to wait for Martijn to commit. He seems more knowledgeable about all this kdelibs stuff.
Subject: Re: [Kopete-devel] animated icons do not work in KDE < 3.2 On Thursday 13 November 2003 22:04, Matt Rogers wrote: > I don't see a problem with it, but you might want to wait for > Martijn to commit. He seems more knowledgeable about all this kdelibs > stuff. Since you asked for it... I'm a bit surprised by all the loadMovie calls all around the code, I thought we had a single entry point for that, but that's a side issue not related to Richard's patch. What's definitely wrong is this: +libkopetecompat_la_SOURCES = (...) loadmovie.cpp loadmovie.h header files never belong in a _SOURCES line! + KIconTheme crystal( QString::fromLatin1("crystalsvg"), appname ); Is it possible to make this follow the user's own theme EASILY? It's not a biggie as it's themed in KDE 3.2 and if it's more than one or two lines of code don't do it unless you really like to do it just for fun. Also, in loadmovie.h there are a lot of #includes that can be forward declared or left out entirely, speeding up compilation. Once again not important here, but for the 'normal' libkopete code please don't do this. The actual payload in loadmovie.cpp looks ok at first glance, but that's stuff I'm not too familiar with myself. I trust you if you say it works :) All in all, nice code, thanks for the work! Oh, and I *really* like it that you even took care of the sync.sh script!
Subject: Re: animated icons do not work in KDE < 3.2 On Friday 14 November 2003 9:57 pm, you wrote: > On Thursday 13 November 2003 22:04, Matt Rogers wrote: > > I don't see a problem with it, but you might want to wait for > > Martijn to commit. He seems more knowledgeable about all this kdelibs > > stuff. > > Since you asked for it... > > I'm a bit surprised by all the loadMovie calls all around the code, I > thought we had a single entry point for that, but that's a side issue not > related to Richard's patch. It'd be nice specifically if there were a 'newMessageMovie' call or some such. But nicer would be if KopeteEmailWindow and KopeteChatWindow had a base class teased out of them (there's more duplication than just the loadMovie call). Is anyone keeping a list of things like this to do post-3.2? > What's definitely wrong is this: > > +libkopetecompat_la_SOURCES = (...) loadmovie.cpp loadmovie.h > > header files never belong in a _SOURCES line! Oops! I'll fix that before I commit. > + KIconTheme crystal( QString::fromLatin1("crystalsvg"), appname ); > > Is it possible to make this follow the user's own theme EASILY? It's not a > biggie as it's themed in KDE 3.2 and if it's more than one or two lines of > code don't do it unless you really like to do it just for fun. Not trivially, but it is doable. You'd call KIconTheme::inherits(), and walk down the tree. The kdelibs stuff has protection against cyclic includes, and does something magic if it sees 'hicolor', so it'd take me more investigation time than I think is necessary for a temporary band-aid like this. Anyway, it's not a regression any more, and that was the thing I was most concerned about. > Also, in loadmovie.h there are a lot of #includes that can be forward > declared or left out entirely, speeding up compilation. Once again not > important here, but for the 'normal' libkopete code please don't do this. I did think about this. <qmovie.h> - returned by value, can't forward declare <kicontheme.h> - can't forward declare KIcon::Group; it's a class member <qstring.h> - I'll make it forward declared before I commit. Every little helps :) > The actual payload in loadmovie.cpp looks ok at first glance, but that's > stuff I'm not too familiar with myself. I trust you if you say it works :) Very honourable of you. I'm a little suspicious of it myself, since it worked first time, but I guess sometimes you get lucky. > All in all, nice code, thanks for the work! Hey, no problem. Now we just have to sit back and see if anyone ever actually draws any custom connecting animations... :) > Oh, and I *really* like it that you even took care of the sync.sh script! So have I done enough to persuade you to let me put your name down in the CVS log? lilac
Subject: Re: [Kopete-devel] animated icons do not work in KDE < 3.2 On Friday 14 November 2003 23:23, Richard Smith wrote: > It'd be nice specifically if there were a 'newMessageMovie' call or some > such. But nicer would be if KopeteEmailWindow and KopeteChatWindow had a > base class teased out of them (there's more duplication than just the > loadMovie call). Is anyone keeping a list of things like this to do > post-3.2? Not to my knowledge. I usually just go over the code and look at it for stuff that is duplicated or otherwise needs cleaning up. And yes, giving the two classes a single base class is one of my longer-term wishes too. > Not trivially, but it is doable. Well, like I said, either do it trivially or don't do it at all, so just leave it. > I did think about this. > > <qmovie.h> - returned by value, can't forward declare Hmm, AFAIR classes are actually returned internally as a reference, so a forward declare 'class QMovie' might actually work. Would be interesting to test. > <qstring.h> - I'll make it forward declared before I commit. Every little > helps :) Heh, like I said, it doesn't matter a lot in compat/, but in libkopete it certainly helps, also to keep the code as clean (and pedantic ;) as possible. > So have I done enough to persuade you to let me put your name down in the > CVS log? My name??? Don't you have your own CVS account or am I mis-translating your English now?
Subject: Re: animated icons do not work in KDE < 3.2 On Friday 14 November 2003 10:47 pm, you wrote: > > <qmovie.h> - returned by value, can't forward declare > > Hmm, AFAIR classes are actually returned internally as a reference, so a > forward declare 'class QMovie' might actually work. Would be interesting to > test. Well, what do you know? It works! > > So have I done enough to persuade you to let me put your name down in the > > CVS log? > > My name??? Don't you have your own CVS account or am I mis-translating your > English now? Just trying to follow the rules for the freeze; last I heard we needed non-trivial commits to have the name of another developer who approved it. lilac
Subject: Re: [Kopete-devel] animated icons do not work in KDE < 3.2 On Saturday 15 November 2003 00:03, Richard Smith wrote: > Just trying to follow the rules for the freeze; last I heard we needed > non-trivial commits to have the name of another developer who approved it. Oh, well, my English skills certainly weren't good enough to understand that :( Anyway, as far as libkopete/compat (being KDE 3.1 only) falls under the feeze rules, feel free to mention my name.
Subject: kdenetwork/kopete CVS commit by lilachaze: Make animated icons work again in KDE 3.1.x Added a KopeteCompat::loadMovie function to compat/ to load movies from Crystal SVG (in KDE 3.1 icon theme inheritance was not respected for movies). Approved by Martijn Klingens CCMAIL: 67953-done@bugs.kde.org A libkopete/compat/loadmovie.cpp 1.1 [LGPL (v2+)] A libkopete/compat/loadmovie.h 1.1 [LGPL (v2+)] M +8 -0 kopete/kopetewindow.cpp 1.162 M +8 -0 kopete/systemtray.cpp 1.31 M +6 -0 kopete/chatwindow/kopetechatwindow.cpp 1.58 M +8 -0 kopete/chatwindow/kopeteemailwindow.cpp 1.29 M +1 -1 libkopete/compat/Makefile.am 1.12 M +1 -1 libkopete/compat/sync.sh 1.4