Bug 438273

Summary: Titlebar colors not changed correctly when switching from a header-color-using color scheme to a non-header-color-using color scheme
Product: [Applications] systemsettings Reporter: Nate Graham <nate>
Component: kcm_colorsAssignee: Noah Davis <noahadvs>
Status: RESOLVED FIXED    
Severity: normal CC: benjamin.port, jpwhiting, mwoehlke.floss, noahadvs, oded, xaver.hugl
Priority: VHI Keywords: regression
Version: 5.21.90   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed In:
Sentry Crash Report:

Description Nate Graham 2021-06-08 17:11:31 UTC
Everything KDE from current git master on top of Fedora 34.

STEPS TO REPRODUCE
1. Go to the colors KCM and switch to Breeze Light, if you are not already using it
2. Switch to Breeze

OBSERVED RESULT
The titlebar does not turn dark colored as expected.

EXPECTED RESULT
The titlebar turns dark-colored, as everything was before we switched to Breeze Light by default.

The same thing happens for other non-header-using color schemes. Perhaps relatedly, in ~/.config/kdeglobals, the [Colors:Header] and [Colors:Header][Inactive] groups are not being deleted as expected; instead they look like this:

[Colors:Header]
BackgroundAlternate[$d]
BackgroundNormal[$d]
DecorationFocus[$d]
DecorationHover[$d]
ForegroundActive[$d]
ForegroundInactive[$d]
ForegroundLink[$d]
ForegroundNegative[$d]
ForegroundNeutral[$d]
ForegroundNormal[$d]
ForegroundPositive[$d]
ForegroundVisited[$d]

[Colors:Header][Inactive]
BackgroundAlternate[$d]
BackgroundNormal[$d]
DecorationFocus[$d]
DecorationHover[$d]
ForegroundActive[$d]
ForegroundInactive[$d]
ForegroundLink[$d]
ForegroundNegative[$d]
ForegroundNeutral[$d]
ForegroundNormal[$d]
ForegroundPositive[$d]
ForegroundVisited[$d]

Deleting them manually causes the titlebar to change color to the color of the last-used color scheme. It seems like the titlebar colors are not getting updated.
Comment 1 Noah Davis 2021-06-08 17:46:55 UTC
I can't reproduce this on git master.
Comment 2 Zamundaaa 2021-08-13 13:08:07 UTC
Can reproduce. All non-header color schemes don't have their distinct title bar color anymore
Comment 3 Nate Graham 2021-08-13 13:52:10 UTC
Noah, could you take another look maybe?
Comment 4 Oded Arbel 2021-08-29 16:34:17 UTC
I'm not sure if its related or not, but I think I can reproduce the behavior by turning on and off the new "accent color" feature (I commended on the feature MR here: https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/305#note_295265 ).

The behavior I see is:

1. in the Colors KCM, select default accent from color scheme, choose "Breeze" color scheme, and apply -> Title bar becomes dark (if it wasn't that before).
2. Choose an accent color and apply -> Title bar becomes window background colored.
3. Select default accent from color scheme and apply -> Title bar becomes dark again.

Regarding the content of `~/.config/kdeglobals` - with the color scheme accent color, there are no `Colors:header` groups in the file. When I set the accent color (to the first color, in this example), I get this in the file:

```
[Colors:Header]
DecorationFocus=233,58,154
DecorationHover=233,58,154
ForegroundActive=233,58,154
ForegroundLink=233,58,154
```

and setting back to accent from color scheme, that section gets removed.
Comment 5 Nate Graham 2021-10-18 20:44:42 UTC
I think I found the root cause of the issue.

When we switch color schemes, we first delete all the config groups and build up a new set of config groups. The presence or absence of the "[Header]" color group determines whether the color scheme is marked as having header colors.

However, calling kconfig::deleteGroup() doesn't *actually* delete the group. It instead changes all of the values of its keys to "[$d]" which I gather means "I've been deleted!" But the group itself still exists. so kconfig::hasGroup() will always return true for it.
Comment 6 Nate Graham 2021-10-18 21:29:54 UTC
Never mind, that was a red herring.
Comment 7 Nate Graham 2021-10-18 22:00:58 UTC
Found the real bug.

People who are affected have at least one more kdeglobals file on their system besides ~/.config/keglobals, and this other kdeglobals file has a color scheme specified in it that includes a [Colors:Header] group. Those people can work around it by removing the [Colors:Header] and [Colors:Header][Inactive] groups from that file.

When this file exists, the code that deletes the [Colors:Header] group when switching to a non-header color scheme doesn't actually delete the group and all its entries, it sets the keys' values to [$d] which is supposed to override the other file. However something about this does not work with the way we read colors from the color scheme to actually apply them to the system. Probably we are failing to check for whether the [Colors:Header] group exists or not, and are instead unconditionally reading its entries, rather than either checking for the group's existence using hasGroup() or verifying the existence of its entries' values with hasKey().
Comment 8 Nate Graham 2021-10-18 22:26:05 UTC
Next challenge is to find the actual piece of code that reads kdeglobals and applies the color scheme there to apps. Something in there is broken but I can't find where the code lives...
Comment 9 Nate Graham 2021-10-19 12:32:57 UTC
Plot twist: it looks like another element of it--or maybe all of it--is where KWin opens kdeglobals to read the active color scheme in abstract_client.cpp. It asks for kdeglobals wich makes use of the cascade config system.

In my case, I have a kdeglobals file located at ~/.config/kdedefaults/kdeglobals which has color data in it for Breeze Light. ~/.config/kdedefaults takes precedence over ~/.config/, so when KWin says, "yo, give me kdeglobals" it gets the data from ~/.config/kdedefaults/kdeglobals first. And that config file has a color scheme with header colors in it, so if ~/.config/kdeglobals specifies a color scheme without header colors, they get inherited automatically from ~/.config/kdedefaults/kdeglobals, if they exist there.

Removing ~/.config/kdedefaults/kdeglobals fixes the bug for me, though I'm still trying to figure out where it comes from.

For anyone else affected, do you also have a file at ~/.config/kdedefaults/kdeglobals which includes color data for a color scheme with header colors in it?
Comment 10 Zamundaaa 2021-10-19 12:38:31 UTC
There is no kdedefaults folder on my PC
Comment 11 Zamundaaa 2021-10-19 12:40:18 UTC
But it seems like I'm not affected anymore, unless I use an accent color. If I use the accent color from the color scheme it seems to work fine again
Comment 12 Oded Arbel 2021-10-19 12:49:03 UTC
(In reply to Nate Graham from comment #9)
> In my case, I have a kdeglobals file located at
> ~/.config/kdedefaults/kdeglobals which has color data in it for Breeze
> Light.

I don't have a ~/.config/kdedefaults in my system, but also - with the current Neon unstable builds - I can no longer reproduce the issue I reported on in comment #4.
Comment 13 Nate Graham 2021-10-19 13:02:49 UTC
Ben I see that this "kdedefaults" folder support was added in https://invent.kde.org/plasma/plasma-workspace/-/commit/0d0bca58cfd6dcdba6bb12f8f69d7b0f7d9e1b69.

Unfortunately this causes the problem here because ~/.config/kdedefaults/kdeglobals is preferred over ~/.config/kdeglobals anytime something opens kdeglobals, which can cause config data to be read from that file instead.

There might be a KConfig bug here in that 

key=value from ~/.config/kdedefaults/kdeglobals is read when key=[$d] from ~/.config/kdeglobals exists, but it should not be.

Ben, can you take a look?
Comment 14 Benjamin Port 2021-10-19 13:05:56 UTC
I will take a look
Comment 15 Nate Graham 2021-10-19 13:09:29 UTC
Thank you!

(In reply to Zamundaaa from comment #11)
> But it seems like I'm not affected anymore, unless I use an accent color. If
> I use the accent color from the color scheme it seems to work fine again
That's Bug 443786 which I just fixed yesterday.
Comment 16 Nate Graham 2021-10-20 18:11:18 UTC
Ben and I investigated this and discovered that the issue was caused by an older version of the KCM defaults stuff which was thankfully never released to users. So the only people who were affected were devs who tested stuff and built plasma form source. Affected people can safely delete either ~/.config/kdedefaults/kdeglobals, or all of the color definitions inside that file.