Bug 365363

(In reply to Wolfgang Bauer from comment #18)
> Rather sounds like a problem with actually saving the selected icon theme to
> kdeglobals to me...

In fact, it saves the selected icon theme to ~/.config/kdeglobals properly right after applying it from SS -> Icons KCM. It's very easy to check...

(In reply to Pulfer from comment #1)
> In fact, it saves the selected icon theme to ~/.config/kdeglobals properly
> right after applying it from SS -> Icons KCM. It's very easy to check...

I cannot reproduce this here on openSUSE with Plasma 5.7.0/Frameworks 5.24.0/Qt 5.6.1, and I have no idea how to debug this.

(In reply to Wolfgang Bauer from comment #2)
> I cannot reproduce this here on openSUSE with Plasma 5.7.0/Frameworks
> 5.24.0/Qt 5.6.1

Hm, actually I can.

It seems that the icon module isn't correctly loading the setting then if you just re-enter it without quitting systemsettings5 completely.
Maybe that's related to the fact that running applications don't notice icon theme changes, this only affects newly started apps, i.e. maybe kcm_icons doesn't actually read the config file but rather asks kiconthemes what the current theme is (just guessing though, haven't looked at the code).

I don't see "messed up icons" or a "damaged" icon cache though.

Created attachment 100052 [details]
Debug output for KIconLoaderPrivate::insertCachedPixmapWithPath

(In reply to Wolfgang Bauer from comment #3)
> I don't see "messed up icons" or a "damaged" icon cache though.

You can apply the attached debug patch to kiconthemes and then start systemsettings from terminal. You will see that System Settings keeps inserting icons from the theme it was started with even after icon theme was changed (and back button was pressed).

(In reply to Wolfgang Bauer from comment #3)
> Maybe that's related to the fact that running applications don't notice icon
> theme changes, this only affects newly started apps, i.e. maybe kcm_icons
> doesn't actually read the config file but rather asks kiconthemes what the
> current theme is (just guessing though, haven't looked at the code).

Indeed that's what it does:
- to "load" the settings it just calls KIconTheme::current()
- when saving, it writes it to kdeglobals, then deletes the icon cache, calls KIconTheme::reconfigure() and emits changed signals.

I suspect this is a problem in kiconthemes then, e.g. that it doesn't correctly reload the settings.

(In reply to Pulfer from comment #4)
> You will see that System Settings keeps
> inserting icons from the theme it was started with even after icon theme was
> changed (and back button was pressed).

I suppose that kiconthemes does that because it doesn't notice the settings change...

Ok, I did some debugging myself, and this is the problem:
    // Check application specific config for a theme setting.
    KConfigGroup app_cg(KSharedConfig::openConfig(QString(), KConfig::NoGlobals), "Icons");
    theme = app_cg.readEntry("Theme", QString());
    if (theme.isEmpty() || theme == QLatin1String("hicolor")) {
        // No theme, try to use Qt's. A Platform plugin might have set
        // a good theme there.
        theme = QIcon::themeName(); <---
    }
    if (theme.isEmpty() || theme == QLatin1String("hicolor")) {
        // Still no theme, try config with kdeglobals.
        KConfigGroup cg(KSharedConfig::openConfig(), "Icons");
        theme = cg.readEntry("Theme", QString());
    }
(that's in KIconTheme::current() )

If there is no application specific override, it tries to get the current theme from QIcon::themeName() before it reads kdeglobals.
But for some reason, QIcon::themeName() still returns the old one, i.e. the Plasma platform plugin apparently doesn't notice that the settings have been changed.

Removing that line ("theme = QIcon::themeName();") "fixes" the problem in systemsettings5.
I'm not sure that's the correct fix though, as it will prefer kdeglobals also on other desktops.
Maybe the platform plugin should be fixed/improved instead...

Anyway, reassigning to kiconthemes for now, it's not a bug in systemsettings/kcm_icons IMHO. Though maybe plasma-integration would be the better component, I'm not sure.

Created attachment 102826 [details]
Reset QIcon's assigned icon theme along with KDE global config

I wish I'd seen this bug earlier, this is a good investigation into the issue.  I think I've reproduced this before but didn't get this far before I had to move on to other things.

The attached patch might help, if the cause of the issue is indeed in KIconTheme::current().  It resets Qt's assigned icon themes for programs running at the time of the theme change (this should be ensured by placing the update code within _k_refreshIcons, which should be called in each running KDE program at runtime when the icon theme is updated).

However I haven't had a chance to test thoroughly, so let me know if there are still problems.

(In reply to Michael Pyne from comment #7)
> The attached patch might help, if the cause of the issue is indeed in
> KIconTheme::current().

I'm pretty sure it is.

Although it may be preferable if the systemsettings module would not use KIconTheme::current() for the current settings as it only writes to kdeglobals (so IMHO it should also only read from kdeglobals).

I have a patch for that, but haven't submitted it yet though to not hide the underlying problem... ;-)

> However I haven't had a chance to test thoroughly, so let me know if there
> are still problems.

I will try it.
Thanks for the patch.

The patch works.

Running applications (including systemsettings5) now immediately use the new icon theme after you change it and click Apply. This should also prevent old icons being added to the cache AFAICT.

And as a result, the systemsettings module also preselects the new theme now if you close and reopen it (without closing systemsettings5 completely).

Thanks for the analysis and patch. If you think the patch could have negative side effects, I suggest to post a review request. Otherwise, please commit soon, so that we get some testing until the next KF5 release.

Git commit 39e3815c6fc03fc0792e6a3c61245b09a75dfebc by Michael Pyne.
Committed on 21/12/2016 at 01:03.
Pushed by mpyne into branch 'master'.

Inform QIconLoader also when the desktop icon theme is changed.

This change is needed because KIconTheme will defer to QIconLoader's
concept of the current icon theme in preference to our own global
config, if that current icon theme is set.

Qt normally leaves it unset but once it starts loading icons through our
platform plugin, it will remember which theme was in use.

This means when we change the configured icon theme, that we would
sometimes accidentally continue to load icons from the old icon theme
(the one remembered by Qt).

Thanks to Wolfgang Bauer for debugging the issue, narrowing the
potential causes of the bug was instrumental in fixing it!
FIXED-IN:5.30

M  +12   -1    src/kiconloader.cpp

https://commits.kde.org/kiconthemes/39e3815c6fc03fc0792e6a3c61245b09a75dfebc

(In reply to Michael Pyne from comment #7)
> Created attachment 102826 [details]
> Reset QIcon's assigned icon theme along with KDE global config

Michael, can you do this for kdelibs too?

(In reply to Pulfer from comment #12)
> Michael, can you do this for kdelibs too?
???
I doubt that this applies to kdelibs/Qt4 at all.

Actually it was a regression in Frameworks 5.23.0, when KIconTheme::current() was extended to try to get the icon theme from more places than just kdeglobals:
https://cgit.kde.org/kiconthemes.git/commit/?id=1425523dad0ba4f3acb9d0caa0ff772fd0a1a7e2
(this change hasn't been backported to the KDE4 version, and I'm not sure it would even make sense for Qt4)

I just tried here and I see nothing wrong with kdelibs anyway:
- Changing the icon theme in systemsettings(4) applies it to (running) KDE4 applications immediately (including systemsettings)
- Changing the icon theme in systemsettings5 forwards the change to KDE4 applications too (also running ones).

(tested with Frameworks 5.31.0, Plasma/systemsettings5 5.9.3, kdelibs 4.14.30)

(In reply to Wolfgang Bauer from comment #13)
> I just tried here and I see nothing wrong with kdelibs anyway:
> - Changing the icon theme in systemsettings(4) applies it to (running) KDE4
> applications immediately (including systemsettings)

But sometimes it leaves old icons in cache, resulting mix of old and new theme icons.

Deleting /var/tmp/kdecache-$USER/icon-cache.kcache from startkde fixes this issue but only after re-login. Deleting icon-cache.kcache from the running KDE 4 session doesn't help.

I didn't try to investigate why it happens yet. The issue applies only to KDE 4 session. In Plasma 5 session changing icons works perfectly for both KDE4 and KF5 applications.

Well, maybe it's a wontfix issue anyway because KDE 4 Workspace is obsolete and no longer supported in upstream.

Created attachment 104649 [details]
KDE Workspace 4.11.22 + dolphin-4.14.3 + kdelibs 4.14.29 with messed up icons

KDE Workspace 4.11.22 + dolphin-4.14.3. First icons theme was Rosa, then it was changed to Breeze. Even after reboot icons in Dolphin are messed up (Breeze for toolbar and Rosa for folders).

Deleting the icon-cache on-disk only ensures that a new icon cache is created for newly-run applications.  It doesn't, by itself, invalidate any existing icons in any icon caches already existing somewhere in the desktop session in running applications.

Fixing those icons is supposed to be handled separately (some kind of IPC message sent by the kcm to all running apps telling them that there's been a theme or style update, which should cause them to flush their individual icon caches among other fixes).

That's always seemed to work for me but if it doesn't work for you then maybe then will help with debugging the problem.

(In reply to Michael Pyne from comment #16)
> That's always seemed to work for me but if it doesn't work for you then
> maybe then will help with debugging the problem.

I remember it worked for me in past too. But something has changed since that and I haven't figured out what exactly.

Some more info:
1. Close all dolphin windows
Run kcmshell4 icons from konsole, change icon theme
Start dolphin from konsole, see new icon theme properly applied

2. Close all dolphin windows
Run kcmshell4 icons from konsole, change icon theme
Start dolphin either from kicker or Alt+F2, see old cached icon theme. Old cache is NOT replaced.

3. Start dolphin
Run kcmshell4 icons from konsole, change icon theme
Switch to dolphin window, icons are still old but they change to new theme on entering directories etc. Old cache is replaced.

If I modify /usr/share/applications/kde4/dolphin.desktop this way:
-Exec=dolphin %i -caption %c %u
+Exec=strace dolphin %i -caption %c %u

starting dolphin from kicker or Alt+F2 works like if it was started from konsole (all icons are properly refreshed, old cache is not used).

Your help with debugging this issue would be appreciated :-)

P.S. Versions used for testing:
KDE Workspace 4.11.22 + dolphin-4.14.3 + kdelibs 4.14.29
KDE Workspace 4.11.22 + dolphin-15.04.3 + kdelibs 4.14.29

I also can reproduce this issue in RHEL 7 (kdelibs-4.14.8 + kde-workspace-4.11.19 + kde-baseapps-4.10.5).

And it looks like it's not exactly a cache issue. Something seems to pass incorrect current icon theme to Dolphin when it's run from Kicker/Alt+F2.

I suspect the behavior you're seeing when running another Dolphin instance from KRunner (not kicker ;)) is related to an optimization enabled by default in KDE 4 that is supposed to speed up startup of KDE apps (by routing startup through an always-running kdeinit that clones another copy of itself and morphs into the right application).

If this is the problem, then running your KDE desktop with the env var "KDE_IS_PRELINKED" set to 1 should cause Dolphin to work with icon theme changes even when run from KRunner. (This env var disables the kdeinit trick)

(In reply to Michael Pyne from comment #19)
> If this is the problem, then running your KDE desktop with the env var
> "KDE_IS_PRELINKED" set to 1 should cause Dolphin to work with icon theme
> changes even when run from KRunner. (This env var disables the kdeinit trick)

Thanx for the hint, it fixes the issue indeed. :-) Should I add KDE_IS_PRELINKED export to startkde script in my distro's package? Or maybe you can fix kdeinit to handle icon theme changes even if KDE_IS_PRELINKED is not set to 1?

I'd recommend instead adding the env variable to your system startup scripts using whatever method is appropriate (/etc/local.d or something like that?).  You don't want to manually edit the distro startkde if you can avoid it.

As far as fixing it, that's rather far outside of what I will be able to accomplish unless the cause is already fully identified/diagnosed, especially given that it's KDE4. :(

(In reply to Michael Pyne from comment #21)
> As far as fixing it, that's rather far outside of what I will be able to
> accomplish unless the cause is already fully identified/diagnosed,
> especially given that it's KDE4. :(

Thanx for your help anyway :-)