Closed Bug 469664 Opened 11 years ago Closed 11 years ago

Selected tasks (with start/due date) are unreadable on windows with black desktop

Categories

(Calendar :: Tasks, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: andreas.treumann, Assigned: sipaq)

Details

Attachments

(13 files, 7 obsolete files)

25.26 KB, image/jpeg
Details
149.69 KB, image/png
Details
2.02 KB, text/calendar
Details
54.97 KB, image/png
Details
46.49 KB, image/png
Details
55.15 KB, image/png
Details
38.92 KB, image/png
Details
39.27 KB, image/png
Details
19.49 KB, image/png
Details
38.84 KB, image/png
Details
6.28 KB, patch
berend.cornelius09
: review+
sipaq
: ui-review+
Details | Diff | Splinter Review
144.39 KB, image/jpeg
Details
8.52 KB, patch
Details | Diff | Splinter Review
STEPS TO REPRODUCE:
===================

- use a black desktop color 
- create a future task with start date and due date
- select this task in the task list

RESULT:
=======

- the selected task is completely black (background color and characters) and unreadable

EXPECTED RESULT:
================

- the selected task should be readable


REPRODUCIBLE:
=============

- always
-> me

The problem is, that we are using system background-colors here without corresponding system text colors. We just assume that the text color is always black unless defined otherwise. 

So if you combine a black text color with a black background color (as defined by the system), things get messy :)

Thanks for noticing, Andreas.
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
OS: Windows XP → All
Hardware: PC → All
Attached patch patch v. #1 (obsolete) — Splinter Review
I also found out that this is a regression to Bug 393748 -  Active tasks are dimmed (grey) in task list while completed tasks are shown in black
and prepared a patch for it. Looking at that patch I have certain doubts if similar flaws as described here in conjunctions with duetoday¨¨where the textcolor is not fitting to the background. Without having investigated any deeper I would prefer to return to the constant values that we were using before bug 393748
Berend,
constant values suck in terms of accessibility and will break you even more often. Just think about what might happen with your patch, if the text color is blue in combination with your constant blue background color?

Like I said, the right thing to do is to find a suitable system foreground color that works well with the assigned system background color on all system color schemas.

Let me hook up a patch tonight and run this by you, ok?
Attached patch My Patch - v1 (obsolete) — Splinter Review
Here's my take on this bug.

My goals here were:
- Always try to find a good corresponding system background-color to a system 
  foreground color and vice versa
- Choose an accompanying hardcoded background-color or foreground color in case 
  there is already a hardcoded color defined, which has no system color equivalent
Attachment #353114 - Flags: ui-review?(christian.jansen)
Attachment #353114 - Flags: review?(Berend.Cornelius)
Comment on attachment 353114 [details] [diff] [review]
My Patch - v1

Looks good for me. r=christian
Attachment #353114 - Flags: ui-review?(christian.jansen) → ui-review+
My concerns are that the constants ¨Menu¨ ¨Background¨...  refer to the current desktop background, menu-color and so on and are likely to change with the surrounding conditions like a theme change. I think that the user does not want this in this context. Once he learned that a blue background-color is associated with a ¨duetoday- task¨ a future task with another color and so on he does not want to learn new associations again after the theme changed or the desktop color...
Comment on attachment 353114 [details] [diff] [review]
My Patch - v1

Apart from my last comment I am afraid I have to deny the review. When I tested it under Windows I got the strange result that you see in the screenshot that I will attach. But besides that the patch worked fine for all kinds of tasks.
Attachment #353114 - Flags: review?(Berend.Cornelius) → review-
(In reply to comment #6)
> My concerns are that the constants ¨Menu¨ ¨Background¨...  refer to the 
> current desktop background, menu-color and so on and are likely to change 
> with the surrounding conditions like a theme change.

Exactly.

> I think that the user does not want this in this context. Once he learned 
> that a blue background-color is associated with a ¨duetoday- task¨ a future 
> task with another color and so on he does not want to learn new associations 
> again after the theme changed or the desktop color...

I think this is exactly what the user wants. If he wanted all colors to stay 
the same, why would he change his color scheme in the first place?

(In reply to comment #7)
> Apart from my last comment I am afraid I have to deny the review. When I 
> tested it under Windows I got the strange result that you see in the 
> screenshot that I will attach. But besides that the patch worked fine for 
> all kinds of tasks.

It seems that the hardcoded color combination does not work. This does not 
only not work for the selected overdue task in your example, but also for a 
selected task in progress. I will attach an updated patch soon.
Attached patch My Patch - v2 (obsolete) — Splinter Review
New patch with issues fixed that Berend raised in his review. Carrying forward UI-review.
Attachment #353550 - Flags: ui-review+
Attachment #353550 - Flags: review?(Berend.Cornelius)
Attachment #353114 - Attachment is obsolete: true
Andreas told me on Thursday last week that he still discovered a problem with the new patch under a certain Linux environment, but I have no closer information about this. I guess I have to wait with the review under these circumstances.
(In reply to comment #11)
> Andreas told me on Thursday last week that he still discovered a problem with
> the new patch under a certain Linux environment, but I have no closer
> information about this. I guess I have to wait with the review under these
> circumstances.

Ok, CC'ing Andreas here for his input.
Attached image unselected task, linux
Attached image selected task with/without Simos patch (obsolete) —
The problem on linux is not caused by Simons patch.

Unselected tasks with a start date today, e.g. 2 hours in the future and one hour duration are displayed in light Grey color (characters) on a white background (see screen shot). This bug occurs also with the official nightly build.

The visibility of the same task but selected is much better with Simons patch ( see second screen shot)
Comment on attachment 353550 [details] [diff] [review]
My Patch - v2

I am really sorry but when I tested this under Linux I saw several flaws that just can't be ignored as can bee seen in the screenshot that I will attach. Basically the problem is that the background-color is not set over the whole column but only underneath the script. With "duetoday" the script is even the  same as the background-color so that it is completely invisible.
I personally don't find that the menu-color (grey) is really suitable for the background on Linux. But if we should decide to use it we should take it for all rows and not only for some I think.
Attachment #353550 - Flags: review?(Berend.Cornelius) → review-
Attached image screenshot
Berend, if you have a problem with the grey script color, then this is a new bug that should be fixed separately. As Andreas said, this was also a problem before my patch came along.
Attached file test-calendar (obsolete) —
in reply to comment #18:
I could not detect the mentioned flaws of comment #16 without the patch, so I guess that we somehow have to deal with this in this issue. Attached I send an ics file in which the problems can be reproduced. Please notice that the dates of the tasks might be adapted after some time.
Andreas could not provide an ics file so I set up one on my own with five different tasks. Based on these tasks I could observe the following flaws:
-In all tasks  except "overdue" only the character's background color is set. The rest of the field remains white (see also my latest screeenshot).
-due-today tasks have grey characters on grey background and are therefor invisible.
- When selected "overdue" and "due-today" have mixed background-color, too.
Attached patch Patch v3 (obsolete) — Splinter Review
I finally managed to work through this. I also finally realized the fine distinction between -moz-tree-row and -moz-tree-cell-text and adjusted the patch accordingly.

I'll attach a new test calendar (also containing an open-ended task) and three screenshots showing the new patch behavior.
Attachment #353030 - Attachment is obsolete: true
Attachment #353210 - Attachment is obsolete: true
Attachment #353550 - Attachment is obsolete: true
Attachment #354138 - Attachment is obsolete: true
Attachment #356074 - Flags: ui-review+
Attachment #356074 - Flags: review?(Berend.Cornelius)
Attached file Tasks Test Calendar v2
Attachment #355594 - Attachment is obsolete: true
Attachment #356075 - Attachment description: Test Calendar v2 → Tasks Test Calendar v2
This is the only case when the bug still exists, when due today is actually due today and it is selected and there's no focus on lightning.

Almost there, it seems. :)
Comment on attachment 356074 [details] [diff] [review]
Patch v3

Canceling review for now. I'll work on the small Mac issue that Gary found.

I would still appreciate it if someone could test this patch on Linux. So that I don't have to jump through more hoops to finally get this in.
Attachment #356074 - Flags: review?(Berend.Cornelius)
Attached patch Patch v4Splinter Review
So, hopefully this is the last installment of this patch.

The only difference to the v3 patch is here:

 .calendar-task-tree > treechildren::-moz-tree-cell-text(duetoday) {
-    color: ActiveCaption;
+    color: WindowText;
+    font-weight: bold;
 }

As can be seen from attachment 356185 [details], OS X uses a light grey color for ActiveCaption instead of the blue color that WinXP uses, which makes the entry unreadable when the entry is selected but without focus, as the light grey of the unfocused selection is very similar to the light grey of ActiveCaption.

So instead I switched to a simple black color. But as normal tasks and future tasks already have that text-color, I choose to make tasks that are due today more easily identifiable by making the script bold.

I'm not sure whether that decision is still covered by Christian ui-review, that he originally gave for the v1 patch. Berend, I leave it to your discretion whether I should ask for ui-review again or not.

Again, if someone with a Linux box, who uses the standard desktop theme on a GNOME or KDE desktop could test this patch and post some screenshots here, that would be very much appreciated.
Attachment #356074 - Attachment is obsolete: true
Attachment #356313 - Flags: ui-review+
Attachment #356313 - Flags: review?(Berend.Cornelius)
I checked this patch (v4) with GNOME and KDE3.5/4, default desktop theme (see screen shot), looks okay. Minor issue: completed tasks, with selection but no focus -> contrast maybe a little bit to low with KDE.
Comment on attachment 356313 [details] [diff] [review]
Patch v4

Patch looks good and works fine from my point of view. I noticed that you only provide rules for the combination of "selected" and "focus", but nonfocused selected rows always have the same  background color. Maybe it is an option to set a certain opacity for these cases, but this should of course not block this issue and should be decided by Christian.
Attachment #356313 - Flags: review?(Berend.Cornelius) → review+
Comment on attachment 356313 [details] [diff] [review]
Patch v4

I forgot to mention you could consolidate the rules where you assign the color HighlightText.
Maybe while you are at it you could also remove all the redundant 
    list-style-image: url(chrome://calendar/skin/unifinder-images.png);
rules, too.
(In reply to comment #32)
> I noticed that you only provide rules for the combination of "selected" and 
> "focus", but nonfocused selected rows always have the same  background color. 
> Maybe it is an option to set a certain opacity for these cases

(In reply to comment #33)
> I forgot to mention you could consolidate the rules where you assign the 
> color HighlightText.

(In reply to comment #34)
> Maybe while you are at it you could also remove all the redundant 
>     list-style-image: url(chrome://calendar/skin/unifinder-images.png);
> rules, too.

Berend, I tried all three of those suggestions, but to no avail. 

- Setting an opacity of 0.5 only for selected (but not focussed) tasks did 
  not work (no change visible)
- I did not manage to Consolidate the the rules where I assign the color 
  HighlightText and remove all 
    list-style-image: url(chrome://calendar/skin/unifinder-images.png)
  It had unintended side-effects, probably caused by my lack of deep CSS 
  knowledge. I'll file a followup-bug for this. Maybe you can take it as 
  you probably know much better what you're doing?
This is what I checked in.

3 changes happened compared to the v4 patch that was reviewed:
- I added myself to the credits
- I removed the rule text-decoration: line-through for the class 
  .calendar-task-tree > treechildren::-moz-tree-row(completed, selected, focus)
  as this rule was already covered by the class
  .calendar-task-tree > treechildren::-moz-tree-cell-text(completed)
- Since the GrayText-color doesn't work equally on all platforms and is hardly 
  visible when selected but not focussed even on the Mac or Windows platform, 
  I decided to give it a black text-color as well, but show it in an italic 
  font-style.
Changeset ce533e4625e8 pushed to comm-central
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.0
Checked in lightning and sunbird build 20090113 -> verified.
Status: RESOLVED → VERIFIED
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
You need to log in before you can comment on or make changes to this bug.