1410712 - Toolbarbutton not white in titlebar when disabled

Status: RESOLVED FIXED

(Thunderbird :: Theme, enhancement)

Description

[Richard Marti (:Paenglab)]

When you have a toolbarbutton in the titlebar, for example the Chat button and have the chat tab selected, the button icon isn't white but light grey.

This comes from the toolkit rule where the disabled toolbarbutton get the GrayText color. We need to override this.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: toolbarbuttonInheritColor.patch

Always, also when disabled, inherit the color from toolbar.

Comment 2

[Jorg K (CEST = GMT+2)]

Another review which I don't understand :-(
"have a toolbarbutton in the titlebar"?? How do I do that? I managed to put the chat button into the menubar with customise. Then, with the chat tab active, that button is faintly grey, with the patch it's still grey, but maybe a bit darker.

I've never seen a white button. So please include STR and a screenshot.

Comment 3

[Richard Marti (:Paenglab)]

You need drawInTitlebar and a dark titlebar color. You can in customize mode move buttons beside the Lightning buttons.

Comment 4

[Jorg K (CEST = GMT+2)]

Attachment: 2017-10-22 22_36_10-Chat - Daily.png

I'd say this is wrong then? The chat button is almost invisible, certainly not white.

Comment 5

[Richard Marti (:Paenglab)]

Yes, it should be white and because it's disabled a bit transparent. Not the focus of this bug but the chat button shouldn't be disabled, when the main tab isn't focused, to better show when a new chat message waits.

Comment 6

[Jorg K (CEST = GMT+2)]

Well, I see no difference with and without the patch. So comment #0 talks about white and light grey, but I see very dark grey for a disabled chat button, maybe due to white + transparency on black background, but it's exactly the same with a current Daily. How do I see the difference due to the patch?

Comment 7

[Richard Marti (:Paenglab)]

A black background wouldn't be optimal to see a difference. Maybe try try a red or green background. Then the icon should not be grey but a lighter red or green. With my blue titlebar Without patch I see almost no icon without the patch.

Comment 8

[Jorg K (CEST = GMT+2)]

Attachment: with and without.png

No difference, with or without the patch on Win7. Please attach a screenshot since I've spent way too much time on this already.

Comment 9

[Richard Marti (:Paenglab)]

Attachment: chat-win10.png

Above without and below with patch

Comment 10

[Jorg K (CEST = GMT+2)]

OK, and on Win7?

Comment 11

[Richard Marti (:Paenglab)]

Attachment: chat-win7-classic.png

Win7 Aero and Basic are not possible to change the window color that TB uses white icons. With Classic I got this one. Again without patch is above, with patch below.

Comment 12

[Jorg K (CEST = GMT+2)]

Attachment: Classic.png

I'm not sure what's happening, but Classic has a gradient and the patch makes no difference. To get a bar without gradient, I need to use a high contrast theme and there again is no difference as you can see in attachment 8920909 [details].

I can rs the patch, but I'd prefer to see some action. Do I have to build in Win10 for that?

Comment 13

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2) from comment #12)
> ... and the patch makes no difference.
That's not quite true. With classic and high contrast the colour difference is #110 or #111 or something equally unnoticeable.

Comment 14

[Richard Marti (:Paenglab)]

Attachment: color-settings-win7-classic.png

In Classic mode choose color settings, then click on the titlebat of the sample in the dialog. Now you see 1. color and 2. color where you can set both titlebar colors.

Comment 15

[Jorg K (CEST = GMT+2)]

Attachment: Classic2.png

I managed to get rid of the gradient. The colours identical with and without the patch.

Comment 16

[Richard Marti (:Paenglab)]

Okay, checked deeper. On Win 7 Classic I had no build with the patch and I only changed the rules in inspector and fell in a trap. Win 7 Classic has already a inherit rule.

But now I have the solution: In your Basic theme, change the titlebar text color to red for example and test again.

Comment 17

[Jorg K (CEST = GMT+2)]

In the Basic theme you can't change those colours:
https://social.technet.microsoft.com/Forums/en-US/4e89ac4c-6852-4622-be91-ea8e4937314a/basic-theme-cant-change-title-bar-color?forum=w7itproui

So I need to build on Windows 10, right? Classic and the high contrast themes derived from it don't show any difference.

Comment 18

[Jorg K (CEST = GMT+2)]

Comment on attachment 8920872 [details] [diff] [review]
toolbarbuttonInheritColor.patch

I've spent far too much time with this. I won't spend another hour building on Win10. I hope the screen shots for Win10 are real and not some trickery in the inspector. On Windows 7 there is absolutely now way to see any difference.

rs=jorgk.

Comment 19

[Richard Marti (:Paenglab)]

Attachment: win7-Basic-red.png

Then, I'm doing something wrong. It's possible.

Comment 20

[Richard Marti (:Paenglab)]

Win10 is real. Thanks for the endurance testing it.

Comment 21

[Jorg K (CEST = GMT+2)]

(In reply to Richard Marti (:Paenglab) from comment #19)
> But now I have the solution: In your Basic theme, change the titlebar ***text***(!!!)
> color to red for example and test again.
OK, I can see the effect now when changing the text colour. I tried to change the titlebar colour itself.

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/97ec98c4dd53
Let the .toolbarbutton-1 always inherit the color from toolbar. r=jorgk