Bug 263693

Summary: JJ: The Delete Tracks dialog is misleading/ambiguous
Product: [Applications] amarok Reporter: Paul Allen <paul.l.allen>
Component: Collection BrowserAssignee: Amarok Developers <amarok-bugs-dist>
Status: RESOLVED FIXED    
Severity: minor CC: matej, paul.l.allen, ryan.mccoskrie
Priority: NOR Keywords: junior-jobs, usability
Version: 2.4-GIT   
Target Milestone: 2.4.1   
Platform: Unlisted Binaries   
OS: Linux   
Latest Commit: Version Fixed In: 2.6
Sentry Crash Report:

Description Paul Allen 2011-01-19 22:59:00 UTC 
Version:           2.3.2 (using KDE 4.5.4) 
OS:                Linux

In Amarok 2.3.2, if you right click on an artist in a collection on a music player and pick "Delete Tracks", you get a confirmation dialog with this text: "Do you really want to delete these tracks? They will be removed from disk as well as your collection.", along with a list of the pathnames that will be removed. Depending on the source of the mp3's, the names might be meaningful or complete gibberish.

The message is pretty clear that it is going to remove the tracks from two places, but the paths are all on the mounted player and not in the collection on my computer. Given that Amarok is, umm, not quite finished, what is it going to do? Until now, I didn't have the courage to find out.

In fact, the tracks are only removed from the player. The message is misleading. I suggest that the dialog ask, "Do you really want to remove these tracks from collection 'Foo'?" The distinction between removal from the "disk" and from the "collection" probably exists only in the mind of an Amarok developer and should not be exposed to users. If you're going to impose an abstraction called "collection" between the user and his storage devices, you need to stick strictly to it.

Reproducible: Always

Steps to Reproduce:
Setup: A large music collection on the computer, a subset on a portable player.
Select some tracks while browsing on the portable player.
Right-click and pick Delete Tracks.
Observe the resulting dialog.


Actual Results:  
The dialog appears to state that it will remove the tracks in *two* locations.  This could be interpreted to mean the portable device and the computer.  In fact, the tracks are only removed from the portable player, the correct behavior, but this is not clear beforehand.

Expected Results:  
A better message would say simply that the tracks will be removed from the collection.  Best would be to name the collection.

OS: Linux (x86_64) release 2.6.34.6-47.fc13.x86_64
Compiler: gcc
Portable player: iPod Video, iPod Classic
Comment 1 Myriam Schweingruber 2011-05-07 11:32:01 UTC 
Could you please upgrade to a newer Amarok version and test again? Current is Amarok 2.4.0, Amarok 2.4.1 is to be released tomorrow.

Please report back.
Comment 2 Myriam Schweingruber 2011-06-18 22:15:51 UTC 
Any news on this? Without feedback within 2 weeks I will close this report.
Comment 3 Paul Allen 2011-06-19 06:16:18 UTC 
In 2.4.1, the dialog uses "move to the trash" instead of "delete".  No significant change since 2.3.2.  The issue I reported has not been addressed.
Comment 4 Myriam Schweingruber 2011-06-19 15:42:33 UTC 
Thank you for the feedback.
Comment 5 Ryan McCoskrie 2011-08-05 11:19:30 UTC 
Just checked from the latest code. This inconstancy is still present. I'm familiarising my self with the relevant code for fixing it right now.
Comment 6 Myriam Schweingruber 2011-08-07 10:19:47 UTC 
Thank you Ryan. Please also make sure to get in touch with our developers either via #amarok on irc.freenode.net or by sending a mail to amarok-devel@kde.org.

You can submit your patch to http://reviewboard.kde.org (you will need an account on http://identity.kde.org first).
Comment 7 Bart Cerneels 2011-11-03 20:07:20 UTC 
Git commit c38f70b6ba2f3ddf1eecf66ce42aca8407b2db02 by Bart Cerneels.
Committed on 03/11/2011 at 21:05.
Pushed by shanachie into branch 'master'.

Make delete track dialog less confusing.

TODO: we should probably add the collection name in there but need some
refactoring.

Patch by Gilles Habran.

BUG:263693
FIXED-IN: 2.5
REVIEW: 103026

M  +2    -0    ChangeLog
M  +4    -4    src/core-impl/collections/support/CollectionLocationDelegateImpl.cpp

http://commits.kde.org/amarok/c38f70b6ba2f3ddf1eecf66ce42aca8407b2db02
Comment 8 Matěj Laitl 2011-11-05 19:52:46 UTC 
Bart, I think the fix is not complete. The original description said:

> In fact, the tracks are only removed from the player. The message is
> misleading. I suggest that the dialog ask, "Do you really want to remove these
> tracks from collection 'Foo'?" The distinction between removal from the "disk"
> and from the "collection" probably exists only in the mind of an Amarok
> developer and should not be exposed to users.

The reporter, Paul, made a good point here about minds of Amarok devs.

The message now is: "Do you really want to delete this track? It will be removed from your collection." and the "collection" word is hardcoded. It should much rather use collection->prettyName() instead, so that it gives nice "Local Collection" and "iPod" etc. for media devices.

https://git.reviewboard.kde.org/r/102236/ also deals with another aspect of the bug, I'll lok at it.
Comment 9 Matěj Laitl 2012-03-08 21:41:16 UTC 
Git commit e90844f6028017c571455dfdf4467c0d87449431 by Matěj Laitl.
Committed on 08/03/2012 at 22:36.
Pushed by laitl into branch 'master'.

Better "Really delete/trash/move" dialogs, include track title and artist

This patch is heavily based on Ryan McCoskrie's <ryan.mccoskrie@gmail.com>
work in the review request, cleaned up and mover further by me.
FIXED-IN: 2.6
REVIEW: 102236

M  +70   -68   src/core-impl/collections/support/CollectionLocationDelegateImpl.cpp
M  +6    -0    src/core-impl/collections/support/CollectionLocationDelegateImpl.h
M  +1    -1    src/core-impl/collections/support/TrashCollectionLocation.cpp

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