tab bar UI broken on windows classic theme, TB31

RESOLVED FIXED in Thunderbird 34.0

Status

defect
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: andrixnet, Assigned: Paenglab)

Tracking

31 Branch
Thunderbird 34.0
x86_64
Windows 7
Bug Flags:
in-testsuite -

Thunderbird Tracking Flags

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

Details

Attachments

(11 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Posted 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. 
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

5 years ago
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Comment 2

5 years ago
Posted 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.
(Reporter)

Comment 3

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

Comment 4

5 years ago
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.

Updated

5 years ago
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
(Assignee)

Comment 8

5 years ago
Posted 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
Status: NEW → ASSIGNED
Attachment #8466673 - Flags: review?(philipp)
Attachment #8466673 - Flags: review?(josiah)
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. :)
Attachment #8466673 - Flags: review?(josiah) → review+
(Assignee)

Comment 10

5 years ago
Posted image HC-active.png
Win8 HC theme active window. Good to see the white icons.
(Assignee)

Comment 11

5 years ago
Posted image HC-inactive.png
Win8 HC theme inactive window. The toolbar icons are still inverted but the tabbar one normal.
Very nice, thanks.
(Reporter)

Comment 13

5 years ago
Posted image tb31-ui3.png
TB-31 on Win7 high contrast "white" theme, active window.
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.
Attachment #8466673 - Flags: review?(philipp) → review+
(Assignee)

Comment 15

5 years ago
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
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Comment 16

5 years ago
Posted 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]
1046563.patch

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

Pretty slick. I'm a fan.
Attachment #8474212 - Flags: review?(josiah) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Comment 18

5 years ago
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.
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+
https://hg.mozilla.org/comm-central/rev/81dc14b19c1b
https://hg.mozilla.org/releases/comm-aurora/rev/00106aeb7a62
Status: ASSIGNED → RESOLVED
Last Resolved: 5 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+
Posted patch ESR PatchSplinter Review
This patch is one I've adapted for esr.
Attachment #8480507 - Flags: review?(richard.marti)
(Assignee)

Comment 22

5 years ago
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+
(Assignee)

Updated

5 years ago
Blocks: 1059927
(Assignee)

Comment 24

5 years ago
Posted patch ESR fix patchSplinter Review
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.
Attachment #8480807 - Flags: review?(standard8)
Comment on attachment 8480807 [details] [diff] [review]
ESR fix patch

[Triage Comment]
r+a=Standard8
Attachment #8480807 - Flags: review?(standard8)
Attachment #8480807 - Flags: review+
Attachment #8480807 - Flags: approval-comm-esr31+
(Assignee)

Updated

4 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.