Closed Bug 1710643 Opened 3 years ago Closed 3 years ago

[Linux, theme-specific] Background tabs and buttons in the tab strip are not readable with Breeze light theme

Categories

(Core :: Widget: Gtk, defect, P3)

Desktop
Linux
defect

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox89 --- wontfix
firefox90 --- fixed

People

(Reporter: Gijs, Assigned: emilio)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [proton-tabs-bar])

Attachments

(10 files)

Attached image breeze-tabstrip.png

This seems to happen with the "Breeze" GTK theme that was apparently selected in the linux VM I keep around for testing... It's probably not the default so not a lot of people will presumably run into it, but equally the usability of this level of text/bg contrast is... not great.

Screenshot is from a local build from yesterday morning's m-c .

Flags: needinfo?(emilio)

Breeze is important because it's the default theme for KDE users.

Assignee: nobody → emilio
Attached image image.png

Hmm, breeze theme looks fine here. Do you recall where you installed the theme from? Which Linux distro / version are you using?

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

Created attachment 9221361 [details]
image.png

Hmm, breeze theme looks fine here. Do you recall where you installed the theme from?

No. :-(
I tried looking through bugzilla and my bugmail archives but didn't find an obvious culprit.

This is what apt list --installed | grep breeze says:

breeze-cursor-theme/groovy,groovy,now 4:5.19.5-0ubuntu1 all [installed]
breeze-gtk-theme/groovy,now 5.19.5-0ubuntu2 amd64 [installed]
breeze-icon-theme/groovy,groovy,now 4:5.74.0-0ubuntu1 all [installed]
breeze/groovy,now 4:5.19.5-0ubuntu1 amd64 [installed]
kde-style-breeze/groovy,now 4:5.19.5-0ubuntu1 amd64 [installed]
kwin-style-breeze/groovy,now 4:5.19.5-0ubuntu1 amd64 [installed]
libreoffice-style-breeze/groovy-updates,groovy-updates,now 1:7.0.5-0ubuntu0.20.10.1 all [installed]
qml-module-qtquick-controls-styles-breeze/now 4:5.16.5-0ubuntu1 amd64 [installed,local]
sddm-theme-breeze/groovy,now 4:5.19.5-0ubuntu2 amd64 [installed]

Which Linux distro / version are you using?

Ubuntu 20.10 / groovy.

Flags: needinfo?(gijskruitbosch+bugs)

Perhaps worth noting that your screenshot looks similar to what my browser window looks like when it's not the focused/active window. When I focus any window, its titlebar becomes much darker and that then yields the bad-contrast situation from my earlier screenshot.

These are the files from /usr/share/themes/Breeze, in case that helps...

Ok, so I can repro in an ubuntu 20.10 VM. The "good" thing is that this is not a proton regression.

Blocks: gtktitlebar

Can you sanity-check this build when it finishes? https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8b40f333b4ac58d2646d6dcf8ec54776de403ba

It works for me. The appearance when the window is inactive is a bit off because it becomes bright enough so that we don't draw a shadow, and the tab background happens to be exactly the same as the inactive titlebar appearance (because of the theme), but I think we can live with that.

Flags: needinfo?(emilio) → needinfo?(gijskruitbosch+bugs)

Presumably they were for notifications and now we use native notifications?

As the titlebar may be different when active / inactive, and have
other colors than a menubar.

Depends on D114874

Priority: -- → P3

Luminance goes from 0 to 255, so using 127 makes sense, and allows all
disabled titlebar colors that I found in various GTK themes to still be
considered dark enough (for those, 110 was too low).

I noticed while testing the previous patch on various gtk themes that
the cache wasn't cleared / sytem colors weren't recomputed properly when
the native theme changes. This trivial patch fixes it.

Status: NEW → ASSIGNED

(In reply to Emilio Cobos Álvarez (:emilio) from comment #7)

Can you sanity-check this build when it finishes? https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8b40f333b4ac58d2646d6dcf8ec54776de403ba

It works for me. The appearance when the window is inactive is a bit off because it becomes bright enough so that we don't draw a shadow, and the tab background happens to be exactly the same as the inactive titlebar appearance (because of the theme), but I think we can live with that.

Apologies for the delay. This build works for the tabstrip colours but not the menubar. From a brief exploration, I think that's because of https://searchfox.org/mozilla-central/rev/27722db2f164add7047d7db03169966cb806e927/toolkit/themes/linux/global/menu.css#54,56 ?

I also noted that now, when the window is not focused, the tabstrip is illegible - I've attached a screenshot of that state.

I wonder if instead of the change to the luminance threshold, we should really evaluate the contrast with the foreground colour or something? But I haven't thought about it too much, and perhaps my understanding of the root cause here is incomplete.

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(emilio)

Apologies for the delay. This build works for the tabstrip colours but not the menubar. From a brief exploration, I think that's because of https://searchfox.org/mozilla-central/rev/27722db2f164add7047d7db03169966cb806e927/toolkit/themes/linux/global/menu.css#54,56 ?

Yeah, that's right, we'd also need to use color: inherit on those menus when the tabs are in the titlebar... But that's a bit annoying because then it overrides the color when the menu is active as well, so I'd rather leave that for now as it's a bit more complex.

I also noted that now, when the window is not focused, the tabstrip is illegible - I've attached a screenshot of that state.

That is the color that titles in other unfocused windows have (see incoming screenshot), so I think that's fine. What my patch does is basically using the same colors as the titlebar would have (since the background is the titlebar, after all). If the theme says that is the color for text on top of the titlebar on an unfocused window then so be it, IMO :)

I wonder if instead of the change to the luminance threshold, we should really evaluate the contrast with the foreground colour or something? But I haven't thought about it too much, and perhaps my understanding of the root cause here is incomplete.

To be clear the luminance threshold only changes whether we draw a shadow around the active tab in the unfocused window (which is useful since that is using a different background, it's just that in this particular theme, the unfocused titlebar background and the background we use for the active tab is the same). It preserves that behavior also in other more popular themes which also dim colors on the titlebar for unfocused windows.

Flags: needinfo?(emilio)
Keywords: leave-open
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/91d0ca6e1ddf
Clear luminance cache on native theme changes. r=Gijs
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/602011cd698c
Remove support for unused gtk infobar widget and colors. r=stransky
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/a9c41ba93f80
update property database to account for removed color.
Pushed by emilio@crisal.io:
https://hg.mozilla.org/integration/autoland/rev/e2e39e32a588
Fix encoding of the theme change kind so it doesn't crash when getting converted to JS if non-zero.
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/130b698d875c
Use titlebar colors when using CSD rather than -moz-menubartext. r=stransky
https://hg.mozilla.org/integration/autoland/rev/254860ea3dde
Use an slightly higher threshold to consider a color dark. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/a29e0122f130
Fix menubar color too. r=Gijs

I think this can be resolved fixed now? :-)

Flags: needinfo?(emilio)
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: needinfo?(emilio)
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
Regressions: 1711308
No longer regressions: 1711308
Regressions: 1722588
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: