Bug 372964 - At least one Oxygen icon in Krusader is not correctly seen (using Kubuntu 16.04)
Summary: At least one Oxygen icon in Krusader is not correctly seen (using Kubuntu 16.04)
Status: RESOLVED FIXED
Alias: None
Product: krusader
Classification: Applications
Component: general (show other bugs)
Version: Git
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Krusader Bugs Distribution List
URL:
Keywords:
: 381676 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-11-26 18:02 UTC by Toni Asensi Esteve
Modified: 2018-05-06 00:16 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
Krusader in Plasma 5 using Breeze icons, that's ok. (4.62 KB, image/png)
2016-11-26 18:02 UTC, Toni Asensi Esteve
Details
Krusader in Plasma 4 using Oxygen icons, that's ok. (9.35 KB, image/png)
2016-11-26 18:03 UTC, Toni Asensi Esteve
Details
Krusader in Plasma 5 using Oxygen icons, but at least the penultimate is not correct. (7.77 KB, image/png)
2016-11-26 18:05 UTC, Toni Asensi Esteve
Details
Less important information: all the icons using Plasma 5 and Breeze icons (22.57 KB, image/png)
2016-11-26 18:07 UTC, Toni Asensi Esteve
Details
Less important information: all the icons using Plasma 5 and Oxygen icons (32.84 KB, image/png)
2016-11-26 18:07 UTC, Toni Asensi Esteve
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Toni Asensi Esteve 2016-11-26 18:02:28 UTC
Created attachment 102451 [details]
Krusader in Plasma 5 using Breeze icons, that's ok.

Hi!

When you use KDE Plasma 5 you can specify which icons you want to use (you can go to System Settings > Icons), when you use the Oxygen icons in Kubuntu 16.04: there is at least one icon in Krusader that is not correctly seen (for more detail: some screenshots are attached).
            
Thanks for Krusader!
Comment 1 Toni Asensi Esteve 2016-11-26 18:03:23 UTC
Created attachment 102452 [details]
Krusader in Plasma 4 using Oxygen icons, that's ok.
Comment 2 Toni Asensi Esteve 2016-11-26 18:05:53 UTC
Created attachment 102453 [details]
Krusader in Plasma 5 using Oxygen icons, but at least the penultimate is not correct.
Comment 3 Toni Asensi Esteve 2016-11-26 18:07:05 UTC
Created attachment 102454 [details]
Less important information: all the icons using Plasma 5 and Breeze icons
Comment 4 Toni Asensi Esteve 2016-11-26 18:07:56 UTC
Created attachment 102455 [details]
Less important information: all the icons using Plasma 5 and Oxygen icons
Comment 5 Toni Asensi Esteve 2016-11-26 18:37:20 UTC
Information about the icons:

In the source code of Krusader, in the viewactions.cpp file, we can see:
    actSelect = action(i18n("Select &Group..."), "edit-select", Qt::CTRL + Qt::Key_Plus, SLOT(markGroup()), "select group");
    actSelectAll = action(i18n("&Select All"), "edit-select-all", Qt::ALT + Qt::Key_Plus, SLOT(markAll()), "select all");
    [...]
    actUnselectAll = action(i18n("U&nselect All"), "edit-select-none", Qt::ALT + Qt::Key_Minus, SLOT(unmarkAll()), "unselect all");
and therefore we can see that each action has to have a related icon ("edit-select", "edit-select-all" and "edit-select-none"). 
            
It it's executed     
    $ find / -iname "*edit-select.*" 2>/dev/null        
    /usr/share/icons/breeze-dark/actions/22/edit-select.svg
    /usr/share/icons/breeze-dark/actions/16/edit-select.svg
    /usr/share/icons/breeze-dark/actions/24/edit-select.svg
    /usr/share/icons/breeze/actions/22/edit-select.svg
    /usr/share/icons/breeze/actions/16/edit-select.svg
    /usr/share/icons/breeze/actions/24/edit-select.svg
    /usr/share/icons/oxygen/16x16/actions/edit-select.png
    /usr/share/icons/oxygen/22x22/actions/edit-select.png
    /usr/share/icons/oxygen/48x48/actions/edit-select.png
    /usr/share/icons/oxygen/32x32/actions/edit-select.png
then we can see that there are Oxygen icons for "edit-select" (altough they show only a generic image of a mouse arrow, unlike its Breeze equivalent, which shows a mouse arrow and a distinctive little circle).

If using Kubuntu 16.04 it's executed
    $ find / -iname "*edit-select-all*" 2>/dev/null
    /usr/share/icons/Adwaita/scalable/actions/edit-select-all-symbolic.svg
    /usr/share/icons/Humanity/actions/48/edit-select-all.svg
    /usr/share/icons/Humanity/actions/22/edit-select-all.svg
    /usr/share/icons/Humanity/actions/16/edit-select-all.svg
    /usr/share/icons/Humanity/actions/24/edit-select-all.svg
    /usr/share/icons/breeze-dark/actions/22/edit-select-all.svg
    /usr/share/icons/breeze-dark/actions/16/edit-select-all.svg
    /usr/share/icons/breeze-dark/actions/16/edit-select-all-layers.svg
    /usr/share/icons/breeze-dark/actions/24/edit-select-all.svg
    /usr/share/icons/breeze/actions/22/edit-select-all.svg
    /usr/share/icons/breeze/actions/16/edit-select-all.svg
    /usr/share/icons/breeze/actions/16/edit-select-all-layers.svg
    /usr/share/icons/breeze/actions/24/edit-select-all.svg
    /usr/share/icons/oxygen/16x16/actions/edit-select-all.png
    /usr/share/icons/oxygen/22x22/actions/edit-select-all.png
    /usr/share/icons/oxygen/48x48/actions/edit-select-all.png
    /usr/share/icons/oxygen/32x32/actions/edit-select-all.png
then we can see that there are Oxygen icons for "edit-select-all".

It it's executed     
    $ find / -iname "*edit-select-none*" 2>/dev/null
    /usr/share/icons/breeze-dark/actions/22/edit-select-none.svg
    /usr/share/icons/breeze-dark/actions/16/edit-select-none.svg
    /usr/share/icons/breeze-dark/actions/24/edit-select-none.svg
    /usr/share/icons/breeze/actions/22/edit-select-none.svg
    /usr/share/icons/breeze/actions/16/edit-select-none.svg
    /usr/share/icons/breeze/actions/24/edit-select-none.svg
then we can see that there are not Oxygen icons for "edit-select-none".
Comment 6 Alex Bikadorov 2016-12-15 19:03:47 UTC
"edit-select-none" and "edit-select-invert" are only included in Breeze and not standard icon names (see https://specifications.freedesktop.org/icon-naming-spec/0.8/ar01s04.html).

Sooo.... what should we do? I really don't know.

* change the icon names to standard ones (i couldn't find something suitable)
* include these breeze icons as fallback in the Krusader package (will look ugly with other themes)
* do nothing (always a lazy option)

?
Comment 7 Alex Bikadorov 2017-07-06 18:24:13 UTC
*** Bug 381676 has been marked as a duplicate of this bug. ***
Comment 8 Alex Bikadorov 2017-11-12 17:30:10 UTC
@Toni
if you agree, I would like to set this report as a duplicate of Bug 372966. If this bug is about Krusader using icons only included in Breeze, this is what 372966 is about.
If there are icon-names is Oxygen which are somehow not used by Krusader it would be different though.
Comment 9 Toni Asensi Esteve 2017-11-14 19:06:50 UTC
Thank you for your comments, Alex!

> If this bug is about Krusader using icons only included in Breeze, this is what 372966 is about.

> If there are icon-names is Oxygen which are somehow not used by Krusader it would be different though.

After all that has been discussed, it seems that there are those problems:
    - Oxygen not having all the icons that Breeze has and Krusader uses (like "edit-select-none");
    - Oxygen having a "edit-select" icon that shows only a generic image of a mouse arrow, unlike its Breeze equivalent, which shows a mouse arrow and a distinctive little circle.

And maybe Krusader is doing a good job by not mixing Oxygen and Breeze icons. So this bug report would not be a 372966 duplicate, but a bug about the Oxygen icons and Krusader.

Additionally somebody else could think that it would be better that Krusader would include Breeze icons as fallback in the Krusader package (though it would look ugly with other themes), then somebody could write here his opinion.
Comment 10 Alex Bikadorov 2018-01-07 18:28:18 UTC
Well, if there are icons missing in Oxygen (or any other icon theme) which are needed by Krusader, it is more a general problem.

I have thought about solving this and Bug 372966:

Two assumptions I take for granted:
* there should never be missing icons for any actions/buttons in the UI. It simply should not occur in any modern application.
* mixing different icon sets in one application is just ugly and should also never happen. I can't even remember I have ever seen this anywhere.

And I am really against taking files from another project (Breeze) and copying them into our project. This is an ugly design decision and leads to more work: figuring out which icons we need, which icons are obsolete, which icons changed and need to be updated.

What's left is this (and only this) solution: We make Breeze an official dependency of Krusader. Because it is the only icon set which contains all needed icons. And we force using Breeze as icon set if it is installed in the code; regardless of what the user has set as icon set.
And optionally: Show a warning message on Krusader startup if Breeze is not installed. Because in this case icons will be missing and the user maybe does not know why.

Opinions?
Comment 11 Martin Kostolný 2018-01-07 19:18:55 UTC
> Opinions?

As for me, I agree, Alex. This sounds reasonable. We just have to make sure that both Breeze and Breeze Dark will work correctly, but that will probably be no problem.
Comment 12 Toni Asensi Esteve 2018-01-08 18:53:36 UTC
I agree, too!
Comment 13 Toni Asensi Esteve 2018-01-08 19:16:47 UTC
Just in case: a bug report about the Oxygen icons was created (https://bugs.kde.org/show_bug.cgi?id=388691)
Comment 14 Christoph Feck 2018-01-10 18:12:04 UTC
The issue is that Breeze uses Oxygen as fallback (because historically, Oxygen was considered 'complete', whereas Breeze was still evolving), but now the fallback order should get reversed.

Users that use the Oxygen icons (e.g. me) will probably report it as a bug that Krusader no longer respects their icon theme choice, but they will understand that Oxygen is no longer maintained, and with reversed fallback, they would see a minimal amount of Breeze icons, instead of no icons.
Comment 15 Ganton 2018-02-08 09:51:20 UTC
As Andreas Kainz has updated Oxygen icons (very good!), Breeze and Oxygen have all the icons that Krusader needs, there are more details in:
    https://bugs.kde.org/show_bug.cgi?id=388691
    https://commits.kde.org/oxygen-icons5/857400cb1a51f2287e4b6b285bba0f897066a6f7
Comment 16 Nikita Melnichenko 2018-04-22 22:03:46 UTC
Git commit 94ea5b69af14620abd4a44a13240ca9b2749641b by Nikita Melnichenko.
Committed on 22/04/2018 at 22:00.
Pushed by melnichenko into branch 'master'.

Refactored and unified icon rendering, implemented per-icon fallback logic

Merge branch 'fix-missing-icons'

The merged branch contains implementation of per-icon fallback logic:
1. Search icon in the active icon theme — if found, use it.
2. Search it in the icon theme that is specified in Krusader config — if found, use it.
3. Search it in Breeze or Oxygen (in case any of these installed) — if found, use it.
4. Otherwise use a default "icon-missing" icon bundled with Krusader.

FIXED: [ 372964 ] At least one Oxygen icon in Krusader is not correctly seen (using Kubuntu 16.04)
Related: bug 372966, bug 376699, bug 381676, bug 386554, bug 388691, bug 391899

FIXED: [ 372966 ] Some icons are not seen using a plain Ubuntu (not Kubuntu) 16.04

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

M  +11   -10   krusader/kractions.cpp
M  +2    -8    krusader/krusader.cpp

https://commits.kde.org/krusader/94ea5b69af14620abd4a44a13240ca9b2749641b