Closed Bug 426007 Opened 15 years ago Closed 15 years ago

Remove alternating row colors in the download manager

Categories

(Toolkit :: Downloads API, defect)

defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla1.9

People

(Reporter: faaborg, Assigned: dao)

References

Details

(Keywords: access)

Attachments

(3 files, 1 obsolete file)

I believe we should remove alternating row colors in the download manger due to significant visual incompatibility with the various default windows themes.  The intention was for the alternating row color to be very light, similar to OS X, however none of the available system colors on windows are light enough to achieve this effect, and the end result is an interface that does not achieve the intended appearance.  Other problems include:

-Incompatibility with high contrast themes causing a section 508 violation (white text on white background)
-Tan color appearing in with the Luna Silver OS theme

We can solve the accessibility problem with bug 425598, but that still doesn't address the problem of making the interface look correct given a wide range of OS themes.

As an additional concern, even when done correctly this design goes against our overall goal of visual integration with the platform, since alternating rows are not found anywhere in the windows UI.
Flags: blocking-firefox3?
In particular note the high level of contrast in Vista and classic themes, the tan color in silver mode, and the white text on a white background in high contrast mode.
Keywords: access, sec508
Summary: Remove alternating row colors in the download manger → Remove alternating row colors in the download manager
Agreed. What would you suggest we replace it with on XP? Nothing at all? On Vista we should see if we can get it to act like a menuItem on hover.
Flags: blocking-firefox3? → blocking-firefox3+
Whiteboard: [needs assignee][needs UX decision]
Do we want to get rid of the styling or also the logic that stripeifies?
I think we should remove the logic. I suppose it will be obsolete anyway once we have support for :nth-child, which I think won't take too long after 1.9.
Keywords: uiwanted
Wait, so this will be windows xp/vista only? Keep it on os x and linux?
(In reply to comment #4)
> I think we should remove the logic.

Of course only if we remove the styling on all platforms...
btw, the accessibility problem exists on Linux, too. ThreeDLightShadow was never meant to be a background color.
(In reply to comment #0)
> -Incompatibility with high contrast themes causing a section 508 violation
> (white text on white background)
> -Tan color appearing in with the Luna Silver OS theme
> 
> We can solve the accessibility problem with bug 425598, but that still doesn't
> address the problem of making the interface look correct given a wide range of
> OS themes.

simple solution:

 richlistitem[type="download"][alternate="true"]:not([selected="true"]) {
-  background-color: ThreeDLightShadow;
+  background-color: rgba(0,0,0,0.05);
 }
Attached patch patch: use rgba (obsolete) — Splinter Review
just in case
Attachment #312929 - Flags: ui-review?(faaborg)
(In reply to comment #8)
> simple solution:
> 
>  richlistitem[type="download"][alternate="true"]:not([selected="true"]) {
> -  background-color: ThreeDLightShadow;
> +  background-color: rgba(0,0,0,0.05);
>  }

This uses non-native color (black) and won't have any effect on dark themes (white text, black background).

If only rgba accepts system colors such as rgba(-moz-dialog, 0.5)...
Summary: Remove alternating row colors in the download manager → Remove alternating row colors in the download manager (or make it work as intended)
(In reply to comment #10)
> won't have any effect on dark themes (white text, black background).

Yes.
It makes the most sense to just disable it on all platforms, rather than try and fail to make it work for accessible themes...
It'd look really bad if we didn't do banded backgrounds on the mac, imho.
For a black theme, rgba(0,0,0,0.05) isn't any different from disabling it altogether. I wouldn't call that failure.
For other themes, the question is: Do we still think the original rationale for using alternating row colors is valid?
I think we should keep the alternating row colors on OS X, since that is totally platform native.  The only way to solve the accessibility problem on windows (white on white) is with bug 425598, however that still doesn't address the problem of ThreeDShadow looking tan in the Luna silver theme.  I recommend we remove the row colors on both XP and Vista.  Linux themers should decide based on what they think is the most native.
(In reply to comment #15)
> The only way to solve the accessibility problem on
> windows (white on white) is with bug 425598

No. See the attached patch.
Comment on attachment 312929 [details] [diff] [review]
patch: use rgba

Ok, just realized how this solves the accessibility problem, sorry, slow day.

The thing is hard coding 50% black isn't going to look great given all of the different shades of gray that windows uses.  For instance, Windows Classic vs. Windows Standard.
Attachment #312929 - Flags: ui-review?(faaborg) → ui-review-
Comment on attachment 312929 [details] [diff] [review]
patch: use rgba

this is 5%, not 50%
Attachment #312929 - Flags: ui-review- → ui-review?(faaborg)
Comment on attachment 312929 [details] [diff] [review]
patch: use rgba

reassigning to beltzner for a call on if we want to introduce additional hard coded colors, and do something that is intentionally different from the normal representation of list views on windows.
Attachment #312929 - Flags: ui-review?(faaborg) → ui-review?(beltzner)
(In reply to comment #19)
> if we want to introduce additional hard coded colors

Umm, we shouldn't stigmatize this. After all we also hard code colors in icons. A touch of black is as neutral as could be and doesn't cause any of the known problems.
>Umm, we shouldn't stigmatize this. After all we also hard code colors in icons.
>A touch of black is as neutral as could be and doesn't cause any of the known
>problems.

I'm not trying to be argumentative, but I think large swatches of color (even something as mundane as 5% black) end up not working given the range of windows themes.

Of the 5 most commonly used XP themes, three of them use warm shades of grey/tan, and 3 of then use cold shades.  5% black looks fine with the cold themes, but doesn't look right with the warm ones.
Fair enough, but that still leaves us with the question if not alternating rows at all is better.
UX Team have a decision on this yet?
Comment on attachment 312929 [details] [diff] [review]
patch: use rgba

I said a while back that I agreed with the call to remove the row colouring on Windows, and I'm pretty sure that I meant it.

I merely asked Alex if he thought we should use anything to separate the entries, he said he didn't think it was neccessary, and I'm fine with that.
Attachment #312929 - Flags: ui-review?(beltzner) → ui-review-
Keywords: uiwanted
Whiteboard: [needs assignee][needs UX decision] → [needs assignee]
(In reply to comment #13)
> It'd look really bad if we didn't do banded backgrounds on the mac, imho.

Agreed, so I don't think we should disable the logic that allows them to do so.

(In reply to comment #22)
> Fair enough, but that still leaves us with the question if not alternating rows
> at all is better.

I don't think the value is significant enough to bother expending effort, when there's a simple and expedient solution: remove the alternating colours. :)
Ok, so, Mac makes sense to preserve this, and other themes can do what they want if the logic is still there.  There's probably a way to do this in a way that looks good on multiple OS themes that doesn't break accessibility, but I don't think we have the luxury of making time to do it right.

Dao, please just strip the banding CSS on Windows and Linux, and we can look at finding a better solution after we ship.  Thanks!
Assignee: nobody → dao
Whiteboard: [needs assignee] → [needs patch]
(In reply to comment #24)
> I said a while back that I agreed with the call to remove the row colouring on
> Windows, and I'm pretty sure that I meant it.

What you did is agree with comment 0, but most of comment 0 would have become invalid if we just stopped using ThreeDLightShadow, which is not a valid background color. That's why I thought pointing to a solution that basically works (attachment 313000 [details]) would be worthwhile. I'm sorry if that was wrong.

(In reply to comment #26)
> Dao, please just strip the banding CSS on Windows and Linux, and we can look at
> finding a better solution after we ship.  Thanks!

Sure.
Status: NEW → ASSIGNED
For future reference, bug 412092 will fix this issue for Linux. (See attachment 298385 [details] for an example). Due to the lengthy amount of changes, it won't land until after Firefox 3 is shipped
(In reply to comment #27)
> What you did is agree with comment 0, but most of comment 0 would have become
> invalid if we just stopped using ThreeDLightShadow, which is not a valid
> background color. That's why I thought pointing to a solution that basically
> works (attachment 313000 [details]) would be worthwhile. I'm sorry if that was > wrong.

No, no, that was right of you, sorry for getting snippy. :(
Attached patch patchSplinter Review
Attachment #312929 - Attachment is obsolete: true
Attachment #313384 - Flags: review?(mconnor)
Summary: Remove alternating row colors in the download manager (or make it work as intended) → Remove alternating row colors in the download manager
Whiteboard: [needs patch] → [has patch][needs review mconnor]
Comment on attachment 313384 [details] [diff] [review]
patch

r=sdwilsh, ui-r=mconnor per comment 26.

This is good to go.
Attachment #313384 - Flags: review?(mconnor) → review+
Whiteboard: [has patch][needs review mconnor] → [has patch][has reviews][can land]
Keywords: checkin-needed
Checking in toolkit/themes/gnomestripe/mozapps/downloads/downloads.css;
/cvsroot/mozilla/toolkit/themes/gnomestripe/mozapps/downloads/downloads.css,v  <--  downloads.css
new revision: 1.8; previous revision: 1.7
done
Checking in toolkit/themes/winstripe/mozapps/downloads/downloads.css;
/cvsroot/mozilla/toolkit/themes/winstripe/mozapps/downloads/downloads.css,v  <--  downloads.css
new revision: 1.30; previous revision: 1.29
done
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Keywords: checkin-needed
OS: Windows XP → All
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews][can land]
Target Milestone: --- → Firefox 3
Duplicate of this bug: 417434
Thanks for leaving the capacity there (at least as of this morning's build). One of my themes is so much at odds with OS themes that compatibility is not an issue. With that theme (Halloween), alternating colors are actually helpful.
Note: filed follow up bug 427179 to help with visual grouping
Hardware: PC → All
Verified FIXED using:

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008040704 Minefield/3.0pre

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008040706 Minefield/3.0pre

-and-

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008040706 Minefield/3.0pre
Status: RESOLVED → VERIFIED
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.