523972 - Folder with new messages is yellow text on white - don't use "color: Highlight;"

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[Ben Bucksch (:BenB)]

Environment:
- SuSE 10.3
- KDE 3.x, with gtk-qt theme

Reproduction:
- Make your system theme so that selection (selected text in a textfield, or selected items in a listbox) is white text on turquoise background, or better yet black text on yellow background.
- Start Thunderbird, get new mail, look at folder pane

Actual result:
The folder with the new mail (not just unread, but really not seen yet) is bold and turquoise or yellow text on white background.
Hard to read, and really ugly.

Folders with unread, but not new mail are black text in bold. Until recently, folders with new mail also were like that.

Expected result:
Text color is readable and does not hurt in the eyes.
Normal black text (like all the other folder names), in bold, but with a different icon, e.g. the sparkl/splat as used in bug 516860 comment 26 for new messages in the thread pane.

Severity:
- Hurts eyes, hard to read
- No accessibility

Implementation:
Please don't use "color: Highlight" anywhere for anything. Highlight is a
background and border color. It may look good on your system/theme, but not on
mine: My Highlight color is a bright turquoise, which looks awful as text
color, and is hard to read.

FWIW, the correct (and common) usage of Highlight would be:
color: HighlightText;
background-color: Highlight;
but that's inappropriate in this case, because that's for selection.
That also shows the problem: If you use a very faint color as selection
background color (e.g. black on yellow), you get a very strange effect when
using that (yellow) as *foreground* color (with a white background).

Just use an icon.

Comment 1

[Phil Ringnalda (:philor)]

I was surprised to see how good, relatively speaking, the yellow on white on Linux looked last night when I was looking at the splat, since I'd only been seeing the use of Highlight in Windows, where I was wondering wtf we were thinking setting folders to the grey of disabled things to say that they were new. That was honestly my first thought the first time I saw it on Windows - "why is my Inbox _disabled_?"

Comment 2

[Bryan Clark (DevTools PM) [:clarkbw]]

We're not going to block the release on this.  However it would be good to get some work done on this.  Andreas, feel free to take this from me if you want.

Comment 3

[Pádraig Brady]

Sigh, It looks horrible on ubuntu (orange on white)

Comment 4

[Ben Bucksch (:BenB)]

I just had some user in a web forum wondering why the folders are gray (for him).

Comment 5

[Roby Van Hoye]

I'd like to chime in that I miss the star/splat/whatever it was on the folder-view that indicated something new had arrived. Most of the time I have the folder-view in "Unread Folders"-mode. 
Each time I have 'new mail message' I can go see where that message was added by the filters as I have hundreds of unread messages spread among tens of folders and hence need a 'way' to see where the new messages went to. 
In tb2 the folder icon would receive a star/splat top right, but this seems to have gone. After a while I accidentally found out that the folders containing new messages in fact where of a different shade than those that did not receive anything. This difference is waayyy to subtle. 
I'm willing to attribute that this is partly because I use the 'classic windows' view (not vista's Wintendo look, nor Aero) with some modifications to the default back-ground colour and all that on a 1900x1200 TFT laptop screen that might not represent all colours perfectly.
I liked the way it worked on TB2, please either "revert" to that, or make the colour-scheme configurable. As it is now it's not really usable =(

Comment 6

[Etienne URBAH]

I suggest to mark this bug as a duplicate of bug 533695.

Comment 7

[Andreas Nilsson (:andreasn)]

Attachment: use "blue" instead of Highlight

Here is a patch using the color blue instead of Highlight.

Comment 8

[Andreas Nilsson (:andreasn)]

Attachment: screenshot

Comment 9

[Ben Bucksch (:BenB)]

Andreas, you don't know which background color the user uses on his desktop color scheme. There's no guarantee that it's something bright like white, it may just as well be black (and text white).

> Expected result:
> Text color is readable and does not hurt in the eyes.
> Normal black text (like all the other folder names), in bold, but with a
> different icon, e.g. the sparkl/splat as used in bug 516860 comment 26 for new
> messages in the thread pane.

Comment 10

[Roby Van Hoye]

Like Ben says, please don't use a fixed colour. Although I guess that most people stick to the OS-defaults, some of us prefer to use alternative colour-schemes.
Frankly I still prefer the splat-thing as I seem to remember it was in the previous version of Thunderbird...

(additionally I hope to buy a Notion Ink Adam (or similar) sometime which will actually be monochrome in some conditions, having the splat will then be ENORMOUSLY more useful than colours!)

Comment 11

[Andreas Nilsson (:andreasn)]

Sounds sane, I'll try to put together a patch with regular text colors and splats.

Comment 12

[Andreas Nilsson (:andreasn)]

Attachment: patch using splat instead of color

Here is a version using the splat.

Comment 13

[Andreas Nilsson (:andreasn)]

Attachment: screenshot of the above patch

Comment 14

[Roby Van Hoye]

Much more clear IMHO, thx !

Comment 15

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 433050 [details] [diff] [review]
patch using splat instead of color

splat it is

Comment 16

[Andreas Nilsson (:andreasn)]

whups, sorry, no checkin-needed unless it got a code review

Comment 17

[Phil Ringnalda (:philor)]

Comment on attachment 433050 [details] [diff] [review]
patch using splat instead of color

I'm pretty sure you don't want to wait while I sort out why my Linux VM has decided that building is something it only used to do, but: do you really mean "left" all those times, or do you actually mean "start" so it's not mangled in RTL?

Comment 18

[Magnus Melin [:mkmelin]]

This splat looks like a star, not a sun like next to messages (so we'd have bug 516860 again). It also covers the green arrow on the inbox in a not so nice way.

Comment 19

[Andreas Nilsson (:andreasn)]

(In reply to comment #18)
> This splat looks like a star, not a sun like next to messages (so we'd have bug
> 516860 again). It also covers the green arrow on the inbox in a not so nice
> way.

It's the same graphics we use in the message list, bug 516860 is about stars.

Comment 20

[Magnus Melin [:mkmelin]]

Comment on attachment 433050 [details] [diff] [review]
patch using splat instead of color

(In reply to comment #19)
> It's the same graphics we use in the message list.

Actually it isn't, that was my objection.

Minusing until that's sorted out

Comment 21

[Dan Mosedale (:dmosedale, :dmose)]

We'd hold a few hours to get this fix, but not a day.  Reassigning to Andreas, since he's driving the patching here.

Comment 22

[Andreas Nilsson (:andreasn)]

(In reply to comment #20)
> (From update of attachment 433050 [details] [diff] [review])
> (In reply to comment #19)
> > It's the same graphics we use in the message list.
> 
> Actually it isn't, that was my objection.

As I'm calling this piece of graphics as a background, I can't use -moz-image-region (yet [1]), so it needs to be a separate image.

1. http://www.css3.info/firefox-3-6-adds-background-clipping/

Comment 23

[Andreas Nilsson (:andreasn)]

Attachment: attempt to use -moz-image-region on a background

also added some color to the text to match bug 562934

Comment 24

[Ben Bucksch (:BenB)]

> also added some color to the text to match bug 562934

I don't know about the qute theme, but in general, it's a bad idea to mix system colors (which gnomestripe uses) with absolute colors. You don't know what the system color is going to be: Background may be black, white, yellow, dark green, you name it. (E.g. Ubuntu has 2 themes: one with dark background and one with bright background.) The foreground needs to have appropriate contrast, and with an absolute color, you're pretty much guaranteed to stomp on some themes. In fact, that's pretty much what this bug is about. Or am I missing something in this particular case?

Comment 25

[Andreas Nilsson (:andreasn)]

Attachment: the above patch without hardcoded colors

Comment 26

[Andreas Nilsson (:andreasn)]

(In reply to comment #24)
> > also added some color to the text to match bug 562934
> 
> I don't know about the qute theme, but in general, it's a bad idea to mix
> system colors (which gnomestripe uses) with absolute colors.

My guess is that cases where the color #0066cc is used as a background color is quite rare. The bug is about the fact that the systems Highlight color for some themes where producing odd results and that it wasn't a highlight. I would love for the GTK+ theming systems to give me more safe colors to play with, but that's a GTK+ bug.
Added another patch however.

Comment 27

[Ben Bucksch (:BenB)]

> that's a GTK+ bug.

No, it's inherent: A theme may use any color as background or foreground - that's the idea of themes. It's unlikely that they use exactly #0066cc, yes, but it's very likely that some of them use a background color which has little contrast to that, making the text unreadable.

> Added another patch however.

Thank you.

Comment 28

[Andreas Nilsson (:andreasn)]

Comment on attachment 433050 [details] [diff] [review]
patch using splat instead of color

Asking for a re-review based on the data in comment22. Will spin off a separate bug about using the moz-rect approach on the background image once the code base supports that so that we can use the same graphics for both.

Comment 29

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 433050 [details] [diff] [review]
patch using splat instead of color

Stealing this review (sorry, Magnus!) so that we can get this in the tree by tonight's midnight freeze.

Comment 30

[Dan Mosedale (:dmosedale, :dmose)]

Given comment 22 (note that the link name itself is deceiving: apparently it _actually_ is only in 3.7 aka 1.9.3), I think this patch is Good Enough, so I'm giving it r+. Marking in-testsuite-, as I think the cost/benefit of writing an automated test for this patch is very low.

Comment 31

[Dan Mosedale (:dmosedale, :dmose)]

Pushed to the branch: <http://hg.mozilla.org/releases/comm-1.9.2/rev/9556406c5f63>.  The trunk tinderboxen are a bit of mess, so holding off on pushing to them for now.

Comment 32

[Mark Banner (:standard8)]

Landed on trunk as well:

http://hg.mozilla.org/comm-central/rev/c6fe0eb353a7

Comment 33

[Roby Van Hoye]

@Ben Bucksch : on the contrary, your comment (#24) is exactly what (was/is) going wrong here... I found myself having trouble reading the "highlight coloured text" on a custom background (WinXP). I've switched to Win7 in the meanwhile and although I use a personalized 'classic' theme again, it now seems to be more readable... yet, only because I took the time to make Highlight more standing out towards the background colour, instead of 'just a couple of shades' away as before... 

I understand the developers 'running out of colours', but in that case why not implement a colour-options form and use whatever default you want but let the user tweak it as he sees fit. Might be a bit of a challenge across the different platforms, yet what's life without its little challenges ? =)

Anyway, looking forward to the next update...

Comment 34

[Ben Bucksch (:BenB)]

Roby, I think you misunderstood me. The problem you ran into is exactly what I wanted to prevent (I filed this bug).

Thanks, Andreas, for fixing this (and dmose for review/approval)!