768202 - Datepicker widget does not obey "Optimize colors for accessibility" setting, text not readable

Status: RESOLVED FIXED

(Calendar :: OS Integration, defect)

Description

[Stefan Sitter [:ssitter]]

Attachment: screenshot of problem

Lightning 1.8a1 + Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Thunderbird/16.0a1

Steps to reproduce:
1. "Optimize colors for accessibility" is enabled in the Lightning preferences
2. high contrast color theme is selected for Windows
3. check UI for accessibility issues

Actual result:
The datepicker widget in e.g. Edit Event dialog or Edit Recurrence dialog doesn't obey the "Optimize colors for accessibility" setting. Some text is not readable. It works as fine for the datepicker widget that is displayed in the left sidebar and the today pane. See attached screenshot.

Comment 1

[Richard Marti (:Paenglab)]

The "Optimize colors for accessibility" works only in main window. We could extend to append the attribute to calendar-event-dialog and calendar-event-dialog-recurrence. But is this possible at startup when this windows aren't opened?

I could also only use system colors for this widgets.

What do you think?

Comment 2

[Philipp Kewisch [:Fallen][☁️📆]]

I don't think it would be much work to create a helper that can be called in each kind of window we open that injects the right attributes.

Comment 3

[Richard Marti (:Paenglab)]

You know I'm so good in JS ;), please could you provide me such a helper? Then I have to add on CSS dialog[systemcolors].

Comment 4

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Loader for systemcolors attribute

Oh yes, I know. Hope springs eternal ;-) Here is a patch that takes care of the JS part. All you need to do is insert the function call in the windows you need this in. I've done so in example for the event dialog.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: patch

Thank you Philipp, your code works like a charm.

I'm asking Mathew for review because most of the code is from Philipp.

Comment 6

[Stefan Sitter [:ssitter]]

Should or can this be done on the datepicker widget itself instead of the dialog that contains the datepicker widget? Otherwise you will have to repeat it for all other dialogs that contain the datepicker widget, e.g. the custom reminder dialog, the invite attendees dialog, the print dialog, ....

Comment 7

[Philipp Kewisch [:Fallen][☁️📆]]

I guess I could modify the helper to work for not only windows but also dom nodes and then call it in the datepicker contstructor, but I think if we continue this route, we will end up checking for this pref on each and every control we create. Not sure if thats something we want to do since this means one observer per instance of each control we use.

Opinions?

Comment 8

[Richard Marti (:Paenglab)]

I tried to set the observer on minimonth.xul and now on every dialog with the mnimonth the systemcolors are set.

I think we can do this this way and on windows/dialogs without minimonth we can put the observer when needed.

Updated patch is coming.

Comment 9

[Richard Marti (:Paenglab)]

Attachment: patch v2

New patch with observer call in minimonth.xml

Comment 10

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Fix - v3

Sorry if this was unclear in comment 7, but what will happen with your patch as is, is that an observer will be added for each minimonth control. So for example in the custom recurrence dialog, when the systemcolors pref is changed, then it will be updated 3 times on the window.

Another downside to this approach is that the logic to update a window attribute is dependent on a control placed arbitrarily in the window. Lets say we have a dialog with one minimonth control, but lots of places where the systemcolors attribute is used in CSS. A developer might assume the systemcolors attribute is just given, remove the minimonth for some reason, and we have a regression if the systemcolors mode is not checked.

Here is a different approach, that sets the systemcolors attribute on all opened windows, I've also imported the CSS bits.

Comment 11

[Matthew Mecca [:mmecca]]

Comment on attachment 638106 [details] [diff] [review]
Fix - v3

Looks good. r=mmecca

Comment 12

[Richard Marti (:Paenglab)]

Pushed to comm-central: <http://hg.mozilla.org/comm-central/rev/09afb9b552a9>