1838460 - Using titlebar colour to render menu bar & tabs looks badly in some themes

Status: RESOLVED FIXED

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

Description

[Harald van Dijk]

Attachment: Firefox Theme.png

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

Steps to reproduce:

• Change layout to show title bar and menu bar
• Upgrade from Firefox 113.0b9 to Firefox 115.0b4

Actual results:

Menu and tab bar show using titlebar colour

Expected results:

Menu and tab bar should show as they used to

Please see the attached screenshot. I am using the BeOS window decorations https://www.gnome-look.org/p/1078568 and Haiku controls https://github.com/B00merang-Project/Haiku.

I mentioned this in a comment in bug 1704086, thinking it was the same issue (the background colour may be different now, but it's still not right), where Emilio requested I file this as a separate bug.

Comment 1

[Harald van Dijk]

@Emilio Does this provide all the required information? If you need anything else let me know and I will try to answer.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Widget: Gtk' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Yeah, so not sure what the right heuristic here should be... Maybe if there's a client offset we should use the window or toolbar background colors?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: beos theme

So that's a gtk2 theme, but with the latest version of https://github.com/B00merang-Project/beOS this looks pretty ok to me on GNOME?

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Haiku theme

The haiku theme in comment 0 also looks fine to me, see screenshot.

Do you know about other details of your setup that can get me to see what you're seeing? Maybe the window manager is different?

In general I think the new thing looks good for most cases, but I can see how if you have weird window manager stuff it might look a bit weirder, specially with themes with bright titlebars.

Comment 6

[Harald van Dijk]

The difference between your setup and mine, I think, is that you left the Firefox default setting to hide the system title bar and render tabs in a custom title bar. For that setup, it makes sense that the titlebar colour is used for the tab bar. With browser.tabs.inTitlebar=0 (or , I expect you will see the same as what I have shown. I also have the menu bar always visible, which changes things a little bit more.

Comment 7

[Harald van Dijk]

Apologies, submitted too soon by mistake. The (or was supposed to be a reference to where the option can be changed without going to about:config (Customize Firefox > Title Bar), because I understand that problems that only arise through manual changes through about:config are generally considered a lower priority.

Comment 8

[Harald van Dijk]

Oh no, wait, that isn't it at all, you did make that same change. And, you are actually seeing the same thing I am, except for not having unhidden the menu bar. Your window manager's title bar is slightly different, but the important part there is that the tab bar has that yellowish background. It is made to look like part of the title bar in your screenshot too, and that confused me. Is the point of the "Title Bar" setting not just to get the system title bar? Having the system title bar but then also painting the content to make it look like you aren't getting the system title bar seems like it defeats the point somewhat.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Not really, those screenshots use the native titlebar, see the window title above the tab bar, that's not Firefox UI

Comment 10

[Harald van Dijk]

Yes, I was wrong about that in comment #6, and wrote so in comment #8. Apologies for the confusion.

Comment 11

[Emilio Cobos Álvarez (:emilio)]

(In reply to Harald van Dijk from comment #8)

Oh no, wait, that isn't it at all, you did make that same change. And, you are actually seeing the same thing I am, except for not having unhidden the menu bar. Your window manager's title bar is slightly different, but the important part there is that the tab bar has that yellowish background. It is made to look like part of the title bar in your screenshot too, and that confused me. Is the point of the "Title Bar" setting not just to get the system title bar? Having the system title bar but then also painting the content to make it look like you aren't getting the system title bar seems like it defeats the point somewhat.

Err sorry I had missed this one. It depends, for most themes, the titlebar colors are quite close and making the tab bar melt with the titlebar and react to activeness etc makes sense. Having very diverging styles depending on whether the system titlebar is enabled is also quite confusing.

Comment 12

[Harald van Dijk]

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

Having very diverging styles depending on whether the system titlebar is enabled is also quite confusing.

I have to disagree here. We are talking about a config option that is literally called "tabs in title bar". It makes perfect sense that when "tabs in title bar" is disabled, tabs are not made to look as if they are in the title bar, and when it is enabled, they are. Arguing that it looks better to always draw them as if they are in the title bar is in effect arguing to remove this config option, or to reduce its behaviour by so much that it becomes, for me, meaningless.

In bug 1704086, you referenced bug 1832732, which links to https://hg.mozilla.org/mozilla-central/rev/5868f3e8b379 / https://phabricator.services.mozilla.com/D177882. There, you wrote:

This shouldn't meaningfully change behavior (menubar and titlebar colors
are virtually always the same in all themes I tried), and we actually
react to inactive window state, so it looks nice.

Knowing now that this did in fact meaningfully change behaviour, is that not already sufficient reason to reconsider?

This does not need to be an invasive change or a big revert. Looking at that commit (thanks for the reference), I tried to hack together something locally, and I am seeing good results with simply

--- a/browser/themes/linux/browser.css
+++ b/browser/themes/linux/browser.css
@@ -140,12 +140,12 @@ menuitem.bookmark-item {
   min-height: 0;
 }
 
-#titlebar {
+:root[tabsintitlebar] #titlebar {
   background-color: ActiveCaption;
   color: CaptionText;
 }
 
-#titlebar:-moz-window-inactive {
+:root[tabsintitlebar] #titlebar:-moz-window-inactive {
   background-color: InactiveCaption;
   color: InactiveCaptionText;
 }

If tabs in title bar is enabled, or if a Firefox theme is used that specifies a background, this change also has no effect. This change is limited to no-tabs-in-title-bar, and the change it makes is that the title bar look is never used in that case, which I would say is the correct behaviour.

While I can see a logic for saying that if the active title bar and toolbar colours are the same, or roughly the same, then in inactive state the inactive title bar colour should also be used for the toolbar, this logic already does not work with system title bars for the simple reason that Firefox's idea of what the inactive title bar colour is wrong in that case. I use the MATE desktop environment. One of the system themes is BlackMATE. If you try this theme, you will see that active and inactive windows alike have an almost identical dark background gradient. They are distinguished more by a different font colour, and different window buttons. Firefox, however, makes ActiveCaption a dark black and InactiveCaption a much lighter grey, and applies that style to the menu and tool bars. The result is that the attempt to make the tool bar match the title bar has the opposite effect.

Comment 13

[Emilio Cobos Álvarez (:emilio)]

The above patch just renders the default window color / fg so it wouldn't be quite correct either fwiw.

Comment 14

[Harald van Dijk]

You're right, it's not a complete fix to make it exactly like how Firefox 113 rendered the menu bar and toolbars, but it's as close as I think we can get unless your change to bug 1831841 is reverted: if the code to use draw menu bars as GTK would is no longer there, it's no longer there, there is just no way to use code that isn't there. The window colour is the next best thing, it is close, and it turns Firefox 115 from something that I am not going to use into something I have no problem using.

Ideally the code removed in that bug would be reinstated, but that would be a larger change. I can certainly make that change, but I have no idea whether that has even a remote chance of being acceptable. If it is not, if Firefox keeps its current behaviour, I need a local change I am capable of maintaining. The revert of that bug would cause problems for me going forward. This trivial CSS change does not.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

We still have code to extract menubar colors if needed, it's just that for most users I think the current behavior is better. You can also go to Extensions and use the light theme right? Which gives you something very similar but without extra patches. I guess a bit less dark than before tho.

Comment 16

[Ahmet Sait]

Attachment: Kate Theme (Without Header Color)

Comment 17

[Ahmet Sait]

Attachment: Kate Theme (With Header Color)

Comment 18

[Ahmet Sait]

I'm extremely annoyed by this change. I've specifically made sure that menus/headers are not the same color as title bar in my KDE environment: https://bugzilla.mozilla.org/show_bug.cgi?id=1838460#c16

And yet Firefox behaves as if I modified the [Colors:Header] option in my color scheme: https://bugzilla.mozilla.org/show_bug.cgi?id=1838460#c17

This is clearly a regression and I completely agree with Harald.

Comment 19

[Emilio Cobos Álvarez (:emilio)]

Didn't know this was exposed more generally in your KDE settings. Quick Qs:

• Can you attach a Firefox screenshot pre/post change with your settings?
• Presumably the expectation is the same as harald (tab bar background colors should be different when Firefox renders tabs to titlebar vs. not?)

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Anyways putting on my list of things to look at. Let me try to come up with a not-too-gross solution.

Comment 21

[Ahmet Sait]

Attachment: 2023-07-05 22-44-03 Mozilla Firefox.png

I'm not sure what you mean by "pre/post change with your settings" but this is how it looks right now with browser.tabs.inTitlebar=0 regardless of [Colors:Header] settings.

Comment 22

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1838460 - Deindent linux titlebar CSS. r=stransky,dao

We don't set the tabsintitlebar attribute when not supported anyways.

This fixes the titlebar buttons when in fullscreen for clients where CSD
isn't supported, and it's also simpler.

Comment 23

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1838460 - Don't use titlebar colors when not drawing tabs to the titlebar. r=stransky,dao

This restores our previous behavior in a simpler way.

This brings back bug 1704086 (which was "fixed" by bug 1832732, due to
us using the titlebar colors on Adwaita). I think that's slightly
unfortunate. We could fix it by special-casing Adwaita or something, but
probably not worth it since the default in GNOME has been drawing tabs
to the titlebar for a long while.

Depends on D182887

Comment 24

[Emilio Cobos Álvarez (:emilio)]

Attachment: Kde color settings.

Actually playing a bit more with KDE color settings it seems the default is to use the titlebar color. So this might be a bit more complicated.

Comment 25

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1832732

Comment 26

[Harald van Dijk]

Please note also that the menubar background need not be a single colour, and indeed is not on my theme: it uses a gradient instead. You wrote in comment 15 "We still have code to extract menubar colors if needed" but that approach cannot make it look like it did in Firefox 113, which did pick up the gradient. For me, it isn't necessary to restore that to make it look acceptable, but I am noting it to make sure it does not get missed by accident.

Comment 27

[Ahmet Sait]

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

Created attachment 9342552 [details]
Kde color settings.

Actually playing a bit more with KDE color settings it seems the default is to use the titlebar color. So this might be a bit more complicated.

Beware of "Active/Inactive Titlebar" colors are visible and editable but confusingly unused when Header colors are present if you're trying to understand how KDE color schemes work. If you set Header > Normal Background in settings UI Common Colors > Active Titlebar is ignored. You can remove [Colors:Header] section in the corresponding file in ~/.local/share/color-schemes which will make Common Colors > Active Titlebar effective again.

Comment 28

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1838460 - [GTK] Store theme colors in pairs when possible. r=stransky

This shouldn't change behavior, but it's cleaner.

Depends on D182887

Comment 29

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1838460 - Remove unused moz-menubartext color. r=stransky

-moz-menubarhovertext has one usage that can go away once we remove
windows 7 / 8 so not touching that yet.

Depends on D182896

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4296cbaf7505
Deindent linux titlebar CSS. r=dao

Comment 31

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5f2146e87736
Remove unused moz-menubartext color. r=geckoview-reviewers,devtools-reviewers,nchevobbe,amejiamarmol

Comment 32

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/4296cbaf7505
https://hg.mozilla.org/mozilla-central/rev/5f2146e87736

Comment 33

[Donal Meehan [:dmeehan]]

:emilio do you think you'll have something safe to uplift for 116 at least?
Not sure the priority/severity, if those could be set

Comment 34

[Emilio Cobos Álvarez (:emilio)]

Depends on how fast is reviewed? I think I'd rather not uplift tho.

Comment 35

[Harald van Dijk]

In response to my trivial two-line patch, you wrote in comment #13:

The above patch just renders the default window color / fg so it wouldn't be quite correct either fwiw.

Yet in D182888, you appear to do exactly that for every theme other than Adwaita and Breeze, unless I am misreading this line:

mHeaderBar = mHeaderBarInactive = mWindow;

The claim in D182888 that "This restores previous behavior for most themes" seems dubious. I do not believe it can possibly restore previous behaviour with my theme, which as I noted in comment 26 uses a gradient for the menu bar. This gradient was picked up by Firefox 113 and below. Even for themes where this is a single colour, I would be surprised if most of them have it exactly the same as the window colour, as is needed for the claim to be true "for most themes".

I did note that my trivial two-line change makes Firefox look acceptable to me again, so if that is what you are making it look like, I am happy with that improvement, thank you. It should not be presented as something it is not, though. If it does not restore previous behaviour, it should not be presented as restoring previous behaviour.

Comment 36

[Emilio Cobos Álvarez (:emilio)]

(FWIW I did change the patch to read menubar colors)

Comment 37

[Harald van Dijk]

Thanks, with that change, you are probably right that it restores behaviour for most themes. It won't for mine, but it will probably still be more than good enough for mine, and an improvement over the previous version. It'll be a little while till I can try building with your change to make sure; it'll probably be pushed and in a nightly before then, I'll make sure to check that instead.

Comment 38

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1838460 - Remove unused -moz-buttondefault color. r=stransky

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d56da923dd9b
[GTK] Store theme colors in pairs when possible. r=stransky

Comment 40

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/d56da923dd9b

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2bf7a06a7f33
Don't use titlebar colors when not drawing tabs to the titlebar. r=stransky

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9330f8cb5765
Remove unused -moz-buttondefault color. r=stransky

Comment 43

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/2bf7a06a7f33
https://hg.mozilla.org/mozilla-central/rev/9330f8cb5765