Closed Bug 1046563 Opened 6 years ago Closed 6 years ago

tab bar UI broken on windows classic theme, TB31


(Thunderbird :: Theme, defect)

31 Branch
Windows 7
Not set


(thunderbird32 fixed, thunderbird33 fixed, thunderbird34 fixed, thunderbird_esr3132+ fixed)

Thunderbird 34.0
Tracking Status
thunderbird32 --- fixed
thunderbird33 --- fixed
thunderbird34 --- fixed
thunderbird_esr31 32+ fixed


(Reporter: andrixnet, Assigned: Paenglab)




(11 files, 1 obsolete file)

Attached image 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. 
- 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.
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.
Ever confirmed: true
Attached image 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.
Also the windows 7 high contrast default themes, with dark title bar, make TB a visual disaster.
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.
Attachment #8466555 - Attachment description: C:\Users\matt\Documents\Thunderbird 31 Tabs - Doesn't Have Focus.png → TB31 tabs, Windows 7 Classic Theme, app doesn't have focus
Attached patch 1046563.patch (obsolete) — Splinter Review
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.
Assignee: nobody → richard.marti
Attachment #8466673 - Flags: review?(philipp)
Attachment #8466673 - Flags: review?(josiah)
Comment on attachment 8466673 [details] [diff] [review]

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. :)
Attachment #8466673 - Flags: review?(josiah) → review+
Attached image HC-active.png
Win8 HC theme active window. Good to see the white icons.
Attached image HC-inactive.png
Win8 HC theme inactive window. The toolbar icons are still inverted but the tabbar one normal.
Very nice, thanks.
Attached image tb31-ui3.png
TB-31 on Win7 high contrast "white" theme, active window.
Comment on attachment 8466673 [details] [diff] [review]

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.
Attachment #8466673 - Flags: review?(philipp) → review+
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.
Keywords: checkin-needed
Keywords: checkin-needed
Attached patch 1046563.patchSplinter Review
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.
Attachment #8466673 - Attachment is obsolete: true
Attachment #8474212 - Flags: review?(josiah)
Comment on attachment 8474212 [details] [diff] [review]

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

Pretty slick. I'm a fan.
Attachment #8474212 - Flags: review?(josiah) → review+
Keywords: checkin-needed
Comment on attachment 8474212 [details] [diff] [review]

[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.
Attachment #8474212 - Flags: approval-comm-esr31?
Attachment #8474212 - Flags: approval-comm-beta?
Attachment #8474212 - Flags: approval-comm-aurora?
Attachment #8474212 - Flags: approval-comm-aurora? → approval-comm-aurora+
Closed: 6 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 34.0
Attachment #8474212 - Flags: approval-comm-esr31?
Attachment #8474212 - Flags: approval-comm-esr31+
Attachment #8474212 - Flags: approval-comm-beta?
Attachment #8474212 - Flags: approval-comm-beta+
Attached patch ESR PatchSplinter Review
This patch is one I've adapted for esr.
Attachment #8480507 - Flags: review?(richard.marti)
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.
Attachment #8480507 - Flags: review?(richard.marti) → review+
Attachment #8474212 - Flags: approval-comm-esr31+
Attachment #8480507 - Flags: approval-comm-esr31+
Blocks: 1059927
Attached patch ESR fix patchSplinter Review
I tested the build from and found under Win 8 a missing rule. This patch fixes this.
Attachment #8480807 - Flags: review?(standard8)
Comment on attachment 8480807 [details] [diff] [review]
ESR fix patch

[Triage Comment]
Attachment #8480807 - Flags: review?(standard8)
Attachment #8480807 - Flags: review+
Attachment #8480807 - Flags: approval-comm-esr31+
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.