----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100158/#review377 -----------------------------------------------------------
Ship it! Ah, using the actions certainly is a nice move. The removeRow path for deleting playlistbrowser items is a tricky one, so why not avoid it when possible. Thanks for the fix. - Bart On 2010-11-20 14:33:49, Dennis Francis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100158/ > ----------------------------------------------------------- > > (Updated 2010-11-20 14:33:49) > > > Review request for Amarok. > > > Summary > ------- > > This patch fixes the bug 250746 which causes a crash while using 'delete' key > to delete multiple playlists. > When multiple playlists are selected and deleted using delete key, only the > first one in the selectedIndexes() gets row removed properly. > > I used an alternative route to fix the issue, by using actionsFor() method > for getting the actions for the selected > indexes(selected playlists) and then triggering the last action of the > returned ActionList (The last action was found to be the one corresponds to > row remove operation). > > QTreeView is expanded to depth 0. This resolves 'delete key press' version of > bug 245646. Context menu version of this bug can > also be resolved in a similar way through another patch. > > Further, this patch automatically fixes bug 250750 - "Cancel delete of > playlist initialised by 'Delete' key not clean". Now when you press cancel, > none of the playlists are touched. > > > This addresses bugs 250746 and 250750. > https://bugs.kde.org/show_bug.cgi?id=250746 > https://bugs.kde.org/show_bug.cgi?id=250750 > > > Diffs > ----- > > src/browsers/playlistbrowser/PlaylistBrowserView.cpp 2603e2f > src/core-impl/podcasts/sql/SqlPodcastProvider.cpp ae61504 > src/playlistmanager/file/PlaylistFileProvider.cpp 573cdff > src/playlistmanager/sql/SqlUserPlaylistProvider.cpp 12da6ed > > Diff: http://git.reviewboard.kde.org/r/100158/diff > > > Testing > ------- > > Tested with different possible selection patterns using "CTRL+click". Delete > key press deletes only the selected playlists when confirmed, else none of > them are affected. The PlayListBrowserView maintains the same expanded state > after deletion. > Things seems to work fine for me. > > > Thanks, > > Dennis > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel