Centralize dark/light theme decisions and provide reasonable fallback when toolbar color isn't set.
Categories
(Firefox :: Theme, task)
Tracking
()
Tracking | Status | |
---|---|---|
firefox96 | --- | fixed |
People
(Reporter: emilio, Assigned: emilio)
References
Details
Attachments
(5 files)
STR:
- Install a theme that doesn't provide a toolbar color like https://addons.mozilla.org/en-US/firefox/addon/a-n-i-m-a-t-e-d-kitty-cat/
ER:
- Toolbar-color should probably be considered light since it does provide a frame color.
AR:
- Toolbar color is not there, so we fall back to "system", which might be dark and look a bit off.
Assignee | ||
Comment 1•3 years ago
|
||
The code caught my attention because it broke my editor's syntax
highlighting. This should be more correct.
Updated•3 years ago
|
Assignee | ||
Comment 2•3 years ago
|
||
This shouldn't change behavior.
Depends on D130668
Assignee | ||
Comment 3•3 years ago
|
||
Depends on D130669
Assignee | ||
Comment 4•3 years ago
|
||
After the previous patches all this should be unnecessary.
Assignee | ||
Comment 5•3 years ago
|
||
(I ni?d because I can't r?)
Comment 7•3 years ago
|
||
Backed out for causing bc failures on browser_ext_browserAction_theme_icons.js.
The affected platforms are Linux 18.04 x64 WebRender opt and debug.
Comment 8•3 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)
Created attachment 9249840 [details] [diff] [review]
comm-central cleanupAfter the previous patches all this should be unnecessary.
I'll let Alessandro make the review as it is his code that's changed.
Assignee | ||
Updated•3 years ago
|
Comment 10•3 years ago
|
||
Maybe is due to D130670 not in m-c yet, but this patch doesn't work unfortunately.
Switching to dark theme doesn't properly update the in-content style (prefs, account settings, account setup, etc.). Partial sections of the UI and other pages are not properly updated with an hybrid state of light text and light bg.
Thanks for thinking about us and keeping us in the loop :D
Assignee | ||
Comment 11•3 years ago
|
||
Well the other two patches aren't in central either so that's not too surprising. After that happens it should work :)
Comment 12•3 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #11)
Well the other two patches aren't in central either so that's not too surprising. After that happens it should work :)
I'm not a smart man, I didn't see it was on autoland but not on m-c.
I'll circle back to this and test it again, thanks and sorry for noise.
Comment 13•3 years ago
|
||
bugherder |
Comment 14•3 years ago
|
||
Updated•3 years ago
|
Comment 15•3 years ago
|
||
Emilio, can I mark this for checkin-needed-tb
only for the patch you made for TB?
Assignee | ||
Comment 16•3 years ago
|
||
I suppose? I can push it otherwise I guess :)
Comment 17•3 years ago
|
||
Comment 18•3 years ago
|
||
bugherder |
Assignee | ||
Updated•3 years ago
|
Updated•3 years ago
|
Updated•3 years ago
|
Comment 19•3 years ago
|
||
Can you push the comm-central cleanup
patch attached to this bug?
Comment 20•3 years ago
|
||
Updated•3 years ago
|
Updated•3 years ago
|
Comment 21•3 years ago
|
||
Hello Emilio! I'm having some trouble reproducing the issue with Firefox 96.0a1 (20211108214919) on Windows 10x64, macOS 10.15, and Ubuntu 21.1. For me when installing the theme provided in comment 0, the toolbar color is white as well on the affected build even if the system color is dark.
Am I doing something wrong here? Attached screenshot of Windows 10x64 with the affected and fixed build.
Or maybe could you verify the fix on the latest beta if you have the time? Thank you in advance.
Assignee | ||
Comment 22•3 years ago
|
||
The toolbar is always, light, but before the patch we choose a dark color for content (erroneously). With the theme installed both the UI and the content color (i.e., about:support backgrounds etc) should be light.
Description
•