549306 - addressbook background color doesn't change in windows' accessibility mode

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Wayne Mery (:wsmwk)]

addressbook background color doesn't change in windows' accessibility mode
tested with v3.1b1pre

1. open address book
2. activate accessibility mode - alt+shift+PrtScn

expected results: background color changes
actual results: no change

the result, is white characters on a light blue background (or some other light color depending on your theme), as described in http://forums.mozillazine.org/viewtopic.php?f=28&t=1765155

Comment 1

[Wayne Mery (:wsmwk)]

key usage area in thunderbird and this hurts accessibility, so suggest this is wanted for v3.1, but not necessarily blocking

Comment 2

[Mark Banner (:standard8)]

Interesting, I'm fairly sure we already fixed something like this (if not its a dupe of an existing bug)

Can we get a version number and screenshot for this?

Comment 3

[Wayne Mery (:wsmwk)]

Attachment: AB screen shot

right, that was bug 351736 reported on XP where only preview pane was affected. The rest of the window looked good per attachment 237207 [details]. 

I'm testing with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.2.2pre) Gecko/20100224 Lanikai/3.1b1pre.  The mozillazine reporter is probably 3.0.x

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Marking as wanted but not blocking.

Comment 5

[Bob]

For a person with RP (eye disease) this bug makes the address book completely unusable :-(. Such a person cannot use a white screen background. Bug is present in Windows 7, Thunderbird 3.0.3. (Set windows screen background to black. Set Thunderbird options->display to yellow text on black background. Set options -> composition to yellow/black. Address book is displayed yellow text on white background.)

Comment 6

[rsx11m]

This problem is still present after bug 545557 checked in this morning, which made substantial changes to Vista and Windows 7 appearance. The address book
has constant background and splitter colors disregarding the desktop theme.

Most notably, many colors are hardwired still in this patch, which should be avoided to start with for exactly that reason... :-\

Comment 7

[rsx11m]

Attachment: AB high-contrast #1 after bug 545557

Comment 8

[rsx11m]

Attachment: AB Windows Classic after bug 545557

In both cases, the background color is 238, 243, 250.

Comment 9

[rsx11m]

(In reply to comment #2)
> Interesting, I'm fairly sure we already fixed something like this (if not its a
> dupe of an existing bug)

That was bug 351736 which fixed qute for the Windows XP and earlier, but not for aero. The main culprit introducing the hard-wired background color here should be bug 507595, so apologies to Richard if he just followed that pattern based on this example. Nevertheless, this should be fixed on aero as well, and not just for reasons of accessibility.

Comment 10

[Richard Marti (:Paenglab)]

Attachment: Proposed patch

The new colors are only set in -moz-windows-default-theme. In the other cases it's -moz-Field.

What do you think about this?

Comment 11

[rsx11m]

Great! I've applied your patch and it shows the intended results for all four high-contrast and the classic desktop themes coming with Windows 7, while on
the other hand retaining the colorization in the Windows 7 default theme.

-> WFM with that fix.

Comment 12

[tymerkaev]

removing border-right for non-default windows theme will kill 1px space after. Also, border-right won't work with rtl, use -moz-border-end instead. border has same color as background: use transparent value instead.

Comment 13

[Richard Marti (:Paenglab)]

Attachment: Updated patch

Updated patch with Azat's comments

Comment 14

[tymerkaev]

Comment on attachment 495236 [details] [diff] [review]
Updated patch

>-  border-left: none;
>-  border-right: 1px solid #EEF3FA;
>+  -moz-border-start: none;
>+  -moz-border-end: 1px solid transparent;
>   border-bottom: none;
> }

border: none;
-moz-border-end: 1px solid transparent;

> #abResultsTree {
>   -moz-appearance:  none;

Please remove whitespace before 'none'.

>-  -moz-border-right-colors: ThreeDHighlight ThreeDLightShadow;
>-  border-left: none;
>-  border-right: 1px solid;
>+  -moz-border-start: none;
>+  -moz-border-end: 1px solid ThreeDHighlight;
>   border-bottom: none;
> }

Use border: none and -moz-border-end-width: 1px; color and style of border already specified in toolkit.

>@media all and (-moz-windows-default-theme) {
>  #dirTree {
>    background-color: #EEF3FA;
>  }
>}

>@media all and (-moz-windows-default-theme) {
>  #abResultsTree {
>    background-color: #EEF3FA;
>  }
>}

>@media all and (-moz-windows-default-theme) {
>  #CardViewBox {
>    background-color: #EEF3FA;
>  }
> }

Please merge these rules.

Also, don't rewrite things like color: -moz-FieldText and background-color: -moz-Field; they're already specified in toolkit.

Comment 15

[rsx11m]

(In reply to comment #14)
> >@media all and (-moz-windows-default-theme) {
> Please merge these rules.

It is easier to read if the specific rule directly follows the more general
rule, so the current placement may be preferable even if it's more verbose
(I've done it similarly in my patch for bug 564280).

Comment 16

[Richard Marti (:Paenglab)]

(In reply to comment #14)
> Please merge these rules.

I leave it separate for readability and let Blake decide if he wants this merged.

> Also, don't rewrite things like color: -moz-FieldText and background-color:
> -moz-Field; they're already specified in toolkit.

That's right. I stripped all except the -moz-border-end.

The other errors are from original addressbook.css. Maybe a bug should be opened to correct them.

Comment 17

[Richard Marti (:Paenglab)]

Attachment: Final patch?

cleaner patch but still with separated rules for -moz-windows-default-theme

Comment 18

[Andreas Nilsson (:andreasn)]

Comment on attachment 495259 [details] [diff] [review]
Final patch?

Looks good when using both classic and High Contrast!

Comment 19

[Richard Marti (:Paenglab)]

Attachment: fixed indentation

I found two wrong indents.

Taking ui-r + from previous patch.

Comment 20

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 497898 [details] [diff] [review]
fixed indentation

I was kind of wondering about merging the rules, too, but I think it's easier to see what's going on when they're separated like this.

Azat said:
> Also, don't rewrite things like color: -moz-FieldText and
> background-color: -moz-Field; they're already specified in toolkit.
but your patch contains:
>+  background-color: -moz-Field;
for the #CardViewBox.

r=me if you remove that (and make sure it still works!), or give me a reason why you have to specify it there.

Thanks,
Blake.

Comment 21

[Richard Marti (:Paenglab)]

(In reply to comment #20)
> Comment on attachment 497898 [details] [diff] [review]
> fixed indentation
> 
> Azat said:
> > Also, don't rewrite things like color: -moz-FieldText and
> > background-color: -moz-Field; they're already specified in toolkit.
> but your patch contains:
> >+  background-color: -moz-Field;
> for the #CardViewBox.
> 
> r=me if you remove that (and make sure it still works!), or give me a reason
> why you have to specify it there.

The #CardViewBox isn't a tree which is defined in toolkit, it's a vbox and the background color is only with this rule defined. When it isn't defined here, then the background color of #abContent is visible and this is -moz-dialog instead of the -moz-Field used by the trees. On Win7-classic this are different colors.

Comment 22

[Blake Winton (:bwinton) (:☕️)]

Works for me.  I'll check it in when the tree is less orange.

Comment 23

[Blake Winton (:bwinton) (:☕️)]

Committed as http://hg.mozilla.org/comm-central/rev/7e2280fae2a7