Bug 485801 - With mixed light/dark themes (e.g. Breeze Twilight) KSvg and Kirigami.Icon use app color scheme rather than Plasma color scheme when given an absolute path
Summary: With mixed light/dark themes (e.g. Breeze Twilight) KSvg and Kirigami.Icon us...
Status: ASSIGNED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Icon (show other bugs)
Version: 6.0.4
Platform: Manjaro Linux
: NOR normal
Target Milestone: 1.0
Assignee: Plasma Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-04-19 16:43 UTC by Driglu4it
Modified: 2024-05-10 10:00 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
cat icon have a wrong color* (9.69 KB, image/png)
2024-04-19 16:43 UTC, Driglu4it
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Driglu4it 2024-04-19 16:43:45 UTC
Created attachment 168682 [details]
cat icon have a wrong color*

SUMMARY
Using a plasmoid icon with an absolute path (Qt.resolvedUrl(***)) uses the wrong color scheme when using Breeze Twilight.

STEPS TO REPRODUCE
1.  Download any community plasmoid (e.g. CatWalk)l.
2.  Place it on panel.
3. Switch to Breeze Twilight.

OBSERVED RESULT
Dark icon on dark panel.

EXPECTED RESULT
Light icon on dark panel.

SOFTWARE/OS VERSIONS
Operating System: Manjaro Linux 
KDE Plasma Version: 6.0.4
KDE Frameworks Version: 6.1.0
Qt Version: 6.7.0
Kernel Version: 6.8.7-1-MANJARO (64-bit)
Graphics Platform: Wayland
Processors: 12 × Intel® Core™ i5-10500H CPU @ 2.50GHz
Memory: 15.4 ГиБ of RAM

ADDITIONAL INFORMATION
Community widgets behave the same way in the system tray (if the icon is not from the theme).
Comment 1 Nate Graham 2024-04-21 09:50:54 UTC
Cannot confirm with Breeze light or Breeze Dark; works there. Can confirm with Breeze Twilight--which is to say, it's broken with mixed light/dark themes. Looking at the code to ss if it;'s our bug our yours.

JFYI I absolutely adore CatWalk. It's dangerously adorable. I need a version for memory too! Maybe the can can get fat or something.
Comment 2 Nate Graham 2024-04-21 10:00:35 UTC
I can confirm that if I replace `Qt.resolvedUrl(something` with something from the theme like "widgets/clock", that SVG correctly keeps the Plasma coloration rather than inappropriately using the app coloration.

Possibly this is multiple bugs—one in KSvg and one in Kirigami.icon—but maybe they share a root cause. Moving to libplasma for now, and Marco can say what else we need to do.
Comment 3 Driglu4it 2024-04-21 11:29:33 UTC
(In reply to Nate Graham from comment #1)
> Cannot confirm with Breeze light or Breeze Dark; works there. Can confirm
> with Breeze Twilight--which is to say, it's broken with mixed light/dark
> themes. Looking at the code to ss if it;'s our bug our yours.
> 
> JFYI I absolutely adore CatWalk. It's dangerously adorable. I need a version
> for memory too! Maybe the can can get fat or something.

Okay, what kind of visualization should I do for memory?
Is a refillable glass ok? )))
Comment 4 Nate Graham 2024-04-21 11:57:38 UTC
Whatever it is, it needs a cute cat! Maybe the more memory your system is using, the more the cat looks like Garfield.

Anyway, off-topic here, sorry lol
Comment 5 Nicolas Fella 2024-05-06 19:48:11 UTC
Reading the KSVG code this seems to be somewhat intentional.

The documentation for KSvg::Svg (https://invent.kde.org/frameworks/ksvg/-/blob/master/src/ksvg/svg.h?ref_type=heads#L39) says

> Unless an absolute path to a file is provided, it loads the SVG document using KSvg::ImageSet

and in https://invent.kde.org/frameworks/ksvg/-/blob/master/src/ksvg/svg.cpp#L466 svgs with absolute paths are considered unthemed
Comment 6 Nicolas Fella 2024-05-06 19:56:38 UTC
Same with Kirigami.Icon, coloring is explicitly disabled for absolute paths: https://invent.kde.org/frameworks/kirigami/-/blob/master/src/icon.cpp?ref_type=heads#L542
Comment 7 Nate Graham 2024-05-06 22:46:24 UTC
I wonder what the reason for that is. In this use case at least, we want the SVG to be themed.
Comment 8 Marco Martin 2024-05-07 10:16:19 UTC
(In reply to Nicolas Fella from comment #5)
> Reading the KSVG code this seems to be somewhat intentional.
> 
> The documentation for KSvg::Svg
> (https://invent.kde.org/frameworks/ksvg/-/blob/master/src/ksvg/svg.
> h?ref_type=heads#L39) says
> 
> > Unless an absolute path to a file is provided, it loads the SVG document using KSvg::ImageSet
> 
> and in
> https://invent.kde.org/frameworks/ksvg/-/blob/master/src/ksvg/svg.cpp#L466
> svgs with absolute paths are considered unthemed

This is a very, very old thing that's there since 4.0 times, but i think the behavior in that it could be safely changed: as if the svg has the proper stylesheet things would always be styled, otherwise it wouldn't be anyways, so i don't see danger in changing this
Comment 9 Bug Janitor Service 2024-05-07 10:48:47 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/ksvg/-/merge_requests/48
Comment 10 Marco Martin 2024-05-07 10:51:48 UTC
So https://invent.kde.org/frameworks/ksvg/-/merge_requests/48 attempts to always recolor images also the ones "non themed" as in taken from somewhere else in the filesystem

Now this should be checked for possible sideeffect.... worst case scenario can be added api about it, to not change default behavior
Comment 11 Bug Janitor Service 2024-05-10 10:00:29 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kirigami/-/merge_requests/1547