Started building plasma-desktop over here on an affected machine with this diff applied

Other people are confirming that this patch also fixes the issue:
https://bugs.kde.org/show_bug.cgi?id=416358

I was not able to reproduce the issue, but this patch causes no regressions for me.

I still don't fully understand the bug and the fix

So what we're saying is:
Something changes us to needs save early on startup
We emit changed early
That gets lost (?)
So we have to reset back to unchanged after loading so that future changes will enable the apply button?

In D27384#611135, @davidedmundson wrote:

I still don't fully understand the bug and the fix

So what we're saying is:
Something changes us to needs save early on startup
We emit changed early
That gets lost (?)
So we have to reset back to unchanged after loading so that future changes will enable the apply button?

In case it makes any more sense: The bug made Fonts KCM never indicate that unsaved changes have been made, meaning you can never Apply any changes since it doesn't think anything was changed that is now pending being applied. Apply is always insensitive. This patch however fixes that for me and others who were affected by this issue.

This patch however fixes that for me

Sure, but why/how?

@davidedmundson
We have 2 bugs :

• KCModuleQML apply button will stay disabled forever if at the end of load function need save is true (probably qml connection not yet done or something like that I guess, not yet found why) => We need to investigate on it too, I reproduced the same behavior with KCM icons by changing a setting at the end of load method.
• In theory at the end of load we don't need to save data. However in some case for the font KCM data are dirty and need save. Came from Qt font comparison, if in your kdeglobals you have a font definition without style name, our algorithm to match a nearest font will add one, and font comparison between referential data and current data will return false because they are different.

• My first approach was to fix algorithm to find the nearest font and keep style name empty if we don't have information. After this fix apply button work as expected, but when you adjust this font the selected style will be the first one on the list and not the one expected (regular)
• Second approach by setting need save to false at the end of load allow us to be on the good state and the next change will allow us to enable apply button. By the way apply button will be enable directly because settings changed is trigered by another time after loading not sure where (didn't investigated yet)

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. On qml side we use directly stNeedsSave (you can look at save method implementation), so a proper fix can be to add "d->configModule->setNeedsSave(false);" after loading

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.

Yes emiting signal from setNeedsSave in all case fix stuff too

So this patch is here to solve for "short term"

In more long term we need to fix ConfigModule do you think is better to do a setNeedsSave(false) after loading or emiting signal in all case ?

I will look at a proper fix in the next days