Bug 136928 - Wrong tabs icons in dark theme with KDE
Summary: Wrong tabs icons in dark theme with KDE
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
6.4.7.2 release
Hardware: All Linux (All)
: medium minor
Assignee: Not Assigned
URL:
Whiteboard: target:7.5.0 target:7.4.2
Keywords: bibisected, bisected
: 149203 (view as bug list)
Depends on:
Blocks: KDE Linux-Dark-Mode
  Show dependency treegraph
 
Reported: 2020-09-21 11:55 UTC by medmedin2014
Modified: 2022-09-19 11:53 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
Wrong tabs icons in dark mode (45.46 KB, image/png)
2020-09-21 11:55 UTC, medmedin2014
Details
Compared before and after the commit (24.82 KB, image/webp)
2022-09-13 09:18 UTC, Timur
Details
Demo screenshot with hard-coded white as colore (29.81 KB, image/png)
2022-09-14 15:34 UTC, Michael Weghorn
Details
Screenshot with gtk3 and Adwaita-Dark theme with patch from comment 21 in place (32.58 KB, image/png)
2022-09-16 06:25 UTC, Michael Weghorn
Details

Note You need to log in before you can comment on or make changes to this bug.
Description medmedin2014 2020-09-21 11:55:48 UTC
Created attachment 165729 [details]
Wrong tabs icons in dark mode

Wrong tabs icons in dark mode. See attached image for more info.
Comment 1 medmedin2014 2020-09-21 11:56:04 UTC
Version: 7.0.1.2
Build ID: 00(Build:2)
CPU threads: 2; OS: Linux 5.4; UI render: default; VCL: kf5
Locale: en-US (en_US.UTF-8); UI: en-US
=7.0.1-1
Calc: threaded

Operating System: Manjaro Linux
KDE Plasma Version: 5.19.5
KDE Frameworks Version: 5.73.0
Qt Version: 5.15.0
Kernel Version: 5.4.64-1-MANJARO
OS Type: 64-bit
Comment 2 Buovjaga 2021-07-19 09:28:52 UTC
No repro, I get white background for them. Maybe because I have gtk3 VCL backend

NixOS
Version: 7.1.4.2 / LibreOffice Community
Build ID: 10(Build:2)
CPU threads: 16; OS: Linux 5.13; UI render: default; VCL: gtk3
Locale: fi-FI (fi_FI.UTF-8); UI: en-US
Calc: threaded
Comment 3 medmedin2014 2021-07-19 13:57:58 UTC Comment hidden (obsolete)
Comment 4 Dieter 2022-08-19 15:00:30 UTC Comment hidden (obsolete)
Comment 5 medmedin2014 2022-08-20 09:49:46 UTC
(In reply to Dieter from comment #4)
> Hello Medmedin, a new major release of LibreOffice is available since this
> bug was reported. Could you please try to reproduce it with the latest
> version of LibreOffice from
> https://www.libreoffice.org/download/libreoffice-fresh/ ?
> I have set the bug's status to 'NEEDINFO'. Please change it back to
> 'UNCONFIRMED' if the bug is still present in the latest version.



It's still repro on:

Version: 7.5.0.0.alpha0+ / LibreOffice Community
Build ID: 4c96abd81460977d413d4d28e891bbbac5769ede
CPU threads: 2; OS: Linux 5.19; UI render: default; VCL: kf5 (cairo+xcb)
Locale: en-US (en_US.UTF-8); UI: en-US
Calc: threaded

Operating System: Manjaro Linux
KDE Plasma Version: 5.24.6
KDE Frameworks Version: 5.96.0
Qt Version: 5.15.5
Kernel Version: 5.19.1-3-MANJARO (64-bit)
Graphics Platform: X11
Comment 6 Timur 2022-09-12 14:52:29 UTC Comment hidden (me-too)
Comment 7 Timur 2022-09-13 07:47:21 UTC
I can confirm only with kf5, not gtk3 in Ubuntu. Let's call regression from 6.4.

author	Tim Bartlett <github@tim.bartletts.id.au>	2019-08-19
committer	Michael Stahl <Michael.Stahl@cib.de>	2019-09-02
commit 0fbcac4caa971bd8824c96fe9ef7d9338cd37cbc (patch)
tdf#55436 - Add SYMBOL_CHICAGO numbering scheme (for footnotes)

So good news is that this is bisected. Not good is that author had that single fix ever. I add him to CC anyway to see if there will be a response.
Comment 8 Buovjaga 2022-09-13 08:05:10 UTC
(In reply to Timur from comment #7)
> I can confirm only with kf5, not gtk3 in Ubuntu. Let's call regression from
> 6.4.
> 
> author	Tim Bartlett <github@tim.bartletts.id.au>	2019-08-19
> committer	Michael Stahl <Michael.Stahl@cib.de>	2019-09-02
> commit 0fbcac4caa971bd8824c96fe9ef7d9338cd37cbc (patch)
> tdf#55436 - Add SYMBOL_CHICAGO numbering scheme (for footnotes)
> 
> So good news is that this is bisected. Not good is that author had that
> single fix ever. I add him to CC anyway to see if there will be a response.

The commit does not look related to this problem.
Comment 9 Tim B 2022-09-13 08:47:00 UTC
(In reply to Timur from comment #7)
> I can confirm only with kf5, not gtk3 in Ubuntu. Let's call regression from
> 6.4.
> 
> author	Tim Bartlett <github@tim.bartletts.id.au>	2019-08-19
> committer	Michael Stahl <Michael.Stahl@cib.de>	2019-09-02
> commit 0fbcac4caa971bd8824c96fe9ef7d9338cd37cbc (patch)
> tdf#55436 - Add SYMBOL_CHICAGO numbering scheme (for footnotes)
> 
> So good news is that this is bisected. Not good is that author had that
> single fix ever. I add him to CC anyway to see if there will be a response.

Wew, I did manage to actually find my login and password for this bugtracker! Three cheers for KeePass.

That said, my commit was only making changes to add a new footnote numbering scheme that was present in MS Office but not Libreoffice, and didn't touch the GUI at all. So if I somehow managed to break icons with the patch, I'm extremely surprised and curious of how!
Comment 10 Timur 2022-09-13 09:18:10 UTC Comment hidden (obsolete)
Comment 11 Buovjaga 2022-09-13 13:29:39 UTC
(In reply to Timur from comment #10)
> Created attachment 182401 [details]
> Compared before and after the commit
> 
> (In reply to Buovjaga from comment #8)
> > The commit does not look related to this problem.
> I know. But please test. 

I do get the same with 6.4 bibisect repo. However, I manually reverted the commit (it is not possible directly with git revert) and in my build the problem is still seen.
Comment 12 Timur 2022-09-13 13:44:07 UTC Comment hidden (obsolete)
Comment 13 Buovjaga 2022-09-13 13:48:24 UTC
(In reply to Timur from comment #12)
> Can you do new bibisect now, if the same machine?

To be clear, I tested the commit you referenced and the previous commit and I saw the same as you. It is a mystery why, but is not related to the code change.
Comment 14 Michael Weghorn 2022-09-14 09:16:56 UTC
(In reply to Buovjaga from comment #13)
> To be clear, I tested the commit you referenced and the previous commit and
> I saw the same as you. It is a mystery why, but is not related to the code
> change.

The previous commit in the bibisect repo doesn't include the kf5 VCL plugin, so uses gtk3_kde5 by default when run on KDE Plasma (as can be seen in "Help" -> "About LibreOffice"), so that looks qt5/kf5/qt6-specific rather than like a regression.
It looks the same with the commit from comment 7 in place when explicitly choosing to use the gtk3_kde5 plugin there as well by setting SAL_USE_VCLPLUGIN=gtk3_kde5 env variable.

Anyway, what's the expected behavior? I'm far from being a designer or UI expert, but wondering whether the "black sign in a white box with the rest of the dialog using dark background" from attachment 182401 [details] ("before the commit" case) is actually the expected behavior?

Or is the dark background OK and the "tab sign" should have light color?
Comment 15 Timur 2022-09-14 09:35:24 UTC
Thanks for explanation. In those cases we remove regression, but not bibisected, bisected.
Comment 16 Buovjaga 2022-09-14 10:26:17 UTC
(In reply to Michael Weghorn from comment #14)
> (In reply to Buovjaga from comment #13)
> > To be clear, I tested the commit you referenced and the previous commit and
> > I saw the same as you. It is a mystery why, but is not related to the code
> > change.
> 
> The previous commit in the bibisect repo doesn't include the kf5 VCL plugin,
> so uses gtk3_kde5 by default when run on KDE Plasma (as can be seen in
> "Help" -> "About LibreOffice"), so that looks qt5/kf5/qt6-specific rather
> than like a regression.
> It looks the same with the commit from comment 7 in place when explicitly
> choosing to use the gtk3_kde5 plugin there as well by setting
> SAL_USE_VCLPLUGIN=gtk3_kde5 env variable.

Ouch, should have looked at the About dialog :)

> Anyway, what's the expected behavior? I'm far from being a designer or UI
> expert, but wondering whether the "black sign in a white box with the rest
> of the dialog using dark background" from attachment 182401 [details]
> ("before the commit" case) is actually the expected behavior?
> 
> Or is the dark background OK and the "tab sign" should have light color?

I tested in a VM with Debian testing and native GNOME: with gtk3 UI, the tab signs have the white background box. Not sure why that is - lacking dark mode variant icons?
Comment 17 Michael Weghorn 2022-09-14 11:21:27 UTC
(In reply to Buovjaga from comment #16)
> I tested in a VM with Debian testing and native GNOME: with gtk3 UI, the tab
> signs have the white background box. Not sure why that is - lacking dark
> mode variant icons?

@Rizal: Do you happen to know whether that's an issue related to the icons?
(Another potential cause I can think of is that these are not icons but drawing happens in LO code and it's not using proper colors from the style, but e.g. hard-coded black for the signs instead of the style's font color or somesuch.)
Comment 18 Rizal Muttaqin 2022-09-14 14:30:36 UTC
(In reply to Michael Weghorn from comment #17)

> @Rizal: Do you happen to know whether that's an issue related to the icons?
> (Another potential cause I can think of is that these are not icons but
> drawing happens in LO code and it's not using proper colors from the style,
> but e.g. hard-coded black for the signs instead of the style's font color or
> somesuch.)

I have never seen those symbols were being handled by icon theme. Dark Colibre give me dark also with kf5 backend.

Version: 7.5.0.0.alpha0+ / LibreOffice Community
Build ID: d2c500fb40be23aa7bd1d2b7cc02e47ea109d25c
CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: kf5 (cairo+xcb)
Locale: id-ID (id_ID.UTF-8); UI: en-US
Calc: threaded
Comment 19 Michael Weghorn 2022-09-14 15:34:25 UTC
Created attachment 182444 [details]
Demo screenshot with hard-coded white as colore

(In reply to Rizal Muttaqin from comment #18)
> I have never seen those symbols were being handled by icon theme. Dark
> Colibre give me dark also with kf5 backend.

Thanks!

Indeed, drawing happens in C++ code, in `TabWin_Impl::Paint`:
https://git.libreoffice.org/core/+/76342be56ee1ec52a27cf867760c2485ac4fd291/cui/source/tabpages/tabstpge.cxx#55

If I replace the 'rRenderContext.GetSettings().GetStyleSettings().GetFontColor()' with a hard-coded 'COL_WHITE' for a quick test with QT_STYLE_OVERRIDE="Adwaita-Dark", it looks as in the attached screenshot with kf5.

So if that's (almost) the expected outcome, one would presumably have to check where the style settings for the 'rRenderContext' param come from.
Comment 20 Michael Weghorn 2022-09-15 17:51:45 UTC
(In reply to Michael Weghorn from comment #19)
> So if that's (almost) the expected outcome, one would presumably have to
> check where the style settings for the 'rRenderContext' param come from.

Turns out black is actually the unmodified font color from the system style/theme, but there are separate colors for window text, dialog text, etc.

In that context, dialog text color sounds fitting to me, Gerrit patch here:
https://gerrit.libreoffice.org/c/core/+/140029

By itself, that would break gtk3 (which then uses light gray signs/"icons" on white background).

To me, using dark background for gtk3 too would seem consistent e.g. also with the "Character" item just below the tab signs, but I'm not a UI expert.

Any thoughts on this?

@Caolán: Any idea on whether the drawing area background here is white for gtk3 intentionally even with e.g. GTK_THEME=Adwaita:dark ?
Comment 21 Commit Notification 2022-09-16 06:23:46 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/c317f0adb308b1ece3bfa8bdffb4d0bcb1478d30

tdf#136928 Use dialog text color for signs in "Tabs" dialog

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 22 Michael Weghorn 2022-09-16 06:25:44 UTC
Created attachment 182481 [details]
Screenshot with gtk3 and Adwaita-Dark theme with patch from comment 21 in place
Comment 23 Michael Weghorn 2022-09-16 06:27:13 UTC
(In reply to Michael Weghorn from comment #20)
> @Caolán: Any idea on whether the drawing area background here is white for
> gtk3 intentionally even with e.g. GTK_THEME=Adwaita:dark ?

Thanks a lot for the explanation in the Gerrit change and updating it to explicitly set the proper colors!

Merged for master now, backport for 7-4 pending in Gerrit:
https://gerrit.libreoffice.org/c/core/+/139982

attachment 182481 [details] is a screenshot for gtk3 with the patch in place, kf5 looks very similar.
Comment 24 Commit Notification 2022-09-17 08:37:11 UTC
Michael Weghorn committed a patch related to this issue.
It has been pushed to "libreoffice-7-4":

https://git.libreoffice.org/core/commit/67e5d8ff8d9bf029ab7480fa1f907696ca8ee2e9

tdf#136928 Use dialog text color for signs in "Tabs" dialog

It will be available in 7.4.2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 25 Timur 2022-09-19 11:53:39 UTC
*** Bug 149203 has been marked as a duplicate of this bug. ***