Bug 325012

Summary: Playlist reordering works only in one direction
Product: plasma-mediacenter Reporter: Shantanu Tushar <shantanu>
Component: PlaylistAssignee: Sinny Kumari <ksinny>
Severity: major    
Priority: NOR    
Version: 1.1.0   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: this is a patch to the /plasma-media-center/libs/mediacenter/playlistmodel.cpp

Description Shantanu Tushar 2013-09-17 17:38:17 UTC
Playlist reordering works only if you press-n-hold, drag and drop a media item downwards. If you do so upwards, the order remains the same.

Reproducible: Always

Steps to Reproduce:
1. Add more than 2 songs to playlist


2. Drag song1 below song2

Actual Results:  
song1 remains in its original position

Expected Results:  
song1 should go below song2 and the order should become

Comment 1 kaushik varanasi 2013-09-19 15:31:10 UTC
Created attachment 82410 [details]
this is a patch to the /plasma-media-center/libs/mediacenter/playlistmodel.cpp

the drag and drop feature worked only in one direction. I fixed the bug.
now the drag and drop feature is complete.
Comment 2 Shantanu Tushar 2013-09-19 15:34:51 UTC
Thanks for the patch. However, in the following code-

 void PlaylistModel::moveItem(int firstIndex, int secondIndex)
+	beginResetModel();
+    endResetModel();
     emit dataChanged(createIndex(firstIndex, 0), createIndex(secondIndex, 0));

the call to d->musicList.move is there to avoid calling beginResetModel() and endResetModel(). This is because reset'ing a model is expensive, and instead we should only call move.

My guess is that during upwards, the move() method works fine, but dataChanged() is somehow not working. Try to figure out why (maybe look at the values passed to dataChanged and whats the difference between upwards and downwards).
Comment 3 kaushik varanasi 2013-09-19 15:37:32 UTC
Created attachment 82411 [details]

in case my patch dosnt work, replace the /plasma-media-center/libs/mediacenter/playlistmodel.cpp file with this.

PS: as i said i am very new to open source so please excuse my amature behavior.
Comment 4 Shantanu Tushar 2013-09-23 15:28:26 UTC
Hey kaushik,
So as I mentioned, don't use reset and instead use dataChanged (as its already there). I think the problem is that lets say you pass firstIndex, secondIndex to dataChanged, it works only if firstIndex < secondIndex. I suspect that when reordering upwards, firstIndex becomes greater than secondIndex and hence the problem. Try putting a if condition when this happens and in that case reverse the order of firstIndex and secondIndex when calling dataChanged.

If it works, instead of adding a patch here, create a new review request at http://git.reviewboard.kde.org/ and upload your patch there. Add "plasma" in the group and publish your review. Then we can take a look at your patch :)
Comment 5 Shantanu Tushar 2014-03-06 14:20:36 UTC
Git commit 86f20ec9a746efb0add1de56e44bc5064de45a53 by Shantanu Tushar.
Committed on 06/03/2014 at 14:18.
Pushed by shantanu into branch 'master'.

Make playlist reordering work both the ways.

Patch by Ashish Madeti <ashishmadeti@gmail.com>
Welcome to KDE :)
REVIEW: 116626

M  +5    -1    libs/mediacenter/playlistmodel.cpp