Changing status to bug.

A brainstorm: when editing lyrics Amarok could temporary set playing mode to repeat one song until work is done. When return to previous state.

I would prefer they way Amarok 1.4 handled this: while editing lyrics no new lyrics get loaded. 
If I want to edit also the lyrics for the next song I still can press "pause" to get more time.

Is this still valid for the current Amarok versions, either 2.2.1, 2.2.2 beta or 2.2-git?

(In reply to comment #4)
> Is this still valid for the current Amarok versions, either 2.2.1, 2.2.2 beta
> or 2.2-git?

I just tried it with Amarok 2.2.1 and yes, it still happens. Luckily I just copied the whole lyrics to the clipboard before the next song started. 

Oliver

Thank you for the feedback.

Well, I can confirm that this still happens in Amarok 2.2.2.

I was gonna file a new bug, but then I found this.

Thank you for the feedback, I updated the version.

is this still valid in Amarok 2.3.1, to be released next week? I can't reproduce it here

perfectly able to reproduce it here in git amster.

Steps:

1. Play song, let Amarok fetch lyrics
2. edit lyrics in the applet
3. go to next song by pressing the toolbar button (new lyrics show up)
4. go back to previous song

Result:
changes are lost

Expected:
No loading of new lyrics in step 3, was they were still being edited.

This is a dataloss bug.

Hi,

I'm currently working on this.
If you want you can follow my thread on the development mailing-list: http://lists.kde.org/?l=amarok-devel&m=128133767930495&w=2

Unfortunately I cannot promise that this will make it into the next version of amarok. But let's hope for the best.

Regards,
Martin

commit b11cd9dba88a2407015badc76239bb395ca9ad65
branch master
Author: Martin Blumenstingl <darklight.xdarklight@googlemail.com>
Date:   Wed Nov 3 21:05:40 2010 +0100

    Improve the lyrics handling in Amarok.
    -Update the lyrics in the LyricsApplet if they were changed in the TagDialog.
    -Improve the check for cached lyrics: lyrics with just whitespaces in it are now also "empty".
    -Add a warning dialog which notifies the user if changes in the LyricsApplet would get lost.
    -Remove the KMessageBox in the LyricsApplet and replace it by Context::Applet::showWarning().
    -Added a way to show warnings which lock one specific applet.
    -Minor code cleanup.
    
    BUG: 207621
    REVIEWBOARD: http://git.reviewboard.kde.org/r/100013/

diff --git a/src/context/Applet.cpp b/src/context/Applet.cpp
index c25a143..55424a9 100644
--- a/src/context/Applet.cpp
+++ b/src/context/Applet.cpp
@@ -44,6 +44,7 @@ Context::Applet::Applet( QObject * parent, const QVariantList& args )
     , m_canAnimate( !KServiceTypeTrader::self()->query("Plasma/Animator", QString()).isEmpty() )
     , m_collapsed( false )
     , m_transient( 0 )
+    , m_isMessageShown( false )
     , m_standardPadding( 6.0 )
     , m_textBackground( 0 )
 {
@@ -390,9 +391,50 @@ Context::Applet::paletteChanged( const QPalette & palette )
     Q_UNUSED( palette )
 }
 
+void
+Context::Applet::plasmaMessageHidden()
+{
+    // Disconnect from the messageButtonPressed() signal so the next
+    // dialog can be shown.
+    disconnect( SIGNAL( messageButtonPressed( const MessageButton ) ) );
+
+    // No dialog is shown anymore.
+    m_isMessageShown = false;
+}
+
 bool Context::Applet::canAnimate()
 {
     return m_canAnimate;
 }
 
+void
+Context::Applet::showWarning( const QString &message, const char *slot )
+{
+    // Show a message with the "warning" icon.
+    showMessage( message, slot, KIcon( "dialog-warning" ) );
+}
+
+void
+Context::Applet::showMessage( const QString &message, const char *slot, const KIcon &icon )
+{
+    DEBUG_BLOCK
+
+    // Only show the message if none is shown yet.
+    if( !m_isMessageShown )
+    {
+        // Make sure no one else can show a dialog.
+        m_isMessageShown = true;
+
+        // Get the "Yes" and "No" buttons.
+        Plasma::MessageButtons plasmaYesNoButtons = Plasma::ButtonYes | Plasma::ButtonNo;
+
+        // Connect Plasma's messageButtonPressed SIGNAL to the given slot.
+        connect( this, SIGNAL( messageButtonPressed( const MessageButton ) ), slot );
+        connect( this, SIGNAL( messageButtonPressed( const MessageButton ) ), SLOT( plasmaMessageHidden() ) );
+
+        // Show a dialog and ask the user what to do.
+        Plasma::Applet::showMessage( icon, message, plasmaYesNoButtons );
+    }
+}
+
 #include "Applet.moc"
diff --git a/src/context/Applet.h b/src/context/Applet.h
index 137e9ea..e8cad32 100644
--- a/src/context/Applet.h
+++ b/src/context/Applet.h
@@ -26,6 +26,8 @@
 #include <QString>
 #include <QWeakPointer>
 
+#include <KIcon>
+
 class QPainter;
 class QPropertyAnimation;
 
@@ -89,6 +91,27 @@ class AMAROK_EXPORT Applet : public Plasma::Applet
           */
         qreal animationValue() const;
 
+        /**
+         * Shows a warning dialog which blocks access to the applet.
+         * This gives the user the message and a "Yes" and a "No" button.
+         * NOTE: Only one message/warning can be shown at a time.
+         *
+         * @param message The warning message.
+         * @param slot The slot which is called after either "Yes" or "No" has been clicked.
+         */
+        void showWarning( const QString &message, const char *slot );
+
+        /**
+         * Shows a message dialog which blocks access to the applet.
+         * This gives the user the message and a "Yes" and a "No" button.
+         * NOTE: Only one message/warning can be shown at a time.
+         *
+         * @param message The warning message.
+         * @param slot The slot which is called after either "Yes" or "No" has been clicked.
+         * @param icon The icon which will be shown.
+         */
+        void showMessage( const QString &message, const char *slot, const KIcon &icon );
+
     public Q_SLOTS:
         virtual void destroy();
 
@@ -99,6 +122,13 @@ class AMAROK_EXPORT Applet : public Plasma::Applet
     private slots:
         void paletteChanged( const QPalette & palette );
 
+        /**
+         * A private slot used to cleanup internal things like
+         * signals/slots and the flag if a dialog is shown.
+         * This is needed to avoid duplicate code in the applets.
+         */
+        void plasmaMessageHidden();
+
     protected:
         /**
          * Paint the background of an applet, so it fits with all the other applets.
@@ -121,6 +151,7 @@ class AMAROK_EXPORT Applet : public Plasma::Applet
         void cleanUpAndDelete();
 
         bool m_transient;
+        bool m_isMessageShown;
         qreal m_standardPadding;
         Plasma::FrameSvg *m_textBackground;
         QWeakPointer<QPropertyAnimation> m_animation;
diff --git a/src/context/LyricsManager.cpp b/src/context/LyricsManager.cpp
index 67c975d..b8460f9 100644
--- a/src/context/LyricsManager.cpp
+++ b/src/context/LyricsManager.cpp
@@ -17,12 +17,14 @@
 
 #include "LyricsManager.h"
 
+#include "core-impl/collections/support/CollectionManager.h"
 #include "core/support/Debug.h"
 #include "EngineController.h"
 
 #include <KLocale>
 
 #include <QDomDocument>
+#include <QGraphicsTextItem>
 
 ////////////////////////////////////////////////////////////////
 //// CLASS LyricsObserver
@@ -150,7 +152,7 @@ LyricsManager::lyricsResult( const QString& lyricsXML, bool cached ) //SLOT
 
         // FIXME: lyrics != "Not found" will not work when the lyrics script displays i18n'ed
         // error messages
-        if ( !lyrics.isEmpty() &&
+        if ( !isEmpty( lyrics ) &&
               lyrics != "Not found" )
         {
             // overwrite cached lyrics (as either there were no lyircs available previously OR
@@ -158,16 +160,14 @@ LyricsManager::lyricsResult( const QString& lyricsXML, bool cached ) //SLOT
             debug() << "setting cached lyrics...";
             The::engineController()->currentTrack()->setCachedLyrics( lyrics ); // TODO: setLyricsByPath?
         }
-        else if( !The::engineController()->currentTrack()->cachedLyrics().isEmpty() &&
+        else if( !isEmpty( The::engineController()->currentTrack()->cachedLyrics() ) &&
                   The::engineController()->currentTrack()->cachedLyrics() != "Not found" )
         {
             // we found nothing, so if we have cached lyrics, use it!
             debug() << "using cached lyrics...";
             lyrics = The::engineController()->currentTrack()->cachedLyrics();
 
-            // check if the lyrics data contains "<html" (note the missing closing bracket,
-            // this enables XHTML lyrics to be recognized)
-            if( lyrics.contains( "<html" , Qt::CaseInsensitive) )
+            if( isHtmlLyrics( lyrics ) )
             {
                 //we have stored html lyrics, so use that directly
                 sendNewLyricsHtml( lyrics );
@@ -176,7 +176,7 @@ LyricsManager::lyricsResult( const QString& lyricsXML, bool cached ) //SLOT
         }
 
         // only continue if the given lyrics are not empty
-        if ( !lyrics.isEmpty() )
+        if ( !isEmpty( lyrics ) )
         {
             // TODO: why don't we use currentTrack->prettyName() here?
             const QString title = el.attribute( "title" );
@@ -204,7 +204,7 @@ LyricsManager::lyricsResultHtml( const QString& lyricsHTML, bool cached )
 
     Meta::TrackPtr currentTrack = The::engineController()->currentTrack();
     if( currentTrack &&
-        !lyricsHTML.isEmpty() )
+        !isEmpty( lyricsHTML ) )
     {
         sendNewLyricsHtml( lyricsHTML );
 
@@ -254,15 +254,13 @@ bool LyricsManager::showCached()
 {
     //if we have cached lyrics there is absolutely no point in not showing these..
     Meta::TrackPtr currentTrack = The::engineController()->currentTrack();
-    if( currentTrack && !currentTrack->cachedLyrics().isEmpty() )
+    if( currentTrack && !isEmpty( currentTrack->cachedLyrics() ) )
     {
         // TODO: add some sort of feedback that we could not fetch new ones
         // so we are showing a cached result
         debug() << "showing cached lyrics!";
 
-        // check if the lyrics data contains "<html" (note the missing closing bracket,
-        // this enables XHTML lyrics to be recognized)
-        if( currentTrack->cachedLyrics().contains( "<html" , Qt::CaseInsensitive ) )
+        if( isHtmlLyrics( currentTrack->cachedLyrics() ) )
         {
             //we have stored html lyrics, so use that directly
             sendNewLyricsHtml( currentTrack->cachedLyrics() );
@@ -284,3 +282,40 @@ bool LyricsManager::showCached()
     }
     return false;
 }
+
+void LyricsManager::setLyricsForTrack( const QString &trackUrl, const QString &lyrics ) const
+{
+    DEBUG_BLOCK
+
+    Meta::TrackPtr track = CollectionManager::instance()->trackForUrl( KUrl( trackUrl ) );
+
+    if( track )
+        track->setCachedLyrics( lyrics );
+    else
+        debug() << QString("could not find a track for the given URL (%1) - ignoring.").arg( trackUrl );
+}
+
+bool LyricsManager::isHtmlLyrics( const QString &lyrics ) const
+{
+    // Check if the lyrics data contains "<html" (note the missing closing bracket,
+    // this ensures XHTML lyrics are recognized)
+    return lyrics.contains( "<html" , Qt::CaseInsensitive );
+}
+
+bool LyricsManager::isEmpty( const QString &lyrics ) const
+{
+    QGraphicsTextItem testItem;
+
+    // Set the text of the TextItem.
+    if( isHtmlLyrics( lyrics ) )
+        testItem.setHtml( lyrics );
+    else
+        testItem.setPlainText( lyrics );
+
+    // Get the plaintext content.
+    // We use toPlainText() to strip all Html formatting,
+    // so we can test if there's any text given.
+    QString testText = testItem.toPlainText().trimmed();
+
+    return testText.isEmpty();
+}
diff --git a/src/context/LyricsManager.h b/src/context/LyricsManager.h
index 24de425..7983455 100644
--- a/src/context/LyricsManager.h
+++ b/src/context/LyricsManager.h
@@ -91,6 +91,32 @@ class AMAROK_EXPORT LyricsManager : public LyricsSubject
         void lyricsError( const QString &error );
         void lyricsNotFound( const QString& notfound );
 
+        /**
+          * Sets the given lyrics for the track with the given URL.
+          *
+          * @param trackUrl The URL of the track.
+          * @param lyrics The new lyrics.
+          */
+        void setLyricsForTrack( const QString &trackUrl, const QString &lyrics ) const;
+
+        /**
+         * Tests if the given lyrics are Html or plain text.
+         *
+         * @param lyrics The lyrics which will be tested.
+         *
+         * @return true if the given lyrics are Html, otherwise false.
+         */
+        bool isHtmlLyrics( const QString &lyrics ) const;
+
+        /**
+         * Tests if the given lyrics are empty.
+         *
+         * @param lyrics The lyrics which will be tested.
+         *
+         * @return true if the given lyrics are empty, otherwise false.
+         */
+        bool isEmpty( const QString &lyrics ) const;
+
     private:
         bool showCached();
         
diff --git a/src/context/applets/lyrics/LyricsApplet.cpp b/src/context/applets/lyrics/LyricsApplet.cpp
index 224514d..c80483b 100644
--- a/src/context/applets/lyrics/LyricsApplet.cpp
+++ b/src/context/applets/lyrics/LyricsApplet.cpp
@@ -26,6 +26,7 @@
 #include "core/support/Amarok.h"
 #include "core/support/Debug.h"
 #include "dialogs/ScriptManager.h"
+#include "context/LyricsManager.h"
 
 #include <KConfigDialog>
 #include <KGlobalSettings>
@@ -56,6 +57,8 @@ public:
         , editIcon( 0 )
         , reloadIcon( 0 )
         , closeIcon( 0 )
+        , currentTrack( 0 )
+        , modifiedLyrics( QString() )
         , hasLyrics( false )
         , isRichText( true )
         , showBrowser( false )
@@ -69,8 +72,11 @@ public:
     void collapseToMin();
     void determineActionIconsState();
     void clearLyrics();
+    void refetchLyrics();
     void showLyrics( const QString &text, bool isRichText );
     void showSuggested( const QVariantList &suggestions );
+    void showUnsavedChangesWarning( Meta::TrackPtr );
+    const QString currentText();
 
     // private slots
     void _editLyrics();
@@ -79,6 +85,9 @@ public:
     void _saveLyrics();
     void _suggestionChosen( const QModelIndex &index );
     void _unsetCursor();
+    void _trackDataChanged( Meta::TrackPtr );
+    void _lyricsChangedMessageButtonPressed( const MessageButton );
+    void _refetchMessageButtonPressed( const MessageButton );
 
     // data / widgets
     QString titleText;
@@ -98,6 +107,10 @@ public:
 
     Ui::lyricsSettings ui_settings;
 
+    Meta::TrackPtr currentTrack;
+    Meta::TrackPtr modifiedTrack;
+    QString modifiedLyrics;
+
     bool hasLyrics;
     bool isRichText;
     bool showBrowser;
@@ -243,6 +256,75 @@ LyricsAppletPrivate::showSuggested( const QVariantList &suggestions )
     showSuggestions = true;
 }
 
+const QString
+LyricsAppletPrivate::currentText()
+{
+    return isRichText ? browser->nativeWidget()->toHtml() : browser->nativeWidget()->toPlainText();
+}
+
+void
+LyricsAppletPrivate::refetchLyrics()
+{
+    ScriptManager::instance()->notifyFetchLyrics( currentTrack->artist()->name(),
+                                                  currentTrack->name() );
+}
+
+void
+LyricsAppletPrivate::showUnsavedChangesWarning( Meta::TrackPtr newTrack )
+{
+    Q_Q( LyricsApplet );
+
+    // Set the track which was modified and store the current
+    // lyircs from the UI.
+    modifiedTrack = currentTrack;
+    modifiedLyrics = currentText();
+
+    QString artistName = modifiedTrack->artist() ? modifiedTrack->artist()->name() : i18nc( "Used if the current track has no artist.", "Unknown" );
+    QString warningMessage;
+
+    // Check if the track has changed.
+    if( newTrack != modifiedTrack )
+    {
+        // Show a warning that the track has changed while the user was editing the lyrics for the current track.
+        warningMessage = i18n( "While you were editing the lyrics of <b>%1 - %2</b> the track has changed. Do you want to save your changes?",
+                                artistName,
+                                modifiedTrack->prettyName() );
+    }
+    else
+    {
+        // Show a warning that the lyrics for the track were modified (for example by a script).
+        warningMessage = i18n( "The lyrics of <b>%1 - %2</b> changed while you were editing them. Do you want to save your changes?",
+                               artistName,
+                               modifiedTrack->prettyName() );
+    }
+
+    // Show the warning message.
+    q->showWarning( warningMessage, SLOT( _lyricsChangedMessageButtonPressed( MessageButton ) ) );
+
+    // Make the contents readonly again.
+    // Since the applet is now blocked the user can not enable this again.
+    // Thus we can make sure that we won't overwrite modifiedTrack.
+    setEditing( false );
+}
+
+void
+LyricsAppletPrivate::_refetchMessageButtonPressed( const MessageButton button )
+{
+    // Check if the user pressed "Yes".
+    if( button == Plasma::ButtonYes )
+        // Refetch the lyrics.
+        refetchLyrics();
+}
+
+void
+LyricsAppletPrivate::_lyricsChangedMessageButtonPressed( const MessageButton button )
+{
+    // Check if the user pressed "Yes".
+    if( button == Plasma::ButtonYes )
+        // Update the lyrics of the track.
+        modifiedTrack->setCachedLyrics( modifiedLyrics );
+}
+
 void
 LyricsAppletPrivate::_changeLyricsFont()
 {
@@ -275,9 +357,9 @@ LyricsAppletPrivate::_closeLyrics()
         int savedPosition = vbar->isVisible() ? vbar->value() : vbar->minimum();
 
         if( isRichText )
-            browser->nativeWidget()->setHtml( The::engineController()->currentTrack()->cachedLyrics() );
+            browser->nativeWidget()->setHtml( currentTrack->cachedLyrics() );
         else
-            browser->nativeWidget()->setPlainText( The::engineController()->currentTrack()->cachedLyrics() );
+            browser->nativeWidget()->setPlainText( currentTrack->cachedLyrics() );
 
         vbar->setSliderPosition( savedPosition );
 
@@ -298,19 +380,16 @@ LyricsAppletPrivate::_closeLyrics()
 void
 LyricsAppletPrivate::_saveLyrics()
 {
-    Meta::TrackPtr curtrack = The::engineController()->currentTrack();
-
-    if( curtrack )
+    if( currentTrack )
     {
-        if( !browser->nativeWidget()->toPlainText().isEmpty() )
+        if( !LyricsManager::self()->isEmpty( browser->nativeWidget()->toPlainText() ) )
         {
-            const QString lyrics = isRichText ? browser->nativeWidget()->toHtml() : browser->nativeWidget()->toPlainText();
-            curtrack->setCachedLyrics( lyrics );
+            currentTrack->setCachedLyrics( currentText() );
             hasLyrics = true;
         }
         else
         {
-            curtrack->setCachedLyrics( QString() );
+            currentTrack->setCachedLyrics( QString() );
             hasLyrics = false;
         }
         // emit sizeHintChanged(Qt::MaximumSize);
@@ -347,12 +426,39 @@ LyricsAppletPrivate::_unsetCursor()
         suggestView->unsetCursor();
 }
 
+void
+LyricsAppletPrivate::_trackDataChanged( Meta::TrackPtr track )
+{
+    DEBUG_BLOCK
+
+    // Check if we previously had a track.
+    // If the lyrics currently shown in the browser (which
+    // additionally is in edit mode) are different from the
+    // lyrics of the track we have to show a warning.
+    if( currentTrack &&
+        currentTrack->cachedLyrics() != currentText() &&
+        !browser->nativeWidget()->isReadOnly() )
+    {
+        showUnsavedChangesWarning( track );
+    }
+
+    // Update the current track.
+    currentTrack = track;
+}
+
 LyricsApplet::LyricsApplet( QObject* parent, const QVariantList& args )
     : Context::Applet( parent, args )
     , d_ptr( new LyricsAppletPrivate( this ) )
 {
     setHasConfigurationInterface( true );
     setBackgroundHints( Plasma::Applet::NoBackground );
+
+    EngineController* engine = The::engineController();
+
+    connect( engine, SIGNAL( trackChanged( Meta::TrackPtr ) ),
+             this, SLOT( _trackDataChanged( Meta::TrackPtr ) ) );
+    connect( engine, SIGNAL( trackMetadataChanged( Meta::TrackPtr ) ),
+             this, SLOT( _trackDataChanged( Meta::TrackPtr ) ) );
 }
 
 LyricsApplet::~LyricsApplet()
@@ -628,20 +734,21 @@ void
 LyricsApplet::refreshLyrics()
 {
     Q_D( LyricsApplet );
-    Meta::TrackPtr curtrack = The::engineController()->currentTrack();
-
-    if( !curtrack || !curtrack->artist() )
+    if( !d->currentTrack || !d->currentTrack->artist() )
         return;
 
-    bool refetch = true;
     if( d->hasLyrics )
     {
+        // Ask the user if he really wants to refetch the lyrics.
         const QString text( i18nc( "@info", "Do you really want to refetch lyrics for this track? All changes you may have made will be lost.") );
-        refetch = KMessageBox::warningContinueCancel( 0, text, i18n( "Refetch lyrics" ) ) == KMessageBox::Continue;
+        showWarning( text, SLOT( _refetchMessageButtonPressed( MessageButton ) ) );
+    }
+    else
+    {
+        // As we don't have lyrics yet we will
+        // refetch without asking the user.
+        d->refetchLyrics();
     }
-
-    if( refetch )
-        ScriptManager::instance()->notifyFetchLyrics( curtrack->artist()->name(), curtrack->name() );
 }
 
 void
diff --git a/src/context/applets/lyrics/LyricsApplet.h b/src/context/applets/lyrics/LyricsApplet.h
index bba44ee..4aaaf99 100644
--- a/src/context/applets/lyrics/LyricsApplet.h
+++ b/src/context/applets/lyrics/LyricsApplet.h
@@ -25,6 +25,8 @@
 
 class LyricsAppletPrivate;
 
+using namespace Plasma;
+
 class LyricsApplet : public Context::Applet
 {
     Q_OBJECT
@@ -60,6 +62,9 @@ private:
     Q_PRIVATE_SLOT( d_ptr, void _saveLyrics() )
     Q_PRIVATE_SLOT( d_ptr, void _suggestionChosen(const QModelIndex&) )
     Q_PRIVATE_SLOT( d_ptr, void _unsetCursor() )
+    Q_PRIVATE_SLOT( d_ptr, void _trackDataChanged( Meta::TrackPtr ) )
+    Q_PRIVATE_SLOT( d_ptr, void _lyricsChangedMessageButtonPressed( const MessageButton ) )
+    Q_PRIVATE_SLOT( d_ptr, void _refetchMessageButtonPressed( const MessageButton ) )
 };
 
 K_EXPORT_AMAROK_APPLET( lyrics, LyricsApplet )
diff --git a/src/context/engines/lyrics/LyricsEngine.cpp b/src/context/engines/lyrics/LyricsEngine.cpp
index a03aa53..104742d 100644
--- a/src/context/engines/lyrics/LyricsEngine.cpp
+++ b/src/context/engines/lyrics/LyricsEngine.cpp
@@ -65,7 +65,7 @@ bool LyricsEngine::sourceRequestEvent( const QString& name )
         if( m_prevLyricsList.size() > 0 )
             setData( "lyrics", "lyrics", m_prevLyricsList );
 
-        else if( !m_prevLyrics.isEmpty() )
+        else if( !LyricsManager::self()->isEmpty( m_prevLyrics ) )
             setData( "lyrics", "html", m_prevLyrics );
 
         if( m_prevSuggestionsList.size() > 0 )
@@ -81,6 +81,38 @@ bool LyricsEngine::sourceRequestEvent( const QString& name )
     return true;
 }
 
+bool LyricsEngine::testLyricsChanged( const QString& newLyrics,
+                                      const QString& oldHtmlLyrics,
+                                      QStringList oldPlainLyrics ) const
+{
+    DEBUG_BLOCK
+
+    bool retVal = false;
+
+    if ( LyricsManager::self()->isHtmlLyrics( newLyrics ) )
+        // Compare the old HTML lyrics with the new lyrics.
+        retVal = newLyrics != oldHtmlLyrics;
+    else
+    {
+        // If the given oldPlainLyrics list has >= 3 items
+        // then plaintext lyrics were provided.
+        bool hasPlainLyrics = oldPlainLyrics.count() >= 3;
+
+        if ( hasPlainLyrics )
+            // Previously we got plaintext lyrics -> compare them with
+            // the new ones.
+            retVal = newLyrics != oldPlainLyrics[ 3 ];
+        else
+            // There were no old lyrics -> if the new lyrics are
+            // non-empty they have changed.
+            retVal = !LyricsManager::self()->isEmpty( newLyrics );
+    }
+
+    debug() << "compared lyrics are the same = " << retVal;
+
+    return retVal;
+}
+
 void LyricsEngine::update()
 {
     DEBUG_BLOCK
@@ -145,8 +177,9 @@ void LyricsEngine::update()
     }
 
     // Check if the title, the artist and the lyrics are still the same.
-    // -- check if something changed for the previous fetched lyrics
-    if( title == m_title && artist == m_artist )
+    if( title == m_title &&
+        artist == m_artist &&
+        !testLyricsChanged( currentTrack->cachedLyrics(), m_currentLyrics, m_currentLyricsList ) )
         return; // nothing changed
 
     // -- really need new lyrics
@@ -163,13 +196,12 @@ void LyricsEngine::update()
     QString lyrics = currentTrack->cachedLyrics();
 
     // don't rely on caching for streams
-    const bool cached = !lyrics.isEmpty() && !The::engineController()->isStream();
+    const bool cached = !LyricsManager::self()->isEmpty( lyrics ) &&
+                        !The::engineController()->isStream();
 
     if( cached )
     {
-        // check if the lyrics data contains "<html" (note the missing closing bracket,
-        // this enables XHTML lyrics to be recognized)
-        if( lyrics.contains( "<html" , Qt::CaseInsensitive ) )
+        if( LyricsManager::self()->isHtmlLyrics( lyrics ) )
             newLyricsHtml( lyrics );
         else
         {
diff --git a/src/context/engines/lyrics/LyricsEngine.h b/src/context/engines/lyrics/LyricsEngine.h
index bf4a702..994b12d 100644
--- a/src/context/engines/lyrics/LyricsEngine.h
+++ b/src/context/engines/lyrics/LyricsEngine.h
@@ -54,6 +54,19 @@ private slots:
     void update();
 
 private:
+    /**
+      * Tests if the lyrics have changed.
+      *
+      * @param newLyrics The new lyrics.
+      * @param oldHtmlLyrics The old (unchanged) HTML lyrics.
+      * @param oldPlainLyrics The old plain lyrics (as provided by the LyricsEngine).
+      *
+      * @return true if the lyrics for the current track have changed, otherwise false.
+      */
+    bool testLyricsChanged( const QString& newLyrics,
+                            const QString& oldHtmlLyrics,
+                            QStringList oldPlainLyrics ) const;
+
     // stores is we have been disabled (disconnected)
     bool m_requested;
 
diff --git a/src/core-impl/collections/db/sql/SqlMeta.cpp b/src/core-impl/collections/db/sql/SqlMeta.cpp
index 1d70872..ff899fe 100644
--- a/src/core-impl/collections/db/sql/SqlMeta.cpp
+++ b/src/core-impl/collections/db/sql/SqlMeta.cpp
@@ -1352,6 +1352,8 @@ SqlTrack::setCachedLyrics( const QString &lyrics )
                                   m_collection->sqlStorage()->escape( m_rpath ) );
         m_collection->sqlStorage()->query( update );
     }
+
+    notifyObservers();
 }
 
 bool
diff --git a/src/core-impl/collections/nepomukcollection/NepomukTrack.cpp b/src/core-impl/collections/nepomukcollection/NepomukTrack.cpp
index 02f5bc7..7db01cf 100644
--- a/src/core-impl/collections/nepomukcollection/NepomukTrack.cpp
+++ b/src/core-impl/collections/nepomukcollection/NepomukTrack.cpp
@@ -327,6 +327,8 @@ void
 NepomukTrack::setCachedLyrics ( const QString& value )
 {
     m_nepores.setProperty( QUrl( "http://amarok.kde.org/metadata/1.0/track#lyrics" ), value );
+
+    notifyObservers();
 }
 
 void
diff --git a/src/dialogs/TagDialog.cpp b/src/dialogs/TagDialog.cpp
index ddd73cb..fa0b33a 100644
--- a/src/dialogs/TagDialog.cpp
+++ b/src/dialogs/TagDialog.cpp
@@ -848,6 +848,9 @@ TagDialog::metadataChanged( Meta::AlbumPtr album )
     // If the metadata of the current album has changed, reload the cover
     if( album == m_currentTrack->album() )
         loadCover();
+
+    // TODO: if the lyrics changed: should we show a warning and ask the user
+    // if he wants to use the new lyrics?
 }
 
 void
diff --git a/src/scriptengine/AmarokLyricsScript.cpp b/src/scriptengine/AmarokLyricsScript.cpp
index 5e7a94e..d045b79 100644
--- a/src/scriptengine/AmarokLyricsScript.cpp
+++ b/src/scriptengine/AmarokLyricsScript.cpp
@@ -18,7 +18,6 @@
 #include "AmarokLyricsScript.h"
 
 #include "core/support/Amarok.h"
-#include "core-impl/collections/support/CollectionManager.h"
 #include "core/support/Debug.h"
 #include "EngineController.h"
 #include "LyricsManager.h"
@@ -86,10 +85,7 @@ AmarokLyricsScript::escape( const QString& str )
 void
 AmarokLyricsScript::setLyricsForTrack( const QString& trackUrl, const QString& lyrics ) const
 {
-    DEBUG_BLOCK
-    Meta::TrackPtr track = CollectionManager::instance()->trackForUrl( KUrl( trackUrl ) );
-    if( track )
-        track->setCachedLyrics( lyrics );
+    LyricsManager::self()->setLyricsForTrack( trackUrl, lyrics );
 }
 
 QString
diff --git a/src/themes/context/Amarok-Mockup/colors b/src/themes/context/Amarok-Mockup/colors
index a0b9488..3fde41e 100644
--- a/src/themes/context/Amarok-Mockup/colors
+++ b/src/themes/context/Amarok-Mockup/colors
@@ -64,7 +64,7 @@ ForegroundInactive=152,152,152
 ForegroundLink=0,0,255
 ForegroundNegative=107,0,0
 ForegroundNeutral=0,90,95
-ForegroundNormal=0,0,0
+ForegroundNormal=255,255,255
 ForegroundPositive=0,95,0
 ForegroundVisited=88,55,150