[Linux] Thunderbird side panel icons are broken with system dark theme
Categories
(Thunderbird :: Theme, defect, P1)
Tracking
(thunderbird_esr78 fixed, thunderbird78 wontfix, thunderbird79 wontfix, thunderbird80 fixed)
People
(Reporter: asdfghrbljzmkd, Assigned: aleca)
References
(Blocks 1 open bug)
Details
(Keywords: regression, Whiteboard: [TM:78.2.0])
Attachments
(6 files, 2 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
aleca
:
review+
Paenglab
:
feedback+
darktrojan
:
feedback+
wsmwk
:
approval-comm-beta+
wsmwk
:
approval-comm-esr78+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details |
User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:78.0) Gecko/20100101 Firefox/78.0
Steps to reproduce:
Launched TB 77 Beta with a fresh profile, and added one of my email addresses.
System is Xubuntu 20.04, tested with Adwaita/Adwaita Dark themes.
Actual results:
Icons in side pane are barely visible (dark blue on black)
Expected results:
Icons should be visible (maybe light blue for dark systems themes?)
Unrelated, but not sure if it's worth a new bug report or if it's already known, but the color in the active tab is missing until you switch tabs.
Reporter | ||
Comment 1•5 years ago
|
||
Image of light theme (icons are visible).
Reporter | ||
Comment 2•5 years ago
|
||
Updated•4 years ago
|
Comment 3•4 years ago
|
||
This comes from https://searchfox.org/comm-central/rev/af886b93f1a520cb98cb6c0e3eda612a1caf2560/mail/themes/shared/mail/folderPane.css#6. It's a light black colour for light backgrounds. But Linux themes and Windows high contrast themes can have dark backgrounds without trigger [lwt-tree-brighttext]. I think we should only use a opacity here to make the icons less dark/light.
What do you think, Alessandro?
Assignee | ||
Comment 4•4 years ago
|
||
I think we should only use a opacity here to make the icons less dark/light.
I'm not sure that would be a good solution as we risk to lose too much contrast and the user won't be able to use the colors he wants with a fixed imposed opacity below 0.8
How are we handling the text color? Can we implement CSS variations in order to cover every scenario?
Comment 5•4 years ago
|
||
We use on trees the system colour -moz-FieldText
.
Comment 6•4 years ago
|
||
On toolbars we detect the text colour through toolbarIconColor.js. Maybe something similar could be possible for the tree.
I see that the status of this bug is still unconfirmed. I can confirm that this bug still exist in Thunderbird 78.0 on Linux (Ubuntu with Adwaita dark).
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 8•4 years ago
|
||
Richard, is this something you might be able to tackle?
Comment 10•4 years ago
|
||
I tried it but wasn't successful.
Assignee | ||
Comment 11•4 years ago
|
||
I'll get a stab at this.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 12•4 years ago
|
||
The issue goes a little bit deeper as also some parts of the calendar are not properly styled.
The quick solution would be to tell the user to go to the Add-ons section and select the dark theme, but we should that automatically.
I have a dirty patch we could use as a starting point, which tackles also bug 1592210.
Assignee | ||
Comment 13•4 years ago
|
||
This is more a proof of concept than an actual patch.
We currently have a method in the ThemeVariableMap.jsm
which runs at startup and whenever a theme is changed.
https://searchfox.org/comm-central/rev/eb69eff3ca16509af6cf8f2dd29be396bebbc14d/mail/base/modules/ThemeVariableMap.jsm#162
We could use it and check for the colour contrast of the first toolbar we pick.
There's a small problem sometimes where the method runs before the entire UI switched back to light, therefore failing the contrast check and still returning a dark UI.
I think, and this is just speculation, that we might improve this approach by forcing a theme switch to the dark variation in case the user has the default theme selected, in order to cover these scenarios:
- The OS has a dark theme -> the theme is on default (not edited by the user) -> we add the
brighttext
attribute and theui.systemUsesDarkTheme
. - The OS has a dark theme -> the theme is not default (edited by the user) -> we don't do anything.
- The user switches from default to dark -> we add the
brighttext
attribute and theui.systemUsesDarkTheme
.
Thoughts?
P.S.: I noticed that the observer in the toolbarIconColor.js
basically runs on every window interaction (clicks, window drag, resize, etc.). We should look into that and limit it as I don't think that's good for performance.
Assignee | ||
Updated•4 years ago
|
Comment 14•4 years ago
|
||
Assignee | ||
Comment 15•4 years ago
|
||
with the default Ubuntu theme, which has dark menu/tab toolbars but the rest light, it uses the dark setting.
We need an element that we can safely use to detect the color, do you have anything in mind?
opening the AB or the Composer shows errors and it resets to light.
Ah, I guess that method runs for every new window that is opened, which it shouldn't. Could we detect if we're dealing with the main window or a dialog?
Comment 16•4 years ago
|
||
(In reply to Alessandro Castellani (:aleca) from comment #15)
with the default Ubuntu theme, which has dark menu/tab toolbars but the rest light, it uses the dark setting.
We need an element that we can safely use to detect the color, do you have anything in mind?
Maybe we should use the #folderTree. Check a toolbar text colour could be dangerous because when a dark LW-theme is active, the toolbar text could be light but the normal TB content uses still black text. The only toolbar we could use would be the #folderPane-toolbar but this isn't always shown. Also the window itself can't be used because of the LW-themes applying to it.
Assignee | ||
Comment 17•4 years ago
|
||
Here's another try for this.
I did a bunch of changes, so here's a recap of the reasons why I implemented those things:
Remove extra calls
I explored the code a bit more thoroughly and discovered that the inferFromText()
was getting called multiple times.
From here: https://searchfox.org/mozilla-central/rev/c6676771df58c6e0098574bc6b11517acbf264cf/toolkit/components/extensions/parent/ext-theme.js#398
And here: https://searchfox.org/comm-central/rev/caacc41c6d79e8e4015698bd23a60dbc5a957f1c/mail/base/content/msgMail3PaneWindow.js#2004
If I'm not wrong, all those calls are not necessary since we're relying on the "lightweight-theme-styling-update"
observer triggered from the ext-theme.js
.
I removed the extra call and I don't see any UI regression, but I might have missed something.
Prevent unnecessary attribute changes
The inferFromText()
method runs a lot, like at every click, move, resize, or window opening, and it runs 2 or 3 times for every call (I'm still trying to understand why and if I can prevent that).
I implemented a kCurrentColor
variable to store the most recent color from the main tabmail. If that color didn't change since the last time we called this method, I don't thing we should run it again because, hopefully, nothing changed and we don't need to update the attributes on all our toolbars.
Switching to ui.systemUsesDarkTheme
I'm using the ThemeVariableMap.jsm
to detect the current color and update the attributes accordingly.
The processColor()
method runs only at startup, or when a new window is opened, so if the user changes the OS theme to a dark theme, he'll need to restart TB to see all the changes properly applying.
This is not necessary if the user changes the TB built-in theme to Dark, fixing bug 1592210.
Let me know if you find anything sketchy, or if the performance gets affected by these changes.
Comment 18•4 years ago
|
||
Assignee | ||
Comment 19•4 years ago
|
||
Yeah, that's a limitation of this implementation, as I wrote in comment 17:
if the user changes the OS theme to a dark theme, he'll need to restart TB to see all the changes properly applying.
I'm not sure if I can find a way to let it update dynamically without restart and without hogging the performance.
Comment 20•4 years ago
|
||
It's not so bad when a restart is needed. I don't think, changing the system theme is a regularly thing the user does.
Comment 21•4 years ago
|
||
Assignee | ||
Comment 22•4 years ago
|
||
Thanks for the review.
I still have 1 issue I'm not sure how to solve.
Switching from the built-in TB dark theme to default, doesn't properly update the ui.systemUsesDarkTheme
, since the UI colors are updated slightly after the processColor()
finished running, so it still detects the previous color scheme.
I'm not sure this is an acceptable behaviour.
The theme fixes itself after a restart.
Comment 23•4 years ago
|
||
Comment 24•4 years ago
|
||
(In reply to Alessandro Castellani (:aleca) from comment #22)
Switching from the built-in TB dark theme to default, doesn't properly update the
ui.systemUsesDarkTheme
, since the UI colors are updated slightly after theprocessColor()
finished running, so it still detects the previous color scheme.
If I understand correctly, what it boils down to is trying to find the CSS value of -moz-dialogtext
. Is there some other way to do this? One way would be to find an element or property that doesn't change with the theme, but there may be some less hacky way.
(In reply to Richard Marti (:Paenglab) from comment #23)
A workaround would be when going from dark theme to
default to use the light theme and then switch to default theme.
I think this effectively happens if you disable the current theme to go back to the default, as opposed to enabling the default. Which is a bit weird and probably not at all useful, but I thought I'd mention it.
Comment 25•4 years ago
|
||
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 27•4 years ago
|
||
All right, thanks for the feedback.
I think this effectively happens if you disable the current theme to go back to the default.
Correct, when disabling the dark theme, TB first switches to light and then to default. We could force the same workflow when a user selects default.
If I understand correctly, what it boils down to is trying to find the CSS value of -moz-dialogtext. Is there some other way to do this? One way would be to find an element or property that doesn't change with the theme, but there may be some less hacky way.
I tried to find a way but I couldn't, probably due to my lack of knowledge of this area.
Assignee | ||
Comment 28•4 years ago
|
||
Since this issue affects 78, should we uplift?
Maybe we should let it sit on beta for a while and keep an eye on for any regression, and use the time to explore improvements.
Assignee | ||
Updated•4 years ago
|
Reporter | ||
Comment 29•4 years ago
|
||
IMO this definitely needs uplift as it's a regression from the previous ESR, breaking TB out-of-the-box for many users (myself included).
Assignee | ||
Comment 30•4 years ago
|
||
As a temporary workaround you can set the Theme to dark, until this patch lands in the next release.
Comment 31•4 years ago
|
||
Pushed by geoff@darktrojan.net:
https://hg.mozilla.org/comm-central/rev/eb5bf594244d
Fix default color of folder icons with Adwaita Dark theme. r=darktrojan
Comment 35•4 years ago
|
||
He's away this and next week, but yes we should uplift.
Comment 36•4 years ago
|
||
Comment on attachment 9166660 [details] [diff] [review]
1638233-folders-dark.diff
[Approval Request Comment]
User impact if declined: some parts of the openpgp UI unreadable, see bug 1658695 too
Updated•4 years ago
|
Comment 38•4 years ago
|
||
Comment on attachment 9166660 [details] [diff] [review]
1638233-folders-dark.diff
[Triage Comment]
Approved for beta
Approved for esr78
Because this hasn't gone through beta - we need this explicitly tested with 78 candidate when it becomes available - will post on tb-planning (CCing some likely suspects)
Comment 39•4 years ago
|
||
bugherder uplift |
Thunderbird 78.2.0:
https://hg.mozilla.org/releases/comm-esr78/rev/32f2ba79db84
Comment 40•4 years ago
|
||
bugherder uplift |
Thunderbird 80.0b4:
https://hg.mozilla.org/releases/comm-beta/rev/cd54d39be1c8
Updated•4 years ago
|
Comment 41•4 years ago
|
||
I updated my TB 80.0b3 to 80.0b4 on Ubuntu 18.04.5 LTS with the Adwaita OS theme.
Changed the TB Dark theme I prefer to the Default theme.
Restarted Thunderbird.
Opened Tweaks and changed to the Adwaita Dark theme.
TB 80.0b4 looked like the reporter's screenshot in the bug description.
Restarted TB 80.0b4 and all returned to looking good.
I think I was beginning to test this in the release candidate when I discovered the other theme bugs I filed and quit testing.
Will test again with the 78.2.0 rc.
Comment 42•4 years ago
|
||
Dark theme looks OK on the build sent out via the mailing list (about window reports "78.2.0").
High contrast theme looks fine too.
(This is running under Gnome on Debian, switching the "Applications" appearance via gnome-tweaks)
There was an issue where switching between themes while Thunderbird was running left it a little confused - like the background colours changed, but a bunch of the icons and text seemed wrong.
Worked fine when you switch themes while thunderbird isn't running.
(all a bit alien to an I3 user like me :-) Themes and desktops are usually just things that happen to other people)
Hope this is what you were looking for - let me know if not and I can try some other stuff.
Comment 43•4 years ago
|
||
Oh - forgot to mention that the message display pane seems unaffected by theme changes. I guess HTML emails would be problematic, but I'd have thought text emails would follow the theme (ie light text on dark background).
Comment 44•4 years ago
|
||
The dark message display was reverted because of too much issues with HTML messages. Differ between plain text and HTML messages would also lead to reactions to: "Why are some messages dark and others not?" for users that don't understand the difference between plain and HTML.
Comment 45•4 years ago
|
||
Yes, a better solution for the message pane is tracked in bug 1657833.
Comment 46•4 years ago
|
||
The Folder Pane icons are still not visible with the Thunderbird Default theme, then just applying the Adwaita-dark operating system theme with Thunderbird open using:
Name Thunderbird
Version 78.2.0
Build ID 20200814165000
Also TB 80.0b5.
A restart of Thunderbird and they become visible.
Switching back to my default Adwaita theme with Thunderbird open, I lost all Folder pane and Thread Pane content and had to restart to recover the content.
Appears to work fine if you change operating system themes with Thunderbird closed.
Comment 47•4 years ago
|
||
Based on the results in comment 46 I'm going to reopen this bug.
I still see the same testing the current 78.2.0 release candidate build (20200821101218) on Ubuntu 18.04.5 LTS.
Assignee | ||
Comment 48•4 years ago
|
||
(In reply to WaltS48 [:walts48] from comment #47)
Based on the results in comment 46 I'm going to reopen this bug.
I still see the same testing the current 78.2.0 release candidate build (20200821101218) on Ubuntu 18.04.5 LTS.
This has been reported in comment 22, in this quote:
Switching from the built-in TB dark theme to default, doesn't properly update the ui.systemUsesDarkTheme, since the UI colors are updated slightly after the processColor() finished running, so it still detects the previous color scheme.
I'm not sure this is an acceptable behaviour.
The theme fixes itself after a restart.
Both Geoff and Richard approved this and it's an expected behaviour that will be fixed in a follow up bug.
Assignee | ||
Updated•3 years ago
|
Description
•