Review Request 109817: JJ 313649 - No warning if there are no permissions to read file

[Matěj Laitl]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109817/#review30341
-----------------------------------------------------------


Thanks for the patch. While I understand the motivation behind it, could you describe it in more detail or link to a related bug?

Bee below for technical issues with the patch.


src/playlist/PlaylistModel.cpp
<http://git.reviewboard.kde.org/r/109817/#comment22608>

    This is way too specific and doesn't fit to abstraction Meta::Track provides. Note that here is Track::isPlayable()
    
    You should also show the notice *along* the original tooltip, as for example tracks from Local Collection can show metadata even thought the file ceased to be readable. (so that the changes fit more into tooltipFor() method)
    
    Also note that the notice is track-specific, MetaFile::Track may tell "File doesn't exist", while MetaStream::Track may want to tell "Network connection not available".
    
    Maybe it has sense to intoduce virutal Meta::Track::notPlayableReason(), but that would require acknowledge from other developers.



src/playlist/view/PlaylistViewCommon.cpp
<http://git.reviewboard.kde.org/r/109817/#comment22609>

    This is essentially an abuse of a UI element (an action) which isn't acceptable. Tooltip entry and gray-out should be enough.


- Matěj Laitl


On April 1, 2013, 9:20 p.m., Anmol Ahuja wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109817/
> -----------------------------------------------------------
> 
> (Updated April 1, 2013, 9:20 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Description
> -------
> 
> Added two indicators for no-read permission:
> 1. A tooltip "No read permissions" for tracks with no read permissions
> 2. Upon right-clicking such a track, displays a "No read permissions" disabled action.
> 
> 
> Diffs
> -----
> 
>   src/playlist/PlaylistModel.cpp 246b9a1 
>   src/playlist/view/PlaylistViewCommon.h 9582b1b 
>   src/playlist/view/PlaylistViewCommon.cpp e3cdd53 
> 
> Diff: http://git.reviewboard.kde.org/r/109817/diff/
> 
> 
> Testing
> -------
> 
> Works as expected
> 
> 
> Thanks,
> 
> Anmol Ahuja
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20130403/e6778a9a/attachment.html>