Bug 339193 - Icon overlays are inconsistent
Summary: Icon overlays are inconsistent
Status: RESOLVED FIXED
Alias: None
Product: kio
Classification: Unclassified
Component: general (show other bugs)
Version: 4.14.0
Platform: Other Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: David Faure
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-18 22:52 UTC by Stefan Brüns
Modified: 2018-05-08 17:32 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Dolphin screenshot (108.91 KB, image/png)
2014-09-18 22:56 UTC, Stefan Brüns
Details
Cleaned up overlays (107.42 KB, image/png)
2014-10-16 15:33 UTC, Stefan Brüns
Details
kdelibs 4.14.3, no "locked" overlay (77.79 KB, image/png)
2015-01-21 06:20 UTC, Pulfer
Details
kdelibs 4.14.4, "locked" overlay (78.06 KB, image/png)
2015-01-21 06:21 UTC, Pulfer
Details
Set correct permissions for entries in remote: (944 bytes, patch)
2015-02-20 18:54 UTC, Stefan Brüns
Details
Use correct default value if UDS_ACCESS, UDS_FILE_TYPE is not set (1.21 KB, patch)
2015-02-20 18:56 UTC, Stefan Brüns
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Brüns 2014-09-18 22:52:39 UTC
Dolphin may show several overlay icons for files/folders, but the behaviour is inconsistent:

1. Compressed files may have a "application-zip" overlay, but only if the mimetype is application/x-gzip and it has an ".gz" ending.
a) a .tar.gz will have the overlay
b) the same file named .tgz will not
c) a .tar.xz or .tar.b2 dont have the overlay either

2. if a file is a symlink, the link overlay will be inserted first, moving the "zip" overlay to the next position

3a) an unreadable (no read permission) file will have another overlay added, moving the "zip" overlay further
- but -
3b) unreadable folder has no overlay added (according to kfileitem.cpp " // Locked dirs have a special icon, use the overlay for files only", but that is incorrect).

The "zip" overlay should be removed, as compressed files already have distinct mimetype icons.
Folders should have the locked overlay, when unreadable.

Maybe the corners should be assigned specific functions.

Reproducible: Always
Comment 1 Stefan Brüns 2014-09-18 22:56:33 UTC
Created attachment 88747 [details]
Dolphin screenshot

"locked" files/folders are items without read permissions.
other file types can be deduced from file name.
Comment 2 Emmanuel Pescosta 2014-09-19 07:39:49 UTC
Thanks for this bug report! I can confirm the problems.

 > The "zip" overlay should be removed, as compressed files already have distinct mimetype 
> icons. 

Makes sense.

> Folders should have the locked overlay, when unreadable.

+1

>  Maybe the corners should be assigned specific functions.

Yep that's a good idea :)
Comment 3 Stefan Brüns 2014-10-16 15:33:52 UTC
Created attachment 89160 [details]
Cleaned up overlays

After applying RB:120605
Comment 4 Stefan Brüns 2014-11-10 22:34:00 UTC
Git commit e3e428375c864d8fb025460110bb1e6e742819d4 by Stefan Brüns.
Committed on 10/11/2014 at 22:33.
Pushed by bruns into branch 'master'.

cleanup overlay icon usage

The locked overlay should be added also for directories, as there is no
distinct "unreadable directory" icon.
No overlay icon for "gzip files with .gz file ending", there is a
mimetype icon for gzip files.
REVIEW: 120606

M  +1    -7    src/core/kfileitem.cpp

http://commits.kde.org/kio/e3e428375c864d8fb025460110bb1e6e742819d4
Comment 5 Stefan Brüns 2014-11-10 23:00:46 UTC
Git commit 9ba9651cec6c6b444db1b3ad72ea73552d4a3254 by Stefan Brüns.
Committed on 10/11/2014 at 22:59.
Pushed by bruns into branch 'KDE/4.14'.

cleanup overlay icon usage

The locked overlay should be added also for directories, as there is no
distinct "unreadable directory" icon.
No overlay icon for "gzip files with .gz file ending", there is a
mimetype icon for gzip files.

Backport of 877fdcd9fcbdf8267ce509d8918e5cb5a1a455aa from kio
REVIEW: 120606

M  +1    -7    kio/kio/kfileitem.cpp

http://commits.kde.org/kdelibs/9ba9651cec6c6b444db1b3ad72ea73552d4a3254
Comment 6 Christoph Feck 2014-12-12 23:28:26 UTC
Stefan, do your commits solve this bug?
Comment 7 Stefan Brüns 2014-12-13 04:37:01 UTC
(In reply to Christoph Feck from comment #6)
> Stefan, do your commits solve this bug?

The commit solves 2 out of 3 issues
- unreadable folders have the "locked" overlay
- no overlay for .gz files, use distinct mime type icons for compressed files.

This leaves the last point in the BR - corners have arbitrary functions for overlays, overlays may move.

To get the last issue solved, we have to evaluate:
1. which overlays are used today
2. which may be useful in the future
  a) we use a "shared folder" overlay for nfs/smb shares, but owncloud also has "shared files" [1])
  b) the KVersionControlPlugin shows "untracked",  "conflicting", "locally modified", ... but ownCloud also has a "syncing" state [2]
3. which overlays are used by other environments, e.g. GNOME, Unity, are these considered useful for KDE? [3]

[1] https://github.com/owncloud/client/tree/master/shell_integration/icons/256x256
[2] https://dragotin.wordpress.com/2014/12/08/overlay-icons-for-dolphin-in-owncloud-client/
[3] http://standards.freedesktop.org/icon-naming-spec/icon-naming-spec-latest.html#names - Emblems, Table 7

Maybe it is better to move the remaining point to a new BR, and close this one?
Comment 8 Pulfer 2015-01-20 09:24:37 UTC
(In reply to Stefan Brüns from comment #5)
> Backport of 877fdcd9fcbdf8267ce509d8918e5cb5a1a455aa from kio
> REVIEW: 120606
> 
> M  +1    -7    kio/kio/kfileitem.cpp
> 
> http://commits.kde.org/kdelibs/9ba9651cec6c6b444db1b3ad72ea73552d4a3254

After this commit all "remote:/" folders got locked overlay. Doesn't look like a good idea for me because those folders are not locked.
Comment 9 Stefan Brüns 2015-01-20 20:35:59 UTC
(In reply to Pulfer from comment #8)
> After this commit all "remote:/" folders got locked overlay. Doesn't look
> like a good idea for me because those folders are not locked.

Can you be more specific please? For me, all three entries in remote: look fine:

"Network", has link overlay, points to "network:/"
"Samba Shares", link overlay, points "smb:/"
"Add Network Folder", no overlay
Comment 10 Pulfer 2015-01-21 06:19:55 UTC
(In reply to Stefan Brüns from comment #9)
> (In reply to Pulfer from comment #8)
> > After this commit all "remote:/" folders got locked overlay. Doesn't look
> > like a good idea for me because those folders are not locked.
> 
> Can you be more specific please? For me, all three entries in remote: look
> fine:
> 
> "Network", has link overlay, points to "network:/"
> "Samba Shares", link overlay, points "smb:/"
> "Add Network Folder", no overlay

See attached screenshots. 

With kdelibs 4.14.3:
"network:/" - link overlay
"smb:/" - link overlay
"zeroconf:/" - link overlay
"mtp:/" - link overlay
"Add Network Folder" - no overlay

With kdelibs 4.14.4:
"network:/" - "link" overlay, "locked" overlay
"smb:/" - "link" overlay, "locked" overlay
"zeroconf:/" - "link" overlay, "locked" overlay
"mtp:/" - "link" overlay, "locked" overlay
"Add Network Folder" - no overlay

"Locked" overlay there is misleading.
Comment 11 Pulfer 2015-01-21 06:20:42 UTC
Created attachment 90560 [details]
kdelibs 4.14.3, no "locked" overlay
Comment 12 Pulfer 2015-01-21 06:21:04 UTC
Created attachment 90561 [details]
kdelibs 4.14.4, "locked" overlay
Comment 13 Christoph Feck 2015-02-14 23:28:19 UTC
I can reproduce comment #10 on my system.
Comment 14 Frank Reininghaus 2015-02-16 11:06:55 UTC
I can also reproduce it, but since it is caused by an upgrade of kdelibs according to comment 10, and I can also reproduce it in the file dialog, KIO might be the right product.
Comment 15 Stefan Brüns 2015-02-20 18:53:02 UTC
Actually there are two bugs causing this issue:

1. the "remote:" ioslave does not set the UDS_ACCESS key for any but the "Add network folder" entry.
2. KFileItem uses a default value of 0 (=^ inaccessible) for the permission value if initialized with an UDSEntry which has no UDS_ACCESS key.

See following two patches.
Either fixes the issue, but for correctness sake, both should be applied.
Comment 16 Stefan Brüns 2015-02-20 18:54:33 UTC
Created attachment 91190 [details]
Set correct permissions for entries in remote:
Comment 17 Stefan Brüns 2015-02-20 18:56:06 UTC
Created attachment 91191 [details]
Use correct default value if UDS_ACCESS, UDS_FILE_TYPE is not set
Comment 18 Emmanuel Pescosta 2015-02-20 19:40:54 UTC
(In reply to Stefan Brüns from comment #15)
> Actually there are two bugs causing this issue:
> 
> 1. the "remote:" ioslave does not set the UDS_ACCESS key for any but the
> "Add network folder" entry.
> 2. KFileItem uses a default value of 0 (=^ inaccessible) for the permission
> value if initialized with an UDSEntry which has no UDS_ACCESS key.
> 
> See following two patches.
> Either fixes the issue, but for correctness sake, both should be applied.

Thank you very much for debugging this problem! :)
Can you please upload your patches to reviewboard? Thanks!
Comment 19 Stefan Brüns 2015-02-24 12:22:30 UTC
(In reply to Emmanuel Pescosta from comment #18)
> Thank you very much for debugging this problem! :)
> Can you please upload your patches to reviewboard? Thanks!

https://git.reviewboard.kde.org/r/122652/
https://git.reviewboard.kde.org/r/122653/
Comment 20 Stefan Brüns 2015-03-19 02:02:51 UTC
Git commit 9eb37e029037b54936c6978037a9a108a2ca6458 by Stefan Brüns.
Committed on 20/02/2015 at 21:31.
Pushed by bruns into branch 'master'.

Set permissions for links in remote:, necessary for correct visualization

KFileItem uses UDS_ACCESS to determine permissions. Readability is
subsequently used to create the list of overlay icons.

M  +1    -0    kioslave/remote/remoteimpl.cpp

http://commits.kde.org/kde-runtime/9eb37e029037b54936c6978037a9a108a2ca6458
Comment 21 Stefan Brüns 2015-03-19 02:07:23 UTC
Git commit e44e2a7f35cf96b7304f22b1a5002e629d325e9d by Stefan Brüns.
Committed on 19/03/2015 at 02:05.
Pushed by bruns into branch 'master'.

Set permissions for links in remote:, necessary for correct visualization

KFileItem uses UDS_ACCESS to determine permissions. Readability is
subsequently used to create the list of overlay icons.

M  +1    -0    kioslave/remote/remoteimpl.cpp

http://commits.kde.org/plasma-workspace/e44e2a7f35cf96b7304f22b1a5002e629d325e9d
Comment 22 Stefan Brüns 2015-03-19 02:15:35 UTC
Git commit c4f6453f7f2ba0637fc925f6abe347b9c9c457a6 by Stefan Brüns.
Committed on 20/02/2015 at 20:56.
Pushed by bruns into branch 'KDE/4.14'.

Use correct default value when UDS_ACCESS/UDS_FILE_TYPE is not set

The default value for UDSEntry::numberValue(...) is 0, whereas KFileItem
uses the special value KFileItem::Unknown == (mode_t) -1.

M  +2    -2    kio/kio/kfileitem.cpp

http://commits.kde.org/kdelibs/c4f6453f7f2ba0637fc925f6abe347b9c9c457a6
Comment 23 Stefan Brüns 2015-03-19 02:20:00 UTC
Git commit 6215e878706dc6e79d1b6ac5df33f5751d6c7ec0 by Stefan Brüns.
Committed on 19/03/2015 at 02:19.
Pushed by bruns into branch 'master'.

Use correct default value when UDS_ACCESS/UDS_FILE_TYPE is not set

The default value for UDSEntry::numberValue(...) is 0, whereas KFileItem
uses the special value KFileItem::Unknown == (mode_t) -1.
REVIEW: 122652

M  +2    -2    src/core/kfileitem.cpp

http://commits.kde.org/kio/6215e878706dc6e79d1b6ac5df33f5751d6c7ec0
Comment 24 Nate Graham 2018-05-08 17:03:52 UTC
Stefan, is there anything left to do here?
Comment 25 Stefan Brüns 2018-05-08 17:24:25 UTC
Fixed in all relevant versions for a long time (see commit entries).
Comment 26 Nate Graham 2018-05-08 17:32:13 UTC
Great, thanks!