Bug 220521 - [PATCH]Editing track details changes window title
Summary: [PATCH]Editing track details changes window title
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Playlists/Dynamic Playlists (show other bugs)
Version: 2.3.1
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-29 13:49 UTC by Oliver
Modified: 2010-07-08 22:00 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Screenshot showing the changed window title (63.45 KB, image/jpeg)
2010-01-12 08:31 UTC, Oliver
Details
This simple patch should fix the problem. (505 bytes, patch)
2010-07-07 23:18 UTC, Anton Gritsay
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Oliver 2009-12-29 13:49:19 UTC
Version:            (using KDE 4.3.3)
Compiler:          gcc (Gentoo 4.3.4 p1.0, pie-10.1.5) 4.3.4 
OS:                Linux
Installed from:    Gentoo Packages

When editing the track details of a track other then the currently playing  the Amarok window title changes to the artist and title of the just edited track when saving the changes.

Expected behavior is that it continues to show the artist and title of the currently playing track.

Tested with Amarok 2.2.1
Comment 1 Oliver 2009-12-30 10:51:01 UTC
Just checked with Amarok 2.2.1.90 (2.2.2 Beta 1) and the bug still exists.
Comment 2 Oliver 2009-12-30 12:20:33 UTC
The problem only occurs when making changes on the tags or statistics tab. Editing the lyrics and then saving the track detaiils doesn't change the window title.
Comment 3 Mikko C. 2010-01-03 16:54:37 UTC
I cannot reproduce this bug with amarok from git master, I tried to change both statistics and tags of songs in the playlist: Artist and title in amarok window do not change.
Comment 4 Myriam Schweingruber 2010-01-04 13:41:09 UTC
I can't confirm with current Amarok 2.2-git neither.
Comment 5 Oliver 2010-01-12 08:30:58 UTC
Just installed Amarok 2.2.2 and still have the problem. This is how I can reproduce it:

I recently started to rate all tracks when they are playing.

I'm playing a dynamic playlist and sometimes forget to rate a track while it is playing. Since the next track is already playing I click on the previous, unrated track with the right mouse button and select "Edit Track Details" from the context menu. 

Then I switch to the Statistcs tab, change the rating and click on "Save & Close".

As a result the name of the currently playing track is replaced with artist and title of the edited track. I've added a screenshot showing the result.
Comment 6 Oliver 2010-01-12 08:31:58 UTC
Created attachment 39806 [details]
Screenshot showing the changed window title
Comment 7 Myriam Schweingruber 2010-01-12 13:46:11 UTC
Oh, then this is a display error when Dynamic Playlists are activated, not the TagDialog. This works perfectly well when no dynamic playlist is running.
Comment 8 Oliver 2010-03-27 19:49:41 UTC
Bug confirmed for Amarok 2.3.0.
Comment 9 Oliver 2010-06-15 22:13:23 UTC
Just checked with Amarok 2.3.1 and can't reproduce the problem anymore.

Either it was fixed with 2.3.1 or through my update from KDE 4.3.5 to 4.4.4.
Comment 10 Oliver 2010-06-16 07:29:52 UTC
Sorry, have to reopen this report after I found that it still happens when changing the track details (like rating). The strange thing is that it doesn't happen each time I change a setting.

I'm currently using a dynamic playlist and it happens just when editing some of the last played tracks up to one specific track. When I the tracks I heard before this one nothing happens.

Don't know why it happens up to a specific track. I heard 11 tracks today without any interruption, editing one of the last 6 tracks changes the window title, editing of the first 5 tracks doesn't change the window title.
Comment 11 Anton Gritsay 2010-07-07 23:18:10 UTC
Created attachment 48669 [details]
This simple patch should fix the problem.
Comment 12 Nikolaj Hald Nielsen 2010-07-08 19:24:32 UTC
commit 74a3a06e1d7e2a82bbc44a6da8d5c88da21d8da2
Author: Nikolaj Hald Nielsen <nhn@kde.org>
Date:   Thu Jul 8 19:20:01 2010 +0200

    Fix track name in main window title incorrectly changing when editing tag info for another track.
    Thanks to Anton Gritsay <anton@angri.ru> for the patch
    
    BUG: 220521

diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp
index 6593870..e7f5bb5 100644
--- a/src/MainWindow.cpp
+++ b/src/MainWindow.cpp
@@ -1486,7 +1486,7 @@ MainWindow::engineNewTrackPlaying()
 void
 MainWindow::metadataChanged( Meta::TrackPtr track )
 {
-    if( track )
+    if( track && The::engineController()->currentTrack() == track )
         setPlainCaption( i18n( "%1 - %2  ::  %3", track->artist() ? track->artist()->prettyName() : i18n( "Unknown" ), track->prettyName(), AMAROK_CAPTION ) );
 }
Comment 13 Kevin Funk 2010-07-08 19:37:30 UTC
This is caused by not unsubscribing from one of the last tracks I guess. The "fix" is a workaround.

Maybe this part is missing a unsubscribeFrom() call:
    case Phonon::StoppedState:
        m_currentTrack = 0;
        setPlainCaption( i18n( AMAROK_CAPTION ) );
        break;
Comment 14 Anton Gritsay 2010-07-08 20:01:05 UTC
unsubscribeFrom() is called when state getting changed to PlayingState, so calling it once again in StoppedState won't help (I've tested it). Either unsubscription does not work as it should or the problem is somewhere else.
Comment 15 Anton Gritsay 2010-07-08 21:40:44 UTC
I hope I've found a problem.

1. When playing of the track ends (and when user press "next") MainWindow::engineNewTrackPlaying() changes m_currentTrack _before_ MainWindow::engineStateChanged() is called so when state become PlayingState unsubscription is called for _new_ playing track.
2. Unsubscription in engineStateChanged() definitely should be moved from PlayingState handler to StoppedState one because StoppedState-handler (which is called first) clears m_currentTrack.

With this two issues the MainWindow never gets unsubscribed from just played track.

Please, review the following patch. I think the first one should be reverted.

And someone, please, reopen the bugreport.

diff --git a/src/MainWindow.cpp b/src/MainWindow.cpp
index 178a78f..69a47f8 100644
--- a/src/MainWindow.cpp
+++ b/src/MainWindow.cpp
@@ -1449,12 +1449,12 @@ MainWindow::engineStateChanged( Phonon::State state, Phonon::State oldState )
     switch( state )
     {
     case Phonon::StoppedState:
+        unsubscribeFrom( m_currentTrack );
         m_currentTrack = 0;
         setPlainCaption( i18n( AMAROK_CAPTION ) );
         break;

     case Phonon::PlayingState:
-        unsubscribeFrom( m_currentTrack );
         m_currentTrack = track;
         subscribeTo( track );
         metadataChanged( track );
@@ -1472,8 +1472,7 @@ MainWindow::engineStateChanged( Phonon::State state, Phonon::State oldState )
 void
 MainWindow::engineNewTrackPlaying()
 {
-    m_currentTrack = The::engineController()->currentTrack();
-    metadataChanged( m_currentTrack );
+    // nothing interesting to do here
 }

 void
Comment 16 Kevin Funk 2010-07-08 21:53:11 UTC
Problem is: the call order by the observer is random. This is not predictable. We have to rely on metadataChanged().
Comment 17 Anton Gritsay 2010-07-08 22:00:51 UTC
The second patch does nothing with metadataChanged, it only allows to be sure that when currently playing track changes the MainWindow gets unsubscribed from previous one.

Or you mean the order of phonon state transitions is random?