1046563 - tab bar UI broken on windows classic theme, TB31

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Andrei Boros]

Attachment: tb31-ui.png

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 (Beta/Release)
Build ID: 20140716183446

Steps to reproduce:

I use windows classic theme for my windows desktop. 
I upgraded TB24.x to TB31.


Actual results:

Seems like TB31 has adopted the same "australis" like theme as FF29+. 
Due to my OS theme now there are elements in the tab bar that are barely visible. 
Examples: 
- tab delimiters for inactive tabs
- tab close button (single button at far right)
- calendar extension icons (which blended well into the classic UI theme, until that region has been designated as window title)

Attached is a screen shot. 

These are the same problems that appeared when FF29 was launched, with the australis theme. 
This was also reported for Firefox and "Classic theme restorer" add-on was suggested as a possible fix. However I found no such addon for TB. 
A request for fixing FF "australis" theme for windows classic theme was submitted, but I don't know it's bug ID. 


Expected results:

tab delimiters should be clearly visible, always, not only on mouse over. 
close button should be clearly visible, always. 
Do this in a consistent way so that addons may use icons with transparency in the tab bar, instead of high-contrast icons.

Comment 1

[:aceman]

You have some especially dark window title background. Is that standard for Win 7? In Windows XP, the pure windows classic theme has the color fading from dark blue to less dark blue to the right. And the elements you mention are better visible.

You can try to install some nonstandard theme for TB, maybe it will fit your color scheme better.

Comment 2

[Andrei Boros]

Attachment: tb31-ui2.png

Ok, my theme is a little tweaked to be more like the win95 look. Too many fades are tiresome to the eye.

Anyway, even with the pure w7 classic theme, still quite bad. 
The buttons gain a little visibility, tab separators are still not visible.

Actually, the current default theme, like FF australis, is very bad for any OS theme that features a dark titlebar.

Comment 3

[Andrei Boros]

Also the windows 7 high contrast default themes, with dark title bar, make TB a visual disaster.

Comment 4

[Matthew Kidd]

I'm seeing similar issues with TB31 on Windows 7 with the Windows Classic theme.

Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.0

I'm attaching three screenshots. The first two illustrate what the active tab looks like when the app has does and doesn't have focus. Notice the rendering problem on the rounded edges of the Australis style tabs.

The third screenshot shows the e-mail composition window with focus. The To: field has a white background but the Subject: field does not appear white under the pointer is moved to the text box for the Subject: field.

Must we suffer through UI space wasting Australis style tabs?! I rue the day the Mozilla development team got Chrome envy.

Comment 5

[Matthew Kidd]

Attachment: TB31 tabs, Windows 7 Classic Theme, app has focus

Comment 6

[Matthew Kidd]

Attachment: TB31 tabs, Windows 7 Classic Theme, app doesn't have focus

Comment 7

[Matthew Kidd]

Attachment: TB31, Windows 7 Classic Theme, Compose Window

Comment 8

[Richard Marti (:Paenglab)]

Attachment: 1046563.patch

Philipp, I added r? to you for the Calendar part.

I ported bug 1012629 from FX to TB. This makes with the change from :-moz-lwtheme-brighttext to [brighttext] the rules a lot easier. Also the Win8 [darkwindowframe] rules are no more needed except one. Now it behaves like FX.

I tested it on XP Win7 and Win8 in normal, classic and also HC modes.

Comment 9

[Josiah Bruner [:jsbruner]]

Comment on attachment 8466673 [details] [diff] [review]
1046563.patch

Review of attachment 8466673 [details] [diff] [review]:
-----------------------------------------------------------------

Looks pretty good to me, and you seemed to have tested it pretty well. Though some screenshots of the change would be nice. :)

Comment 10

[Richard Marti (:Paenglab)]

Attachment: HC-active.png

Win8 HC theme active window. Good to see the white icons.

Comment 11

[Richard Marti (:Paenglab)]

Attachment: HC-inactive.png

Win8 HC theme inactive window. The toolbar icons are still inverted but the tabbar one normal.

Comment 12

[Josiah Bruner [:jsbruner]]

Very nice, thanks.

Comment 13

[Andrei Boros]

Attachment: tb31-ui3.png

TB-31 on Win7 high contrast "white" theme, active window.

Comment 14

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8466673 [details] [diff] [review]
1046563.patch

Review of attachment 8466673 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/base/content/msgMail3PaneWindow.js
@@ +2038,5 @@
>    else
>      window.maximize();
>  }
> +
> +let ToolbarIconColor = {

Maybe we can get this into toolkit somehow? I can imagine a similar problem might occur for other dialogs that use their own toolbar, e.g. the event dialog.

Comment 15

[Richard Marti (:Paenglab)]

Yeah, that would be great if this would work on all toolbars in all windows/dialogs. Now it's tied to the main window. The AB and the Composer (especially for bug 1042780) don't profit of this.

Comment 16

[Richard Marti (:Paenglab)]

Attachment: 1046563.patch

I found the standalone message window didn't used the new rules. I initiate now also in messageWindow.js the color check.

To don't copy the check code in all files I moved it to toolbarIconColor.js and made the selectors in it more global to be able to use this also for the AB and Composer.

Comment 17

[Josiah Bruner [:jsbruner]]

Comment on attachment 8474212 [details] [diff] [review]
1046563.patch

Review of attachment 8474212 [details] [diff] [review]:
-----------------------------------------------------------------

Pretty slick. I'm a fan.

Comment 18

[Richard Marti (:Paenglab)]

Comment on attachment 8474212 [details] [diff] [review]
1046563.patch

[Approval Request Comment]
User impact if declined: UI not in sync with FX
Testing completed (on c-c, etc.): in c-c, JS code already in FX 31
Risk to taking this patch (and alternatives if risky): low, because the JS code is already in FX in use.

Comment 19

[Mark Banner (:standard8)]

https://hg.mozilla.org/comm-central/rev/81dc14b19c1b
https://hg.mozilla.org/releases/comm-aurora/rev/00106aeb7a62

Comment 20

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-beta/rev/9ddc53f41075

Comment 21

[Mark Banner (:standard8)]

Attachment: ESR Patch

This patch is one I've adapted for esr.

Comment 22

[Richard Marti (:Paenglab)]

Comment on attachment 8480507 [details] [diff] [review]
ESR Patch

Review of attachment 8480507 [details] [diff] [review]:
-----------------------------------------------------------------

I've also finished my patch and diffed them and found no difference :) So I think it should be correct.

Comment 23

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-esr31/rev/df4a3f69abf5

Comment 24

[Richard Marti (:Paenglab)]

Attachment: ESR fix patch

I tested the build from http://ftp.mozilla.org/pub/mozilla.org/thunderbird/tinderbox-builds/comm-esr31-win32/1409226347/ and found under Win 8 a missing rule. This patch fixes this.

Comment 25

[Mark Banner (:standard8)]

Comment on attachment 8480807 [details] [diff] [review]
ESR fix patch

[Triage Comment]
r+a=Standard8

Comment 26

[Mark Banner (:standard8)]

https://hg.mozilla.org/releases/comm-esr31/rev/38d712c6094b