Bug 343583 - kcm_lookandfeel can't write LookAndFeelPackage if there are systemwide kdeglobals with same key
Summary: kcm_lookandfeel can't write LookAndFeelPackage if there are systemwide kdeglo...
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Global Theme packages (show other bugs)
Version: master
Platform: Other Linux
: NOR major
Target Milestone: 1.0
Assignee: Plasma Bugs List
URL:
Keywords:
: 343644 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-01-30 19:19 UTC by Hrvoje Senjan
Modified: 2015-02-09 16:08 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.2.1
Sentry Crash Report:


Attachments
attempt patch (2.12 KB, patch)
2015-02-04 09:46 UTC, Marco Martin
Details
Potential workaround for kcm_lookandfeel (833 bytes, patch)
2015-02-09 02:19 UTC, Matthew Dawson
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Hrvoje Senjan 2015-01-30 19:19:33 UTC
(maybe wrong product)

so:
1) have /etc/xdg/kdeglobals
2) have the following
[KDE]
LookAndFeelPackage=$somevalidlookandfeelpackage (not breeze though)
3) change the package in the kcm to org.kde.breeze.desktop
4)
[KDE]
LookAndFeelPackage=$somevalidlookandfeelpackage
is present in ~/.config/kdeglobals, instead of
[KDE]
LookAndFeelPackage=org.kde.breeze.desktop

5) remove the keys from systemwide kdeglobals, try again to apply, now local kdeglobals have correct value

Reproducible: Always
Comment 1 Hrvoje Senjan 2015-01-30 20:52:16 UTC
somehow this resolves the problem, but why, i do not know =(

diff --git a/kcms/lookandfeel/kcm.cpp b/kcms/lookandfeel/kcm.cpp
index 6b4bf68..c8398d7 100644
--- a/kcms/lookandfeel/kcm.cpp
+++ b/kcms/lookandfeel/kcm.cpp
@@ -238,7 +238,9 @@ void KCMLookandFeel::save()
         return;
     }
 
-    m_configGroup.writeEntry("LookAndFeelPackage", m_selectedPlugin);
+    m_config2 = KSharedConfig::openConfig("kdeglobals");
+    KConfigGroup group(m_config2, "KDE");
+    group.writeEntry("LookAndFeelPackage", m_selectedPlugin);
 
     if (!package.filePath("defaults").isEmpty()) {
         KSharedConfigPtr conf = KSharedConfig::openConfig(package.filePath("defaults"));
diff --git a/kcms/lookandfeel/kcm.h b/kcms/lookandfeel/kcm.h
index c082cd3..e2d0c7b 100644
--- a/kcms/lookandfeel/kcm.h
+++ b/kcms/lookandfeel/kcm.h
@@ -119,6 +119,7 @@ private:
     QStringList m_cursorSearchPaths;
 
     KConfig m_config;
+    KSharedConfigPtr m_config2;
     KConfigGroup m_configGroup;
 
     bool m_applyColors : 1;
Comment 2 Hrvoje Senjan 2015-02-01 15:39:59 UTC
*** Bug 343644 has been marked as a duplicate of this bug. ***
Comment 3 Marco Martin 2015-02-04 09:46:37 UTC
Created attachment 90906 [details]
attempt patch

can you try with the attached patch?
Comment 4 Hrvoje Senjan 2015-02-04 11:07:06 UTC
(In reply to Marco Martin from comment #3)
> can you try with the attached patch?

unfortunately, it doesn't fix the issue =(
Comment 5 Marco Martin 2015-02-05 12:52:23 UTC
managed to reproduce
Comment 6 Marco Martin 2015-02-05 13:26:14 UTC
kindof fear it may be a kconfig issue.
besides of saving the other style instead of breeze, that key gets written in kdeglobals two times being duplicated
Comment 7 Hrvoje Senjan 2015-02-05 14:28:57 UTC
@Matthew
do you have some ideas about this problem?
Comment 8 Marco Martin 2015-02-05 17:51:33 UTC
digging more into this, it seems the underlying KentryMap inside KConfig in this case gets two keys LookAndFeelPackage one with the value in the global file one in the value of the local one 
(oxygen and breeze in this case)

this seems wrong since i guess KentryMap like a good map should be supposed to have unique keys...
Comment 9 Marco Martin 2015-02-05 19:47:54 UTC
this patch in kconfig appears to fix it, but i realize is pobably very very wrong
but could help identify the real problem

diff --git a/src/core/kconfigini.cpp b/src/core/kconfigini.cpp
index 856b7b7..486cfcb 100644
--- a/src/core/kconfigini.cpp
+++ b/src/core/kconfigini.cpp
@@ -319,7 +319,7 @@ void KConfigIniBackend::writeEntries(const QByteArray &locale, QIODevice &file,
         }
 
         // the only thing we care about groups is, is it immutable?
-        if (key.mKey.isNull()) {
+        if (key.mKey.isNull() || key.bDefault) {
             groupIsImmutable = it->bImmutable;
             continue; // skip
         }
Comment 10 Hrvoje Senjan 2015-02-05 19:58:34 UTC
yep, can confirm it writes the correct entry (once ;-) with the above patch
Comment 11 Matthew Dawson 2015-02-09 02:19:02 UTC
Created attachment 90986 [details]
Potential workaround for kcm_lookandfeel

This does indeed appear to be a KConfig issue.  For know, I'll track the KConfig part in #340691 (marked as a blocker for this).  I think this bug can be fixed without requiring changes to KConfig.

For now, this patch may fix this bug while KConfig is being worked on.  I think this patch would be good to have, ignoring the underlying bug.
Comment 12 Hrvoje Senjan 2015-02-09 02:40:48 UTC
hm, i wonder if bug 342570 could be resolved in the same way (on the lines 89 & 91 in [breeze.git]/misc/kde4breeze/src/main.cpp

will test your patch for the kcm asap
Comment 13 Hrvoje Senjan 2015-02-09 02:45:30 UTC
indeed it fixes the problem here after rebuilding krdb & lookandfeel subdir w/ the patch =)
Comment 14 Marco Martin 2015-02-09 09:47:06 UTC
Ok, would go with this patch for now so.
Comment 15 Matthew Dawson 2015-02-09 16:08:27 UTC
Fixed with commit: http://commits.kde.org/plasma-desktop/523497fde1828acfb70a242959b6ee7af1b0dde4 .