Bug 343891

Created attachment 90962 [details]
khotkeysrc before changing shortcut settings

this is the khotkeysrc that is created automatically after removing it first and then logging in

Created attachment 90963 [details]
khotkeysrc AFTER changing shortcut settings

khotkeysrc after trying to add a custom shortcut in SystemSettings and clicking "apply"

I reproduced this.. but only after a few attempts.

If you could find out what the pattern is that makes this happen that would really help here.

OK so I dug a little deeper... scroll down for [tl:dr] 

I put my .config dir into a git repo and added commits like this:

for file in $( ls ) ; git rm -r $file && sh -c "git commit -am \"$file\"" done 

this way I git-bisected to the commit that removed kglobalshortcutsrc (yay binary search!)

Once I removed BOTH kglobalshortcutsrc and khotkeysrc it worked.
Deleting khotkeysrc from the working setup does not break anything.
Moving the OLD kglobalshortcutsrc in place breaks it.
If I keep khotkeysrc in place now and move the new (saved from the working setup) kglobalshortcutsrc in place does NOT fix it - so the broken kglobalshortcutsrc breaks khotkeysrc.
Starting from this I narrowed it down to the following line in kglobalshortcutsrc:
{d03619b6-9b3c-48cc-9d9c-a2aadb485550}=none,none,Search
which is found in the section [khotkeys]
the whole diff between working and broken kglobalshortcutsrc is:

--- kglobalshortcutsrc.yes      2015-02-08 06:00:11.723950888 +0100
+++ kglobalshortcutsrc.nope     2015-02-08 05:51:26.913967273 +0100
@@ -20,9 +20,7 @@
 
 [khotkeys]
 _k_friendly_name=khotkeys
-{2069c78b-ac0f-43a6-95a4-7f9c57fadadf}=Print,none,PrintScreen
-{d03619b6-9b3c-48cc-9d9c-a2aadb485550}=,none,Search
-{f643b1ce-97fd-4a9d-8748-adbc6bd74b4b}=Meta+D,none,asddf
+{d03619b6-9b3c-48cc-9d9c-a2aadb485550}=none,none,Search
 
 [kmix]
 _k_friendly_name=KMix
@@ -208,4 +206,4 @@
 
 [yakuake]
 _k_friendly_name=Yakuake
-toggle-window-state=F12,F12,Open/Retract Yakuake
+toggle-window-state=Ctrl+1,F12,Open/Retract Yakuake


removing the faulty line fixes the problem.

[tl:dr] 
In fact changing the line from
{d03619b6-9b3c-48cc-9d9c-a2aadb485550}=none,none,Search
to
{d03619b6-9b3c-48cc-9d9c-a2aadb485550}=,none,Search
makes the problem go away.

Do you need anything else to reproduce this? I guess it'd do no harm to upload both files here?

Just tried it the other way around... made scratch user account, logged in, logged out, located the line mentioned above and added the "none". Add new custom shortcut - boom.
The question is: how did the "none" end up in my kglobalshortcutsrc? Did the old systemsettings do that and this is a backwards-compatibility issue? I most certainly did not add it myself ;)

*** Bug 345252 has been marked as a duplicate of this bug. ***

*** Bug 347031 has been marked as a duplicate of this bug. ***

*** Bug 352351 has been marked as a duplicate of this bug. ***

*** Bug 344383 has been marked as a duplicate of this bug. ***

It seems the kded module is the culprit.
When reading shortcuts, it causes a shortcut change, which in turn makes it write back the (partial) list.

This might be a bug/change in kglobalaccel (undelayed shortcut update?) but this patch shedules the save into the next event queue (so it's not performed *while* reading stuff)


After *reliably* nuking my config every single time I tried, it's now gone.
Anybody can give it a try?

@David, since Michael doesn't seem to maintain khotkeys, do you now?
Want a review request?

-------------------

diff --git a/app/kded.cpp b/app/kded.cpp
index 4e20ad4..307d6d4 100644
--- a/app/kded.cpp
+++ b/app/kded.cpp
@@ -39,6 +39,7 @@ using namespace KHotKeys;
 KHotKeysModule::KHotKeysModule(QObject* parent, const QList<QVariant>&)
     : KDEDModule(parent)
     , actions_root(NULL)
+    , _settingsDirty(false)
     , _settings()
     ,_initialized(false)
     {
@@ -64,7 +65,7 @@ void KHotKeysModule::initialize()
     // If a shortcut was changed (global shortcuts kcm), save
     connect(
             keyboard_handler.data(), SIGNAL(shortcutChanged()),
-            this, SLOT(save()));
+            this, SLOT(scheduleSave()));
 
     // Read the configuration from file khotkeysrc
     reread_configuration();
@@ -239,9 +240,18 @@ void KHotKeysModule::quit()
     deleteLater();
     }
 
+void KHotKeysModule::scheduleSave()
+    {
+    if (!_settingsDirty)
+        {
+        _settingsDirty = true;
+        QMetaObject::invokeMethod(this, "save", Qt::QueuedConnection);
+        }
+    }
 
 void KHotKeysModule::save()
     {
+    _settingsDirty = false;
     KHotKeys::khotkeys_set_active( false );
     _settings.write();
     KHotKeys::khotkeys_set_active( true );
diff --git a/app/kded.h b/app/kded.h
index 623b1e8..68bc6a3 100644
--- a/app/kded.h
+++ b/app/kded.h
@@ -62,6 +62,7 @@ class KHotKeysModule
     private Q_SLOTS:
 
         //! Save
+        void scheduleSave();
         void save();
 
         //! Initialize the module. Delayed initialization.
@@ -79,6 +80,7 @@ class KHotKeysModule
 
         //! The current settings
         KHotKeys::Settings _settings;
+        bool _settingsDirty;
 
         //! Is the module initialized
         bool _initialized;

Maybe I should also watch the bug ;-)

https://git.reviewboard.kde.org/r/125607/

Git commit 4747599badf67389530483ea62b6f54bc36ac9c3 by Thomas Lübking.
Committed on 12/10/2015 at 13:47.
Pushed by luebking into branch 'master'.

schedule saving to the next event cycle

notably since writing reading and applying
settings can cause a shortcut update which
itself triggers a save (of the partial-only
database)

But also since several changes can occur in
a row, so we don't write for everyone
FIXED-IN: 5.5

M  +11   -1    app/kded.cpp
M  +2    -0    app/kded.h

http://commits.kde.org/khotkeys/4747599badf67389530483ea62b6f54bc36ac9c3

BTW, shouldn't khotkeys open config as KConfig::SimpleConfig? To avoid adding lines like:

[Icons]
Theme[$d]

Those wouldn't be avoided (local $d simply trumps cascading and moves to the application default) and using simple config would kill kiosk.