Bug 417069 - tab icons of remote urls needs resolving from UDSEntry '.' when available
Summary: tab icons of remote urls needs resolving from UDSEntry '.' when available
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: general (show other bugs)
Version: 19.12.2
Platform: Arch Linux Linux
: NOR minor
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-02-02 17:40 UTC by Patrick Silva
Modified: 2021-10-31 06:49 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
screenshot (22.93 KB, image/png)
2020-02-02 17:40 UTC, Patrick Silva
Details
screenshot-with-info-pane (87.23 KB, image/png)
2020-02-21 12:21 UTC, Harald Sitter
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Silva 2020-02-02 17:40:09 UTC
STEPS TO REPRODUCE
1. open Dolphin
2.  open a samba  share in a new tab
3. 

OBSERVED RESULT
as we can see in the attached screenshot, the tab of the samba share has the "unknown" icon.

EXPECTED RESULT
I'm not sure. Maybe a folder icon, or an specific icon to represent a samba share and other kinds of remote locations.

SOFTWARE/OS VERSIONS
Operating System: Arch Linux 
KDE Plasma Version: 5.17.90
KDE Frameworks Version: 5.66.0
Qt Version: 5.14.1
Comment 1 Patrick Silva 2020-02-02 17:40:32 UTC
Created attachment 125622 [details]
screenshot
Comment 2 Kai Uwe Broulik 2020-02-03 08:58:11 UTC
Seems to be a general pattern, search also has unknown icon. I think KIO::iconNameForUrl needs to do the fallback to protocol icon in all cases.
Comment 3 Harald Sitter 2020-02-21 12:21:56 UTC
Created attachment 126259 [details]
screenshot-with-info-pane

While I agree with Kai, I think there's something else going on here as well. This isn't just a case of we-dont-know the icon. We absolutely do. The only time me may not is when you browse smb:// anything below that we can translate to a dir-like UDSEntry and by extension we know what icon to use, no?

e.g. when you navigate into a directory in a share you still have the questionmark icon despite the dolphin info pane actually showing it is a folder: see screenshot-with-info-pane
Comment 4 Harald Sitter 2020-02-21 12:53:27 UTC
The bug happens because QMimeDatabase::mimeTypeForUrl is dumb WRT urls

```
If the URL is a local file, this calls mimeTypeForFile.

Otherwise the matching is done based on the file name only, except for schemes where file names don't mean much, like HTTP. This method always returns the default mimetype for HTTP URLs, use QNetworkAccessManager to handle HTTP URLs properly.

A valid MIME type is always returned. If url doesn't match any known MIME type data, the default MIME type (application/octet-stream) is returned.
```

This is a cost-vs-visual kinda trade off I guess. Inside KIO::iconNameForUrl we probably shouldn't ask the slave for the mimetype in the interest of speed, but then we can't get accurate mimetype lookup which sucks.

KIO actually falls back to protocol already, but the if conditions is flawed. mimeTypeForFile always returns application/octet-stream which always maps to unknown by default. "i" is then always non-empty rendering the condition useless

```
        if (i.isEmpty()) {
            i = KProtocolInfo::icon(url.scheme());
        }
```

All that being what it is. There are bugs in multiple places here...

- KIO needs a fix for the condition at the very least

But even then the icon lookup isn't working correctly. As I say, we know this is a directory, dolphin just needs to put that knowledge to use. 

- Dolphin needs to only use iconFromUrl until listDir has completed. Afterwards it needs to use the '.' UDSEntry and determine the icon from that (UDS_FILE_TYPE or UDS_ICON; I think KFileItem abstracts that?).

I'll bounce the bug back to dolphin and deal with the KIO defect right now.
Comment 5 Méven Car 2020-02-21 12:57:21 UTC
Patch proposal: https://phabricator.kde.org/D27539
Comment 6 Harald Sitter 2020-02-21 13:29:12 UTC
Méven already has a diff up for the KIO side: https://phabricator.kde.org/D27539
Comment 7 Méven Car 2020-02-22 11:35:53 UTC
Git commit eb20176d1a42eebd4c6c3cbdb5dca6a854760736 by Méven Car.
Committed on 22/02/2020 at 11:35.
Pushed by meven into branch 'master'.

KIO::iconNameForUrl: fix searching for kde protocol icons

Summary:
iconName was set to mt.iconName() at the beginning, but this returns always "application-octet-stream" making all subsequent iconName.isEmpty() check not work as expected.
This patch sets iconName to mt.iconName() only when needed.
Related: bug 417922, bug 417921
FIXED-IN: 5.68

Test Plan:
In dolphin:
 - open "Recent Files"
 - Ctrl+T

 - open any folder
 - Ctrl+T
 - Ctrl+F (Search
 - Search for anything, type something then return

Same for smb:/ location

In all cases the protocol icon is now visible.

Reviewers: ngraham, #frameworks, dfaure, broulik, sitter

Reviewed By: ngraham, dfaure, sitter

Subscribers: sitter, kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D27539

M  +13   -1    autotests/kfileitemtest.cpp
M  +14   -18   src/core/global.cpp

https://commits.kde.org/kio/eb20176d1a42eebd4c6c3cbdb5dca6a854760736
Comment 8 Patrick Silva 2021-09-04 11:47:58 UTC
I think we can close this report after the following fix

https://invent.kde.org/system/dolphin/-/merge_requests/124