Closed Bug 1741095 Opened 4 years ago Closed 4 years ago

91.3.1 dark theme broken in add-ons manager/account manager

Categories

(Thunderbird :: Theme, defect)

Product:
Thunderbird
Component:
Theme
Version:
Thunderbird 91
Type:
defect

Tracking

(thunderbird_esr91+ verified, thunderbird95 unaffected, thunderbird96 unaffected)

Status:
VERIFIED FIXED
Milestone:
91 Branch
Tracking Flags:
Tracking Status
thunderbird_esr91 + verified
thunderbird95 --- unaffected
thunderbird96 --- unaffected

People

(Reporter: bugzilla, Assigned: Paenglab)

References

(Regression)

Details

(Keywords: regression)

Attachments

(2 files)

screen capture.png
57.44 KB, image/png
Details
1741095-rename-prefers-color-scheme-media-query.patch
15.33 KB, patch
aleca
: review+
wsmwk
: approval-comm-esr91+
Details | Diff | Splinter Review
Attached image screen capture.png —

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:91.0) Gecko/20100101 Firefox/91.0

Steps to reproduce:

Install TB 91.3.1

Actual results:

The dark theme is broken. See the screen capture: the left menu is almost unreadable. Also, the content of the events is white again. This was just a quick test and many other things could be broken.
What’s worse is that it messed my profile files. I reverted to TB 91.3.0 and I got the same result.
I also started 91.3.1 on an empty profile to confirm the bug.

However, when I start 91.3.0 on an empty profile, the dark theme is as it used to be.

I’m so disappointed about how TB has been tested before the release of new versions during the last one or two years. Regressions or badly coded functions are quite frequent.

Also, why are release notes only available the next day after the files are published? I would expect the contrary.

Component: Untriaged → Theme

I can confirm this bug. Only about add-ons manager it seems.
https://hg.mozilla.org/releases/comm-esr91/pushloghtml?fromchange=THUNDERBIRD_91_3_0_RELEASE&tochange=THUNDERBIRD_91_3_1_RELEASE

Must be from bug 1736252.
I don't see the problem on trunk. Would otherwise just back out from 91. BUT, that bug had a migration so that may not be good option. https://hg.mozilla.org/releases/comm-esr91/rev/4a45ee2ddd82141ae805a6bbeb1f8259ac11acc2#l2.31

Status: UNCONFIRMED → NEW
status-thunderbird_esr91: --- → affected
tracking-thunderbird_esr91: --- → +
Ever confirmed: true
Keywords: regression
Regressed by: 1736252
Summary: 91.3.1 dark theme broken → 91.3.1 dark theme broken in add-ons manager

Account manager as well.

Summary: 91.3.1 dark theme broken in add-ons manager → 91.3.1 dark theme broken in add-ons manager/account manager

There are a lot more places: every place that uses the @media (prefers-color-scheme: dark) {} media query needs now to be changed to @media (-moz-toolbar-prefers-color-scheme: dark) {}.

This media query was removed in a later version and @media (prefers-color-scheme: dark) {} is used again.

I only tested this inside the Browser Toolbox by changing the query and it worked for me.

Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9250745 - Flags: review?(mkmelin+mozilla)
Attachment #9250745 - Flags: review?(mkmelin+mozilla) → review?(alessandro)

There are a lot more places: every place that uses the @media (prefers-color-scheme: dark) {} media query needs now to be changed to @media (-moz-toolbar-prefers-color-scheme: dark) {}.
This media query was removed in a later version and @media (prefers-color-scheme: dark) {} is used again.

Wait what? This is brutal.
So not using ui.systemUsesDarkTheme doesn't trigger the prefers-color-scheme: dark?

I'm pinging Emilio to see if he has any suggestion.
Otherwise this patch needs to directly be uplifted to 91 without landing on trunk, which is not good at all as we shouldn't skip a small testing period on trunk or beta as this is a substantial change to our CSS.

I don't see the problem on trunk. Would otherwise just back out from 91. BUT, that bug had a migration so that may not be good option. https://hg.mozilla.org/releases/comm-esr91/rev/4a45ee2ddd82141ae805a6bbeb1f8259ac11acc2#l2.31

That migration only clears a pref, which is dynamically added or removed based on the color contrast detection of the toolbar.
Is it dangerous to back that out?

status-thunderbird95: unaffected → ---
status-thunderbird96: unaffected → ---
Flags: needinfo?(emilio)

(In reply to Alessandro Castellani [:aleca] from comment #4)

There are a lot more places: every place that uses the @media (prefers-color-scheme: dark) {} media query needs now to be changed to @media (-moz-toolbar-prefers-color-scheme: dark) {}.
This media query was removed in a later version and @media (prefers-color-scheme: dark) {} is used again.

Wait what? This is brutal.

Well, -moz-toolbar* that was what the UI pages were supposed to be using.

So not using ui.systemUsesDarkTheme doesn't trigger the prefers-color-scheme: dark?

That is right (in 91).

I'm pinging Emilio to see if he has any suggestion.
Otherwise this patch needs to directly be uplifted to 91 without landing on trunk, which is not good at all as we shouldn't skip a small testing period on trunk or beta as this is a substantial change to our CSS.

The patch is the right thing to do. It can't land on trunk anyways because the media query doesn't exist there.

I don't see the problem on trunk. Would otherwise just back out from 91. BUT, that bug had a migration so that may not be good option. https://hg.mozilla.org/releases/comm-esr91/rev/4a45ee2ddd82141ae805a6bbeb1f8259ac11acc2#l2.31

That migration only clears a pref, which is dynamically added or removed based on the color contrast detection of the toolbar.
Is it dangerous to back that out?

I don't think it is (but you probably want to leave the migration number as is).

Flags: needinfo?(emilio)

Magnus, what should we do here?

I'm okay with reviewing this exclusively on 91, but we'll need to uplift it directly without landing on trunk.
Otherwise a patch to back up bug 1736252 without changing the migration number might be another solution.

Flags: needinfo?(mkmelin+mozilla)

I'd guess just doing the 91-only patch might be best.

Flags: needinfo?(mkmelin+mozilla)
Comment on attachment 9250745 [details] [diff] [review] 1741095-rename-prefers-color-scheme-media-query.patch Review of attachment 9250745 [details] [diff] [review]: ----------------------------------------------------------------- This is good and it fixes everything, good job. Thank you so much Rob for the 91 build, which made the review process way easier.
Attachment #9250745 - Flags: review?(alessandro) → review+

(In reply to maison from comment #0)

What’s worse is that it messed my profile files. I reverted to TB 91.3.0 and I got the same result.

The update didn't mess your profile at all as the themeing doesn't affect profile files.
The only thing different is that we don't update anymore the ui.systemUsesDarkTheme, which is what's causing the visual issues.
You can temporarily fix this by entering the config editor and setting ui.systemUsesDarkTheme to 1.
Remember to clear that pref after you receive the next update.

I’m so disappointed about how TB has been tested before the release of new versions during the last one or two years. Regressions or badly coded functions are quite frequent.

What badly coded functions are you referring to? Can you share some pointers?
This issue in particular is very unique as it's a mix of uplifted code that works flawlessly on beta and daily, but it's tightly dependent on code from Firefox (m-c) which is not available on 91.

Also, why are release notes only available the next day after the files are published? I would expect the contrary.

We usually publish them on the same day, but sometimes the DNS and cache take time to refresh. And it also happens that the release notes need some revision and they're slightly delayed.

Comment on attachment 9250745 [details] [diff] [review]
1741095-rename-prefers-color-scheme-media-query.patch

A special bug which does no go the normal way through c-c. Should we close the bug to let it appear in the normal approval filters?

[Approval Request Comment]
User impact if declined: on multiple places wrong colours which made them unreadable
Testing completed (on c-c, etc.): this is a ESR only bug
Risk to taking this patch (and alternatives if risky): there is already a try build which works

Flags: needinfo?(rob)
Attachment #9250745 - Flags: approval-comm-esr91?
See Also: → 1741351

Comment on attachment 9250745 [details] [diff] [review]
1741095-rename-prefers-color-scheme-media-query.patch

[Triage Comment]
Approved for esr91 ... pending testing / verification of try build

Attachment #9250745 - Flags: approval-comm-esr91? → approval-comm-esr91+

I am wondering if my installation is affected by this very bug, too.

I use the dark theme on macOS Big Sur 11.6 but have the light theme in TB 91.3.1. Most TB windows are fine (i.e. white background) but with the most recent update the dark pushes through in some windows. Most notable and thus most annoying is the message compose window.

Here's how that looks https://snipboard.io/2tQgKh.jpg

I found even more cases with black instead of white background: https://snipboard.io/XFS5NB.jpg and https://snipboard.io/hD1RgG.jpg

(In reply to Alessandro Castellani [:aleca] from comment #10)

(In reply to maison from comment #0)

What’s worse is that it messed my profile files. I reverted to TB 91.3.0 and I got the same result.

The update didn't mess your profile at all as the themeing doesn't affect profile files.
The only thing different is that we don't update anymore the ui.systemUsesDarkTheme, which is what's causing the visual issues.
You can temporarily fix this by entering the config editor and setting ui.systemUsesDarkTheme to 1.
Remember to clear that pref after you receive the next update.

I’m so disappointed about how TB has been tested before the release of new versions during the last one or two years. Regressions or badly coded functions are quite frequent.

What badly coded functions are you referring to? Can you share some pointers?
This issue in particular is very unique as it's a mix of uplifted code that works flawlessly on beta and daily, but it's tightly dependent on code from Firefox (m-c) which is not available on 91.

Also, why are release notes only available the next day after the files are published? I would expect the contrary.

We usually publish them on the same day, but sometimes the DNS and cache take time to refresh. And it also happens that the release notes need some revision and they're slightly delayed.

I can confirm that changing the "ui.systemUsesDarkTheme" to "1" fixes this issue into Windows 7 too.

Flags: needinfo?(rob)

In my case, most is fine, except the compose mail editor. It has a dark background for the mail content. But Mail content in viewing mails is white/light (and the rest of the application).

I'm using the mentionend thunderbird verison, windows 10, system enabled darkmode, but disabled darkmode in thunderbird.

(In reply to sebastian.loncar from comment #20)

In my case, most is fine, except the compose mail editor. It has a dark background for the mail content. But Mail content in viewing mails is white/light (and the rest of the application).

I'm using the mentionend thunderbird verison, windows 10, system enabled darkmode, but disabled darkmode in thunderbird.

That’s bug 1741416

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED

Verified in my test of the 91.3.2 release candidate on Windows 10.

Status: RESOLVED → VERIFIED
status-thunderbird_esr91: fixed → verified

Just installed 91.3.2 and all but 1 such issues are fixed; the "Edit"/"HTML" tabs when creating HTML messages.

https://snipboard.io/muXzHd.jpg

(In reply to marcel from comment #23)

Just installed 91.3.2 and all but 1 such issues are fixed; the "Edit"/"HTML" tabs when creating HTML messages.

https://snipboard.io/muXzHd.jpg

This is an extension issue you need to inform the extension author.

Sorry for the spam. I've had this extension installed for so many years I completely forgot it's not a TB feature, sorry.

I have similar issues with Dark Theme in drop-down selection menus in the Lightning Calendar.

Note that these started earlier than the issue mentioned here - these problems existed in 91.2.1.

See bug 1742202 for explanations and screenshots.

This is an extension issue you need to inform the extension author.

Tell them what?
What needs to be changed to make things work again?

Target Milestone: --- → 91 Branch

@all, please use "bug nnnnn" when referencing bug reports rather than URL, so that bugzilla can link to the bug.

(In reply to Gordon Lack from comment #27)

This is an extension issue you need to inform the extension author.

Tell them what?
What needs to be changed to make things work again?

Just give them a description of the symptoms and point them to this bug report.

OK. But my problem is with the Lightning Calendar, which is no longer an extension. I also see the problem in the Preferences screen,
These are both part of Thunderbird itself.
There is nowhere else to report it.

(In reply to Gordon Lack from comment #29)

OK. But my problem is with the Lightning Calendar, which is no longer an extension. I also see the problem in the Preferences screen,
These are both part of Thunderbird itself.
There is nowhere else to report it.

It is already reported in bug 1742202.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: