----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100892/#review2050 -----------------------------------------------------------
src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/100892/#comment1712> Duplicated code - I just used showLyrics() instead. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/100892/#comment1713> Fixed a null-ptr when pressing "ESC" when the lyrics applet was added when amarok was already started and a track was playing. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/100892/#comment1714> This should've used the LyricsData struct, but it wasn't. .toString() simply returned QString() (empty string). src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/100892/#comment1715> This may mean a tiny overhead for non-HTML lyrics, but it's probably good to have it anyway. src/context/applets/lyrics/LyricsApplet.cpp <http://git.reviewboard.kde.org/r/100892/#comment1716> This makes the code even more robust as before: HTML lyrics with no <title> tag simply showed no title - now they do :). src/context/engines/lyrics/LyricsEngine.cpp <http://git.reviewboard.kde.org/r/100892/#comment1717> We should update in any case. The LyricsApplet and update() already check if the lyrics were really updated. src/context/engines/lyrics/LyricsEngine.cpp <http://git.reviewboard.kde.org/r/100892/#comment1718> See what the comment says, that code-block was simply misplaced. - Martin On March 19, 2011, 8:43 p.m., Martin Blumenstingl wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100892/ > ----------------------------------------------------------- > > (Updated March 19, 2011, 8:43 p.m.) > > > Review request for Amarok and Rick W. Chen. > > > Summary > ------- > > This fixes the following lyrics related issues: > -Amarok crashed when the lyrics applet was removed and re-added while a track > was playing and the user pressed "ESC". > -If no lyrics script was running cached lyrics were not displayed (but an > error message that no scripts are running instead). > -HTML lyrics were not displayed (this is a regression). > -If the user edited the lyrics in TagDialog the changes were not > "synchronized" to the lyrics applet (this is probably also a regression). > > Can we make sure this patch gets into the next release? > > > Diffs > ----- > > src/context/applets/lyrics/LyricsApplet.cpp c3d82db > src/context/engines/lyrics/LyricsEngine.h dd6beb9 > src/context/engines/lyrics/LyricsEngine.cpp d346511 > > Diff: http://git.reviewboard.kde.org/r/100892/diff > > > Testing > ------- > > -HTML lyrics are working again > -Amarok does not crash anymore when re-adding the lyrics applet and pressing > "ESC" > etc. > > > Thanks, > > Martin > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
