Version: 2.3.2-GIT (using KDE 4.4.2) OS: Linux When I tried to delete two playlists by selecting the first and then Shift-selecting the second (note, the first is now expanded so the tracks of the first are also included in the selection, but the second it not expanded), then pressing 'Delete' on the keyboard, several things happened. First, many of the visible tracks under the first playlist disappeared and several blank rows were added beneath the remaining ones. Second, the delete playlist confirmation dialog appeared, but instead of asking to confirm the delete of two playlists, it asks in the singular (this counting issue is a separately reported bug). When I clicked 'Cancel', the tracklist for the first playlist did not revert. Upon restarting Amarok, I discovered that every third track, starting with the first, was missing (i.e., the 1st, 4th, 7th, 10th, etc.). Trying the process again, every third track (again, starting with the first) of the remaining tracks were missing. I discovered that this also happens when the playlist plus some of its tracks beneath it are selected and deleted in the same manner. When only part of the tracklist is selected, the number of tracks deleted is the max of the number of tracks selected or the number of tracks in the tracklist / 3 rounded up (and they are removed in order: 1, 4, 7, etc...). Reproducible: Always Steps to Reproduce: 1. In a fresh start, open 'Saved Playlists' then click on either of the two categories, opening it up. 2. Click on any of the playlists, then shift-click one after it (the first playlist, plus its tracklist, plus all of the playlists (unopened) up to the second one clicked should be selected). 3. Press 'Delete' (on the keyboard). 4. Click 'Cancel'. Actual Results: Tracks from the first selected playlist disappear (and the remaining ones are now accompanied by empty rows), and then, upon restarting Amarok, every third track starting from, and including, the first should be removed from the first selected playlist. Expected Results: Nothing should change. Since the delete was canceled, the state of the database/playlist files should not change.
I don't know how "major" this is, but it does cause unintended data loss, so I marked it as such...
Major is OK. For the GUI row issues, see bug 250432
Hello, I have submitted a patch for this issue in the review-board.
commit 22c4f116fbb9ab10650884d181dbe4e25ae2221e branch master Author: Bart Cerneels <bart.cerneels@kde.org> Date: Sun Nov 21 11:46:59 2010 +0100 Fix playlsit delete bugs. Using the playlist actions instead of removeRow(). Patch by Dennis Francis. Removed the expandToDepth( 0 ) hack because comment #6 in BR 245646. BUG:250746 BUG:250750 diff --git a/src/browsers/playlistbrowser/PlaylistBrowserView.cpp b/src/browsers/playlistbrowser/PlaylistBrowserView.cpp index 2603e2f..45be175 100644 --- a/src/browsers/playlistbrowser/PlaylistBrowserView.cpp +++ b/src/browsers/playlistbrowser/PlaylistBrowserView.cpp @@ -263,8 +263,21 @@ PlaylistBrowserNS::PlaylistBrowserView::keyPressEvent( QKeyEvent *event ) { case Qt::Key_Delete: { - foreach( const QModelIndex &selectedIdx, selectedIndexes() ) - model()->removeRow( selectedIdx.row(), selectedIdx.parent() ); + QModelIndexList indices = selectedIndexes(); + QActionList actions = actionsFor( indices ); + + if( actions.isEmpty() ) + { + debug() <<"No actions !"; + return; + } + foreach( QAction *actn, actions ) + if( actn ) + if( actn->objectName() == "deleteAction" ) + { + actn->trigger(); + actn->setData( QVariant() ); + } return; } } diff --git a/src/core-impl/podcasts/sql/SqlPodcastProvider.cpp b/src/core-impl/podcasts/sql/SqlPodcastProvider.cpp index ae61504..bf679fc 100644 --- a/src/core-impl/podcasts/sql/SqlPodcastProvider.cpp +++ b/src/core-impl/podcasts/sql/SqlPodcastProvider.cpp @@ -336,6 +336,7 @@ SqlPodcastProvider::playlistActions( Playlists::PlaylistPtr playlist ) { actionChannels = m_removeAction->data().value<Podcasts::SqlPodcastChannelList>(); } + m_removeAction->setObjectName( "deleteAction" ); actionChannels << sqlChannel; m_removeAction->setData( QVariant::fromValue( actionChannels ) ); @@ -392,6 +393,7 @@ SqlPodcastProvider::trackActions( Playlists::PlaylistPtr playlist, int trackInde m_deleteAction->setProperty( "popupdropper_svg_id", "delete" ); connect( m_deleteAction, SIGNAL( triggered() ), SLOT( slotDeleteDownloadedEpisodes() ) ); } + m_deleteAction->setObjectName( "deleteAction" ); if( m_writeTagsAction == 0 ) { diff --git a/src/playlistmanager/file/PlaylistFileProvider.cpp b/src/playlistmanager/file/PlaylistFileProvider.cpp index 573cdff..22e8abe 100644 --- a/src/playlistmanager/file/PlaylistFileProvider.cpp +++ b/src/playlistmanager/file/PlaylistFileProvider.cpp @@ -138,6 +138,7 @@ PlaylistFileProvider::playlistActions( Playlists::PlaylistPtr playlist ) m_deleteAction->setProperty( "popupdropper_svg_id", "delete" ); connect( m_deleteAction, SIGNAL( triggered() ), SLOT( slotDelete() ) ); } + m_deleteAction->setObjectName( "deleteAction" ); Playlists::PlaylistFileList actionList = m_deleteAction->data().value<Playlists::PlaylistFileList>(); @@ -181,6 +182,7 @@ PlaylistFileProvider::trackActions( Playlists::PlaylistPtr playlist, int trackIn "Remove From \"%1\"", playlist->name() ) ); } + m_removeTrackAction->setObjectName( "deleteAction" ); //Add the playlist/track combination to a QMultiMap that is stored in the action. //In the slot we use this data to remove that track from the playlist. PlaylistTrackMap playlistMap = m_removeTrackAction->data().value<PlaylistTrackMap>(); diff --git a/src/playlistmanager/sql/SqlUserPlaylistProvider.cpp b/src/playlistmanager/sql/SqlUserPlaylistProvider.cpp index 12da6ed..d2ae5b9 100644 --- a/src/playlistmanager/sql/SqlUserPlaylistProvider.cpp +++ b/src/playlistmanager/sql/SqlUserPlaylistProvider.cpp @@ -178,6 +178,7 @@ SqlUserPlaylistProvider::playlistActions( Playlists::PlaylistPtr playlist ) m_deleteAction->setProperty( "popupdropper_svg_id", "delete" ); connect( m_deleteAction, SIGNAL( triggered() ), SLOT( slotDelete() ) ); } + m_deleteAction->setObjectName( "deleteAction" ); Playlists::SqlPlaylistList actionList = m_deleteAction->data().value<Playlists::SqlPlaylistList>(); actionList << sqlPlaylist; @@ -204,6 +205,8 @@ SqlUserPlaylistProvider::trackActions( Playlists::PlaylistPtr playlist, int trac m_removeTrackAction->setProperty( "popupdropper_svg_id", "delete" ); connect( m_removeTrackAction, SIGNAL( triggered() ), SLOT( slotRemove() ) ); } + m_removeTrackAction->setObjectName( "deleteAction" ); + //Add the playlist/track combination to a QMultiMap that is stored in the action. //In the slot we use this data to remove that track from the playlist. PlaylistTrackMap playlistMap = m_removeTrackAction->data().value<PlaylistTrackMap>();