Bug 487235

Summary:	Some Breeze (light) icons not recolored for the Breeze Dark package
Product:	[Plasma] Breeze	Reporter:	Corbin <schwimmbeckc>
Component:	Icons	Assignee:	visual-design
Status:	RESOLVED FIXED
Severity:	normal	CC:	kainz.a, m
Priority:	NOR
Version First Reported In:	6.0.4
Target Milestone:	---
Platform:	unspecified
OS:	All
Latest Commit:		Version Fixed In:
Sentry Crash Report:
Attachments:	breeze 6 left vs breeze 5 right (from the fedora rpms, identical to arch packages) breeze 6 left vs breeze 5 right (dark) (from the fedora rpms, identical to arch packages)

Description Corbin 2024-05-19 14:05:12 UTC

I'm developing an application with PySide6  (Qt for Python) and use xdg icon theme names to let my app be themeable.
This means it pulls icons from the system's icon theme, in my case, Breeze Dark.
Unfortunately, Qt doesn't natively recolor icons, so with Breeze6 I noticed that some of my icons were now from the breeze
instead of the breeze dark set.
Looking at where the icons are stored: /usr/share/icons/breeze-dark I saw that many of the icons, especially in the 
status/16 directory, weren't actually recolored, but instead identical to their breeze (light) counterpart.
Looking at the Gitlab, it's clear that there is an automated process that generates the dark icons from the light icons,
but apparently it isn't working so well, as the number of un-recolored icons has drastically increased since breeze5.

Checking the pacman cache, to look at the contents of the tar.gz that was installed, I saw much the same. 
Then checking another distro, I looked at the contents of the breeze-icon-theme-6.2.0 Fedora rpm 
(https://src.fedoraproject.org/rpms/breeze-icon-theme), which was identical. Attached is a split for the breeze 5
and breeze 6 dark icons.

Using the Icon Explorer, the icons do load correctly, showing no signs of wrong coloring, so it's likely the runtime
icon loader is fixing all these issues, but the static recoloring script isn't doing as good of a job.

OBSERVED RESULT
Many icons in breeze dark aren't fit for a dark background.

EXPECTED RESULT
All icons are properly recolored.

Comment 1 Corbin 2024-05-19 14:07:15 UTC

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

Comment 2 Corbin 2024-05-19 14:11:27 UTC

Created attachment 169616 [details]
breeze 6 left vs breeze 5 right (from the fedora rpms, identical to arch packages)

Not sure why this didn't attach to the main comment.

Comment 3 Corbin 2024-05-19 14:16:03 UTC

Comment on attachment 169616 [details]
breeze 6 left vs breeze 5 right (from the fedora rpms, identical to arch packages)

Wait, wrong picture.

Comment 4 Corbin 2024-05-19 14:18:05 UTC

Created attachment 169617 [details]
breeze 6 left vs breeze 5 right (dark) (from the fedora rpms, identical to arch packages)

All right, sorry for the abject incompetence on display on my part, I hope I got this report properly assembled now.

Comment 5 Corbin 2024-05-19 21:01:14 UTC

Looking into this more, I found the regular expression that find the style color: "\\.ColorScheme-(\\S+){color:(#[0-9a-fA-F]+);}" (https://invent.kde.org/frameworks/breeze-icons/-/blob/master/src/tools/generate-symbolic-dark.cpp?ref_type=heads)

Which works, until some of the svg files are formatted like this: (actions/16/blur.svg)
    <style>.ColorScheme-Text { 
    color:#232629;
}</style>

There you'd need "\\.ColorScheme-(\\S+)\\s*{\\s*color:(#[0-9a-fA-F]+);\\s*}"

But it also fails for files like actions/16/bookmark-remove:
    <style type="text/css" id="current-color-scheme">.ColorScheme-Text{color:#232629;}.ColorScheme-NegativeText { color: #da4453; } </style>

Matter of fact, the pattern also doesn't match the latter negative text color scheme, due to the additional space between color: and the #,
needing even more \s* patterns to properly detect all of these edge cases. Though for this one, it doesn't matter, as only Text and Background
are actually handled in the code anyway. Though it'd probably be simpler to eliminate all whitespace before trying to use regex to parse this.

Interestingly, it did work for a different file, where that looked like this:
    <style type="text/css" id="current-color-scheme">.ColorScheme-Text { color: #fcfcfc; } </style>
with spaces in various positions.

And another one where it failed was well formatted, with no whitespace:
    <style type="text/css" id="current-color-scheme">.ColorScheme-Text{color:#232629;}.ColorScheme-NeutralText{color:#f67400;}.ColorScheme-NegativeText { color: #da4453; } </style>


The only consistent throughline here is that all the ones that failed, had more than one ColorScheme set. Which shouldn't be the issue,
since a regex iterator is used, but that's just what must be happening.

Comment 6 Corbin 2024-05-19 22:44:15 UTC

I shall attempt to fix the issue, but success isn't guaranteed...

Comment 7 Corbin 2024-05-20 01:29:29 UTC

I've found the issue. It's the greedy match on \S+ which WILL match across the other tags, creating one giant abomination
of a match when there are multiple tags present. Classic regex. Merge Request incoming, hopefully I remember how to do that correctly.

Comment 8 Bug Janitor Service 2024-05-20 02:28:35 UTC

A possibly relevant merge request was started @ https://invent.kde.org/frameworks/breeze-icons/-/merge_requests/372

Comment 9 Corbin 2024-05-20 18:29:26 UTC

Ah, turns out I used the wrong email for opening this bug report, so it didn't auto close.
Anyway, my merge request was accepted, this problem will be solved with the next release.