playlist get_selected_paths() is used incorrectly

[Dustin Spicuzza]

In xlgui.widgets.playlist, the function get_selected_paths() returns the selected paths in the *current* tree model. The code misuses the paths in a number of different ways -- predominantly, it assumes that there is a direct correlation between playlist index and path number, which is only sometimes true. If the playlist is reordered using a column, or the search filter is used, then the indexes are no longer correct.

We need a method to translate paths to playlist indexes. However, there doesn't seem to be a good way to do this.

I'm too tired to think of a good way to fix this tonight, but we should definitely fix it before the release. It pops up in all kinds of unexpected ways.

[Dustin Spicuzza (dustin-virtualroadside)]

After some closer examination, a lot of things about the PlaylistModel are broken in very subtle ways, and only haven't shown up because playlists aren't typically modified by anything other than the PlaylistView. If that were to change, this bug would show up in a lot more places. Misuse of the tree path is just one of the ways the PlaylistModel is broken.

Another way that it is broken is the assumption that the position in a playlist can correspond to the position in the model, particularly by using insert(position, ...) and iter_nth_child(None, position). However, a simple test shows that the position is completely arbitrary and is not related to the playlist position at all. A way you can prove this to yourself is populate a playlist in the main window, and open up an IPython window in Exaile:

    import xlgui
    page = xlgui.main.get_selected_page()
    model = page.view.model
    # will show some track here
    model.get(model.iter_nth_child(None, 0), 0)

-> sort the playlist display using one of the columns, then run this again:

    # will show a different track here
    model.get(model.iter_nth_child(None, 0), 0)

In the interest of time, I recommend going ahead with the release without fixing this bug. Because we don't share playlist instances across boundaries, and most of the playlist stuff is modified using the model, this bug doesn't show up very often.

However, definitely needs to be fixed ASAP, as it introduces subtle bugs that a user may not notice right away, until they've done something to a playlist that won't be revertable. Due to limitations in the TreeModel in gtk, I believe that fixing this may require major changes to the PlaylistModel to ensure that they're synced properly.

[Dustin Spicuzza (dustin-virtualroadside)]

After considering the problem some more, it's not quite as bad as I originally thought. I've adjusted the paths so they work the way that one might expect them to work, and the following cases now work correctly when a filter is applied to the playlist view:

- Drag/drop
- Delete track (used to delete the wrong track)
- Get track properties

Fixed in trunk in r4372, on 3.3.x in r4324.