439 bytes, patch
|Details | Diff | Splinter Review|
10.47 KB, image/png
1.50 KB, patch
|Details | Diff | Splinter Review|
7.13 KB, image/png
1.56 KB, patch
|Details | Diff | Splinter Review|
1.54 KB, patch
|Details | Diff | Splinter Review|
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.
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_?"
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.
Sigh, It looks horrible on ubuntu (orange on white)
I just had some user in a web forum wondering why the folders are gray (for him).
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 =(
I suggest to mark this bug as a duplicate of bug 533695.
Created attachment 432819 [details] [diff] [review] use "blue" instead of Highlight Here is a patch using the color blue instead of Highlight.
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.
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!)
Sounds sane, I'll try to put together a patch with regular text colors and splats.
Created attachment 433050 [details] [diff] [review] patch using splat instead of color Here is a version using the splat.
Much more clear IMHO, thx !
Comment on attachment 433050 [details] [diff] [review] patch using splat instead of color splat it is
whups, sorry, no checkin-needed unless it got a code review
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?
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.
(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 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
We'd hold a few hours to get this fix, but not a day. Reassigning to Andreas, since he's driving the patching here.
(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 ), so it needs to be a separate image. 1. http://www.css3.info/firefox-3-6-adds-background-clipping/
Created attachment 445883 [details] [diff] [review] attempt to use -moz-image-region on a background also added some color to the text to match bug 562934
> 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?
(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.
> 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 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 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.
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.
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.
Landed on trunk as well: http://hg.mozilla.org/comm-central/rev/c6fe0eb353a7
@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...
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)!