Bug 194549

Hi,

If you drag the song to the playlist pane instead of that middle pane thing, Amarok does what you want.

Solved since quite some time, current Amarok is 2.3.0, 2.3.1 is to be released next week.

I can reprocude this perfectly since ages and also still in git master:

Steps to reproduce:
1. make sure Amarok is stopped
2. drag and drop a song from media sources to the popup dropper "add to playlist" entry

Result:
Song gets appended and starts playing.

Expected:
Song gets appended and that's it.

I'm assuming the playlist is empty when you do this?

The bug can also be seen when there are already songs in the playlist.

This bug still happens in Amarok 2.3.1.

This is an automated message from the triager:

Amarok 2.4.1 has been released on May 8 already. Could you please upgrade and test if you can still reproduce this bug?

Without feedback within a month we will close this bug as resolved.

Thank you for your understanding.

This still happens in Amarok 2.4.1. In my oppinion or Amarok does not play anything or it plays the song that has just been added. There is no point in playing a song that user has not selected.

Thank you for the fast feedback :)

I have found the problem with this but wanted to confirm,

There are two modes :
Dynamic Mode and Normal Mode.

When in Normal mode ( and no is song is currently being played ) , whether or not it is in pause clicking "add to playlist" ( from right click ) or drag and drop makes the new song to start playing.

When in Dynamic mode ( and no song is being played ) , if it is not paused ( meaning nothing in queue ) new song starts playing, and if it is in pause, it gets appended to playlist.

My change :

 Keep rest same, but in Normal mode, when song is paused i will make it to append to playlist when we click " add to playlist ".

Is this ok?

Created attachment 69055 [details]
Patch to do this wish. ( add only if required ).

Well, what I would like is what the title of this bug says: Amarok do not try to  play anything when I am filling the playlist, just append the song. I want to device when Amarok should start playing and from each song. Nowadays Amarok plays a random song.

(In reply to comment #12)
  Yes once check my attachment. It will append to the playlist when you click add to playlist. But if no song is playing, it will start it.

Created attachment 69057 [details]
Second patch. ( never starts playing, just appends to the playlist) .

1) Applying both patches - Never start playing automatically, only append to playlist.

2) Applying only first patch - Play if no song is being played, append if song is paused.

Hope this is fine.

@Parthasarathy: Please submit the patch to reviewboard.kde.org and take the discussion about code to the amarok-devel@kde.org mailinglist, the bug report is not the place to do that.

*** Bug 308764 has been marked as a duplicate of this bug. ***

For me it seems to be a feature:
If nothing is playing then "append to playlist" also starts it.

Personally this is too confusing for me. I would have clean and simple behaviour, but this bug entry is not the place to discuss this. 
The correct place would be reviewboard or the developer mailing list.

For now I am closing the bug as invalid.

(In reply to comment #18)
> For me it seems to be a feature:
> If nothing is playing then "append to playlist" also starts it.

I do not agree with that, as far as I know no other song player does that. If someone is appending several songs that does not mean he/she wants to play right away or the first song added. Also the song played is not always the song being appended, that is really annoying when I in fact want to play the song I am appending to the playlist. That happens to me when playlist sorting is enabled and the song is inserted in the middle of the list instead of at the end.

I agree.
But somebody consciously implemented the behaviour.
I just checked the code and it seems that is is distributed at four different places and in all places it's doing an AppendAndPlay or something similar.
The code goes back to 2008.

So again, definitely not a bug but I agree with you. There is a mailing list for this kind of discussion.

(In reply to comment #20)
> So again, definitely not a bug but I agree with you. There is a mailing list
> for this kind of discussion.

I still do not agree with that. Playing a song the user has never asked to do so is indeed a bug, there is no reasonable explanation for that. Auto-playing any song when appending to playlist is also a bad feature in my oppinion, it should be optional at most.

Sending this discussion to the mailing list will just make it disappear in the limbo that mailing lists usually are. Developers pay more attention to opened bugs than to a mail of somebody complaining about a feature that the developers do not care about (this bug has been opened for almost four years, so I guess most Amarok developers do not care much about it).

I agree that we should change the behaviour, but at all places consistently and probably not yet for 2.7.

Lamarque, on the other hand, your behaviour kinda surprises me, as you are a free software developer, too, and you must have faced users on bugreports that "are right" no matter what you say to them.

As Ralf correctly pointed out, the behavioural change should be discussed on a mailing list, so that all developers have a chance to participate. amarok-devel for sure isn't in a limbo, and behaviour change suggestion certainly isn't (or: should not be) a complaint.

(In reply to comment #22)
> I agree that we should change the behaviour, but at all places consistently
> and probably not yet for 2.7.

That's ok for me, this is not a show stopper.
 
> Lamarque, on the other hand, your behaviour kinda surprises me, as you are a
> free software developer, too, and you must have faced users on bugreports
> that "are right" no matter what you say to them.

Do you agree with me that Amarok playing a song that the user has never asked to play is indeed a bug? I can acknowledge obvious bugs in my code, I do that all time in Plasma NM even when the code is not mine and even if the reporter is very rude to me. I can point a bug entry where the guy were very rude and yet I acknowledged the problem and fixed it. The guy even thanked me after I fixed it. Here I am just pointing out that this is indeed a bug and this entry should not be closed until it is fixed. I also close bugs as invalid in a regular basis, but I would never close this one as invalid.
 
> As Ralf correctly pointed out, the behavioural change should be discussed on
> a mailing list, so that all developers have a chance to participate.
> amarok-devel for sure isn't in a limbo, and behaviour change suggestion
> certainly isn't (or: should not be) a complaint.

Ok, discussion is good but why close this bug as invalid if it is indeed a bug? After the discussion you will ended up with another bug entry similar to this one, right? There is also two patches attached to this bug that could be used as base for the fix, they will be lost if this bug entry remains closed.

Right.

Git commit a43e7e6f5a14307f543e7807a8d2351af027635a by Matěj Laitl.
Committed on 23/05/2013 at 18:54.
Pushed by laitl into branch 'master'.

Make playlist-related actions consistent throughout Amarok code (behaviour change)

This commits boasts a couple of changes, starting with the
uncontroversial ones:

 1. The Playlist::AddOptions enum is extended with extended with
"convenience consistency" aliases:
    OnDoubleClickOnSelectedItems
    OnMiddleClickOnSelectedItems
    OnReturnPressedOnSelectedItems
    OnPlayMediaAction
    OnAppendToPlaylistAction
    OnReplacePlaylistAction
    OnQueueToPlaylistAction

...and all callers of PlaylistController::insertOptioned() are modified
to use one of these values instead of the "low-level" flags like
DirectPlay that are actually tested for in the insertOptioned()
implementation. This serves that we remain consistent across Amarok
from now on.

 2. The actual "low-level" enum values have been changed and
insertOptioned() was updated accordingly:
    a) PrependToQueue, which implies Queue, was added.
    b) StartPlay (start playing unless something is already playing)
       was removed. No caller uses it anymore (see below) and this was
       convoluted anyway, IMO.
    c) DirectPlay now implies PrependToQueue. This may seem strange,
       but the rationale is following: when you directplay just one
       track (which is the case 90% of the time), it is played
       immediately, and this should apply even when you add more
       tracks. PrependToQueue makes this possible without hacks and is
       invisible in case of just one track, because it is immediately
       popped from the queue. Plus it has a positive side-effect of
       inserting the track at a meaningful place (affects what track is
       played next).
    d) LoadAndPlay and LoadAndPlayImmediatelly were removed, because
       they were replaces with consistency aliases.

 3. Thanks to 2b), 2c) and implementation changes, the actual action
    performed upon a certain trigger no longer depends on any state.
    The state of playlist search no longer affects whether a track will
    be played in case of DirectPlay.

 4. insertOptioned() was cleaned up and changed, for example it tries
    to choose the best place to insert tracks according to
    PrependToQueue or Queue.

 5. The convenience aliases were assigned as follows:

    OnDoubleClickOnSelectedItems = OnReturnPressedOnSelectedItems =
    = OnPlayMediaAction = DirectPlay.
    OnMiddleClickOnSelectedItems = OnAppendToPlaylistAction = Append (0).
    OnReplacePlaylistAction = Replace (no-brainer).
    OnQueueToPlaylistAction = Queue (no-brainer).

    These aren't of course set in stone, they were however chosen to be
    as much consistent with other KDE apps as possible.

Especially the "DirectPlay implies PrependToQueue" change is a bit
controversial, my opinion in that matter is anything but strong and I'm
open to any discussion. But perhaps try to use it for a couple of days
to get over the barrier of change.

CHANGES:
 * Playlist-related actions were harmonized: double-clicking, pressing
   enter or using any "play media" action will prepend tracks to queue
   and immediately start playing; middle-clicking appends to playlist;
   append or replace actions will no longer start playback.

CCMAIL: amarok-promo@kde.org
CCMAIL: amarok-devel@kde.org
Related: bug 145468, bug 145490
FIXED-IN: 2.8
GUI: Behavioural change in some places, to increase consistency. Please
check that the docs don't mention the old behaviour, see CHANGES.
DIGEST: Amarok harmonizes playlist-related actions (double-clicking,
pressing Enter, middle clicking...)

M  +4    -0    ChangeLog
M  +4    -5    playground/src/context/applets/coverbling/CoverBlingApplet.cpp
M  +1    -1    playground/src/context/applets/coverbling/CoverBlingApplet.h
M  +1    -1    playground/src/context/applets/covergrid/AlbumItem.cpp
M  +5    -8    src/App.cpp
M  +5    -5    src/MainWindow.cpp
M  +1    -1    src/amarokurls/BookmarkTreeView.cpp
M  +8    -11   src/browsers/CollectionTreeView.cpp
M  +1    -1    src/browsers/CollectionTreeView.h
M  +7    -7    src/browsers/filebrowser/FileView.cpp
M  +1    -1    src/browsers/playlistbrowser/DynamicView.cpp
M  +10   -12   src/browsers/playlistbrowser/PlaylistBrowserView.cpp
M  +3    -3    src/browsers/playlistbrowser/PlaylistBrowserView.h
M  +12   -6    src/context/applets/albums/AlbumsView.cpp
M  +2    -1    src/context/applets/albums/AlbumsView.h
M  +2    -2    src/context/applets/similarartists/ArtistWidget.cpp
M  +2    -1    src/dbus/mpris1/TrackListHandler.cpp
M  +1    -1    src/dbus/mpris2/MediaPlayer2Player.cpp
M  +63   -48   src/playlist/PlaylistController.cpp
M  +19   -7    src/playlist/PlaylistController.h
M  +0    -1    src/playlist/PlaylistModel.cpp
M  +12   -18   src/playlist/view/listview/InlineEditorWidget.cpp
M  +9    -9    src/playlist/view/listview/PrettyListView.cpp
M  +1    -1    src/playlistgenerator/Preset.cpp
M  +2    -2    src/scriptengine/AmarokPlaylistScript.cpp
M  +1    -1    src/services/amazon/AmazonStore.cpp
M  +1    -1    src/services/lastfm/LastFmService.cpp
M  +3    -3    src/services/lastfm/LastFmTreeView.cpp
M  +1    -1    src/services/lastfm/SimilarArtistsAction.cpp

http://commits.kde.org/amarok/a43e7e6f5a14307f543e7807a8d2351af027635a

Git commit 71a0e27a7aa1899f063affd4854abf650978887b by Matěj Laitl.
Committed on 27/06/2013 at 10:40.
Pushed by laitl into branch 'master'.

Finally harmonize the double-click and other playlist-related actions

CHANGES:
 * Playlist-related actions were harmonized, double-clicking or pressing enter will
   append tracks to playlist, middle-clicking or using any "play media" action will
   prepend tracks to queue and immediately start playing; append or replace actions will
   no longer start playback.

Funnily enough, this is exactly what Myriam suggested from the start, but
wasn't understood by me, because her Amarok behaved differently than mine
(how so?) and we both referred to that behaviour.

The CHANGES above are current state vs. Amarok 2.7.1 and ignore any
intermediate steps.
Related: bug 145468, bug 145490
FIXED-IN: 2.8
GUI: revisit playlist-related actions, the changes are more subtle now, see CHANGES
CCMAIL: amarok-devel@kde.org
CCMAIL: amarok-promo@kde.org

M  +4    -4    ChangeLog
M  +3    -3    src/playlist/PlaylistController.h

http://commits.kde.org/amarok/71a0e27a7aa1899f063affd4854abf650978887b