davidedmundson added a comment.

  > About KCModuleQML didn't investigate a lot but KCModule (parent class) emit 
a changed false on showEvent (the way the handle to set need save to false.
  
  Aha! What a weird bit of code.
  
  so I think I see what happens:
  
  1. KCModule::showEvent()
  2. this queues up a load and queues up a KCModule::changed(false)
  
  
  
  3. during load ConfigModule::setNeedsSave(true) is called we set 
d->_needsSave to true
  4. we emit ConfigModule::changed(true) which we proxy through to 
KCModule::changed(true)
  
  5. we then process the queued KCModule::setChanged(false) from the earlier 
KCModule::showEvent
  6. so we disable the button
  
  7. any subsequent changes in the KCM will call 
ConfigModule::setNeedsSave(true)
  
  but this now matches d->_needsSave so it no-ops. Even though KCModule is out 
of sync.
  
  Systemsettings only knows what KCModule signals, not ConfigModule.
  
  This patch resets ConfigModule::d->_needsSave after step 3
  
  ---
  
  In the morning can you confirm that commenting out the return in
  
    void ConfigModule::setNeedsSave(bool needs)
    {
        if (needs == d->_needsSave) {
            return;
        }
  
  also fixes it.
  
  If so, I understand it all, and will happily approve this patch.

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D27384

To: bport, #plasma, broulik, ervin, crossi, meven, ngraham, davidedmundson, 
The-Feren-OS-Dev
Cc: The-Feren-OS-Dev, plasma-devel, Orage, LeGast00n, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

Reply via email to