Version: 2.3-GIT (using Devel) OS: Linux User is not asked for confirmation if row should be really deleted. Reproducible: Always Steps to Reproduce: Steps to reproduce: 1. Open Playlists -> Saved playlists 2. Select one playlist file 3. Press Del Actual Results: Row is delted Expected Results: User confirmation dialog appears OS: Linux (i686) release 2.6.32-23-generic Compiler: cc
commit 79decb89a331076ec5fea163f9227216c5fc02c1 Author: Bart Cerneels <bart.cerneels@kde.org> Date: Fri Jul 16 20:44:17 2010 +0200 Don't remove playlists before confirmation. BUG:243966 diff --git a/src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp b/src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp index 66b7552..eb15a67 100644 --- a/src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp +++ b/src/browsers/playlistbrowser/PlaylistsByProviderProxy.cpp @@ -59,10 +59,13 @@ PlaylistsByProviderProxy::removeRows( int row, int count, const QModelIndex &par DEBUG_BLOCK bool result; debug() << "in parent " << parent << "remove " << count << " starting at row " << row; - beginRemoveRows( parent, row, row + count - 1 ); QModelIndex originalIdx = mapToSource( parent ); result = m_model->removeRows( row, count, originalIdx ); - endRemoveRows(); + if( result ) + { + beginRemoveRows( parent, row, row + count - 1 ); + endRemoveRows(); + } return result; } diff --git a/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp b/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp index 488d493..d2307d5 100644 --- a/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp +++ b/src/browsers/playlistbrowser/PlaylistsInGroupsProxy.cpp @@ -100,12 +100,13 @@ PlaylistsInGroupsProxy::removeRows( int row, int count, const QModelIndex &paren } //is a playlist not in a folder - //FIXME: before confirming deletion of a playlist it already dissapears from the - //view. The beginRemoveRows should not be called here but in the source model. QModelIndex childIdx = mapToSource( index( row, 0, m_rootNode ) ); - beginRemoveRows( QModelIndex(), row, row + count - 1 ); result = m_model->removeRows( childIdx.row(), count, m_rootNode ); - endRemoveRows(); + if( result ) + { + beginRemoveRows( parent, row, row + count - 1 ); + endRemoveRows(); + } return result; } diff --git a/src/browsers/playlistbrowser/UserPlaylistModel.cpp b/src/browsers/playlistbrowser/UserPlaylistModel.cpp index 50084e1..b5a751e 100644 --- a/src/browsers/playlistbrowser/UserPlaylistModel.cpp +++ b/src/browsers/playlistbrowser/UserPlaylistModel.cpp @@ -124,8 +124,7 @@ PlaylistBrowserNS::UserModel::removeRows( int row, int count, const QModelIndex if( playlistToRemove.isEmpty() ) return false; - The::playlistManager()->deletePlaylists( playlistToRemove ); - return true; + return The::playlistManager()->deletePlaylists( playlistToRemove ); } int playlistRow = REMOVE_TRACK_MASK(parent.internalId()); diff --git a/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.cpp b/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.cpp index 3754c87..e31579f 100644 --- a/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.cpp +++ b/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.cpp @@ -183,13 +183,14 @@ MediaDeviceUserPlaylistProvider::rename( Playlists::PlaylistPtr playlist, const } } -void +bool MediaDeviceUserPlaylistProvider::deletePlaylists( Playlists::PlaylistList playlistlist ) { Playlists::MediaDevicePlaylistList pllist; foreach( Playlists::PlaylistPtr playlist, playlistlist ) { - Playlists::MediaDevicePlaylistPtr pl = Playlists::MediaDevicePlaylistPtr::staticCast( playlist ); + Playlists::MediaDevicePlaylistPtr pl = + Playlists::MediaDevicePlaylistPtr::staticCast( playlist ); if( pl ) { @@ -200,6 +201,8 @@ MediaDeviceUserPlaylistProvider::deletePlaylists( Playlists::PlaylistList playli } emit playlistsDeleted( pllist ); + + return true; } void diff --git a/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.h b/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.h index 5e741f3..e1ae132 100644 --- a/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.h +++ b/src/core-impl/collections/mediadevicecollection/playlist/MediaDeviceUserPlaylistProvider.h @@ -57,7 +57,7 @@ class AMAROK_EXPORT MediaDeviceUserPlaylistProvider : public Playlists::UserPlay virtual void rename( Playlists::PlaylistPtr playlist, const QString &newName ); - virtual void deletePlaylists( Playlists::PlaylistList playlistlist ); + virtual bool deletePlaylists( Playlists::PlaylistList playlistlist ); /// MediaDevice-specific Functions diff --git a/src/core-impl/playlists/providers/user/UserPlaylistProvider.h b/src/core-impl/playlists/providers/user/UserPlaylistProvider.h index 9ede4ad..167bbea 100644 --- a/src/core-impl/playlists/providers/user/UserPlaylistProvider.h +++ b/src/core-impl/playlists/providers/user/UserPlaylistProvider.h @@ -58,10 +58,11 @@ class AMAROK_EXPORT UserPlaylistProvider : public PlaylistProvider int trackIndex ); // UserPlaylistProvider-specific - virtual bool isWritable() { return false; } - virtual void rename( Playlists::PlaylistPtr playlist, const QString &newName ) {Q_UNUSED( playlist ) Q_UNUSED(newName)} - virtual void deletePlaylists( Playlists::PlaylistList playlistlist ) { Q_UNUSED( playlistlist ) } + virtual void rename( Playlists::PlaylistPtr playlist, const QString &newName ) + { Q_UNUSED( playlist ) Q_UNUSED(newName) } + virtual bool deletePlaylists( Playlists::PlaylistList playlistlist ) + { Q_UNUSED( playlistlist ) return false; } signals: void updated(); diff --git a/src/playlistmanager/PlaylistManager.cpp b/src/playlistmanager/PlaylistManager.cpp index d057f70..be50e79 100644 --- a/src/playlistmanager/PlaylistManager.cpp +++ b/src/playlistmanager/PlaylistManager.cpp @@ -351,7 +351,7 @@ PlaylistManager::rename( Playlists::PlaylistPtr playlist ) } } -void +bool PlaylistManager::deletePlaylists( Playlists::PlaylistList playlistlist ) { // Map the playlists to their respective providers @@ -359,7 +359,8 @@ PlaylistManager::deletePlaylists( Playlists::PlaylistList playlistlist ) foreach( Playlists::PlaylistPtr playlist, playlistlist ) { // Get the providers of the respective playlists - Playlists::UserPlaylistProvider *prov = qobject_cast<Playlists::UserPlaylistProvider *>( getProvidersForPlaylist( playlist ).first() ); + Playlists::UserPlaylistProvider *prov = qobject_cast<Playlists::UserPlaylistProvider *>( + getProvidersForPlaylist( playlist ).first() ); if( prov ) { @@ -379,10 +380,13 @@ PlaylistManager::deletePlaylists( Playlists::PlaylistList playlistlist ) // Pass each list of playlists to the respective provider for deletion + bool removedSuccess = true; foreach( Playlists::UserPlaylistProvider* prov, provLists.keys() ) { - prov->deletePlaylists( provLists.value( prov ) ); //TODO: test + removedSuccess = prov->deletePlaylists( provLists.value( prov ) ) && removedSuccess; } + + return removedSuccess; } QList<Playlists::PlaylistProvider*> diff --git a/src/playlistmanager/PlaylistManager.h b/src/playlistmanager/PlaylistManager.h index 6161dbe..943fcf1 100644 --- a/src/playlistmanager/PlaylistManager.h +++ b/src/playlistmanager/PlaylistManager.h @@ -117,7 +117,7 @@ class AMAROK_EXPORT PlaylistManager : public QObject void rename( Playlists::PlaylistPtr playlist ); - void deletePlaylists( Playlists::PlaylistList playlistlist ); + bool deletePlaylists( Playlists::PlaylistList playlistlist ); Podcasts::PodcastProvider *defaultPodcasts() { return m_defaultPodcastProvider; } Playlists::UserPlaylistProvider *defaultUserPlaylists() { return m_defaultUserPlaylistProvider; } diff --git a/src/playlistmanager/file/PlaylistFileProvider.cpp b/src/playlistmanager/file/PlaylistFileProvider.cpp index 7d270cf..5c6bb48 100644 --- a/src/playlistmanager/file/PlaylistFileProvider.cpp +++ b/src/playlistmanager/file/PlaylistFileProvider.cpp @@ -306,7 +306,7 @@ PlaylistFileProvider::rename( Playlists::PlaylistPtr playlist, const QString &ne playlist->setName( newName ); } -void +bool PlaylistFileProvider::deletePlaylists( Playlists::PlaylistList playlists ) { Playlists::PlaylistFileList playlistFiles; @@ -317,10 +317,10 @@ PlaylistFileProvider::deletePlaylists( Playlists::PlaylistList playlists ) if( !playlistFile.isNull() ) playlistFiles << playlistFile; } - deletePlaylistFiles( playlistFiles ); + return deletePlaylistFiles( playlistFiles ); } -void +bool PlaylistFileProvider::deletePlaylistFiles( Playlists::PlaylistFileList playlistFiles ) { DEBUG_BLOCK @@ -335,7 +335,7 @@ PlaylistFileProvider::deletePlaylistFiles( Playlists::PlaylistFileList playlistF dialog.setButtonText( KDialog::Ok, i18n( "Yes, delete from disk." ) ); dialog.setMainWidget( &label ); if( dialog.exec() != QDialog::Accepted ) - return; + return false; foreach( Playlists::PlaylistFilePtr playlistFile, playlistFiles ) { @@ -345,6 +345,8 @@ PlaylistFileProvider::deletePlaylistFiles( Playlists::PlaylistFileList playlistF emit playlistRemoved( Playlists::PlaylistPtr::dynamicCast( playlistFile ) ); } loadedPlaylistsConfig().sync(); + + return true; } void diff --git a/src/playlistmanager/file/PlaylistFileProvider.h b/src/playlistmanager/file/PlaylistFileProvider.h index ac44660..4738f81 100644 --- a/src/playlistmanager/file/PlaylistFileProvider.h +++ b/src/playlistmanager/file/PlaylistFileProvider.h @@ -64,7 +64,7 @@ class PlaylistFileProvider : public Playlists::UserPlaylistProvider virtual bool isWritable() { return true; } virtual void rename( Playlists::PlaylistPtr playlist, const QString &newName ); - virtual void deletePlaylists( Playlists::PlaylistList playlistList ); + virtual bool deletePlaylists( Playlists::PlaylistList playlistList ); virtual void loadPlaylists(); @@ -78,7 +78,7 @@ class PlaylistFileProvider : public Playlists::UserPlaylistProvider void slotRemove(); private: - void deletePlaylistFiles( Playlists::PlaylistFileList playlistFiles ); + bool deletePlaylistFiles( Playlists::PlaylistFileList playlistFiles ); KConfigGroup loadedPlaylistsConfig() const; bool m_playlistsLoaded; diff --git a/src/playlistmanager/sql/SqlUserPlaylistProvider.cpp b/src/playlistmanager/sql/SqlUserPlaylistProvider.cpp index 53bb453..228c7d7 100644 --- a/src/playlistmanager/sql/SqlUserPlaylistProvider.cpp +++ b/src/playlistmanager/sql/SqlUserPlaylistProvider.cpp @@ -221,16 +221,16 @@ SqlUserPlaylistProvider::trackActions( Playlists::PlaylistPtr playlist, int trac return actions; } -void +bool SqlUserPlaylistProvider::deletePlaylists( Playlists::PlaylistList playlistList ) { Playlists::SqlPlaylistList sqlPlaylists; foreach( Playlists::PlaylistPtr playlist, playlistList ) sqlPlaylists << Playlists::SqlPlaylistPtr::dynamicCast( playlist ); - deleteSqlPlaylists( sqlPlaylists ); + return deleteSqlPlaylists( sqlPlaylists ); } -void +bool SqlUserPlaylistProvider::deleteSqlPlaylists( Playlists::SqlPlaylistList playlistList ) { if( !m_debug ) @@ -246,7 +246,7 @@ SqlUserPlaylistProvider::deleteSqlPlaylists( Playlists::SqlPlaylistList playlist dialog.setButtonText( KDialog::Ok, i18n( "Yes, delete from database." ) ); dialog.setMainWidget( &label ); if( dialog.exec() != QDialog::Accepted ) - return; + return false; } foreach( Playlists::SqlPlaylistPtr sqlPlaylist, playlistList ) @@ -258,6 +258,8 @@ SqlUserPlaylistProvider::deleteSqlPlaylists( Playlists::SqlPlaylistList playlist } } reloadFromDb(); + + return true; } Playlists::PlaylistPtr diff --git a/src/playlistmanager/sql/SqlUserPlaylistProvider.h b/src/playlistmanager/sql/SqlUserPlaylistProvider.h index 44547f2..f795618 100644 --- a/src/playlistmanager/sql/SqlUserPlaylistProvider.h +++ b/src/playlistmanager/sql/SqlUserPlaylistProvider.h @@ -58,7 +58,7 @@ class AMAROK_EXPORT SqlUserPlaylistProvider : public UserPlaylistProvider int trackIndex ); /* UserPlaylistProvider functions */ - virtual void deletePlaylists( Playlists::PlaylistList playlistlist ); + virtual bool deletePlaylists( Playlists::PlaylistList playlistlist ); virtual void rename( Playlists::PlaylistPtr playlist, const QString &newName ); Playlists::SqlPlaylistGroupPtr group( const QString &name ); @@ -80,7 +80,7 @@ class AMAROK_EXPORT SqlUserPlaylistProvider : public UserPlaylistProvider void checkTables(); void loadFromDb(); - void deleteSqlPlaylists( Playlists::SqlPlaylistList playlistlist ); + bool deleteSqlPlaylists( Playlists::SqlPlaylistList playlistlist ); Playlists::SqlPlaylistList selectedPlaylists() const { return m_selectedPlaylists; }