239914 - [Linux] When high contrast theme used, automatically ignore author's colors

Status: RESOLVED DUPLICATE of bug 1606036

(Core :: Widget: Gtk, defect, P3)

Description

[Aaron Leventhal]

When a high contrast theme is being used, Mozilla should automatically use those
colors for the web page.

This is the equivalent to "Use my chosen colors, ignoring the colors and
background image specified by the web page". We want to do this automatically
for the high contrast accessibility theme. For one thing, this will give us IE
parity. In addition, when an accessibility theme is being used, it's confusing
when one app doesn't follow it.

On Windows, we can detect that the high contrast is being used by calling
SystemParametersInfo with SPI_GETHIGHCONTRAST.

Under GTK, can we get similar information?

Comment 1

[Christopher Blizzard (:blizzard)]

Is there any reason we can't do this with a userChrome.css type hack?  This is a
example of the classic use of the alternate style sheets.

Comment 2

[Aaron Leventhal]

This bug is about automatically following the current OS theme when that's an
accessibility theme. I don't see what that has to do with user style sheets. The
idea is that the novice disabled user has to do nothing -- it just works.

Comment 3

[Christopher Blizzard (:blizzard)]

I'm talking about a method that we use, not that the user would ever see this. 
Can we poke values into the user default style sheet?

Comment 4

[Bill Haneman]

Although I agree that for some high-contrast users, replacing the stylesheet
with system colors is very important, for others it may not be so.
Since I don't believe *every* high-contrast theme user will want to override doc
stylesheets, I think that this ideally needs to be made an option which is
exposed to the user.  This also gets us around the issue of determining "which
themes are accessibility themes", etc. etc.
I do however think that it may be reasonable to replace the "default" style
sheet (if we indeed have one) to match the system theme, if the document itself
does not explicitly provide one.  So my suggestion:

(1) for docs with no explicit stylesheet (if they use some moz-provided default
stylesheet), map the default fg/bg colors/styles to the system (gtk+) theme.
(2) allow the user to ALWAYS substitute system colors for document-provided
colors, as a user-specifyable setting.

Does that seem feasible?

Comment 5

[Ginn Chen]

Check 'Use System Colors' or choose the fg/bg color you want.
Choose 'Always use the colors and background specified by the web page'
Does it work like Bill's suggestion?

Comment 6

[timeless]

i'm using high contrast atm on my laptop. not because of accessibilty, but just
because I wanted high contrast (i was running w/o lights for a while so that I
could leave the door open at night and not invite bugs).

Comment 7

[Aaron Leventhal]

We already do what Bill says in #4, as Gina points out in comment 5 we have a
pref for this.

Now that I know there is no flag in GTK to say an "accessibility theme" is being
used, I know I can make this a Windows only bug.

-> Making this a Windows-only bug.

On Windows, there is a "high contrast" flag which indicates that the current
theme is used because of accessibility. When that flag is set, we want to always
"Use system colors" and 'Always use the colors and background specified by the
web page'. That's what IE does, which it makes sense for the average user.

Comment 8

[Richie]

Attachment: bug report page in mozilla

text boxes have white text and white background while dropdown lists show
correct background of black

Comment 9

[Richie]

Attachment: bug report page in firefox

text boxes have white text and white background while dropdown lists show
correct background of black. also extremely noticable is the address bar has
white text and white background (system is white text black bg).

Comment 10

[Richie]

Attachment: bug report page in internet explorer based browser

correct colors shown on page.

Comment 11

[Richie]

the 3 attachments i posted are a case where this issue really impairs 
usability: this bug report page. with a high contrast color scheme of black 
window backgrounds and white text the text boxes in mozilla and firefox become 
unreadable. also the firefox address and search bars change text color along 
with the system preference but not the black backgound. bookmark manager in 
firefox also has white text as well as the firefox options menu. the color 
scheme is not labeled as high contrast, the way i changed my colors just 
happens to be very similar to a high contrast theme would display, however the 
effect is the same. using a windows api function to check if a high contrast 
theme is in use would not help those who are using modified color themes. and 
any css based fixes wouldnt help the firefox issues. the best way to combat it 
is to change the way system colors are handled. the way ie handles its use 
system colors option doesnt effect things like dropdown menus and text boxes, 
and modifying even just that part will solve quite a few issues. however for 
those who are using a high contrast theme, the use system colors option to 
change the actual text on a page with help from an api call would be a very 
good option.
sorry if i rambled on a bit there.

Comment 12

[Ginn Chen]

I can't reproduce the screenshot you posted with either mozilla or firefox,
Windows or Linux. Try the latest build and use classic theme for browser.
Textbox have white text on black background on my screen.

Comment 13

[Richie]

with classic theme the address bar text becomes white like the system 
specifies, however the background is also white, so it is like the firefox 
screenshot i posted. also effects mozilla bookmark manager and preferences 
dialog using classic theme. firefox is 0.8, mozilla is 1.7 RC2 and 1.8a1, both 
have same colors.

i just tested it with quite a few themes on 3 separate computers, 2 with 
stylexp and one using the standard windows theme management, and this effect is 
prevalent on all themes system or otherwise except the windows classic theme.

Comment 14

[Aaron Leventhal]

Rich, this is a different bug. This bug relates to how Mozilla should behave
when the "high contrast theme" flag lets us know that a theme is being used for
accessibility reasons.

What you're reporting is occuring when that flag is not set, and needs to be
filed in a new bug.

Comment 15

[Richie]

roger that, posting as new bug 245485 with lots more info on root of problem. 
hopefully the resolutions to these bugs will complement eachother.

Comment 16

[Aaron Leventhal]

Attachment: When launched with OS accessibility turned on, automatically use those colors in every page and choose the classic skin which adheres to the system colors

1) Add metric to nsILookAndFeel to detect if accessibility theme is being used
2) When gettting color info is nsPresContext, override prefs if accessibility
theme is used
3) Use classic skin when launched while OS accessibility theme being used
4) Alter nsXULChromeRegistry so it can handle setting a temporary theme, which
should work but doesn't without this patch

Comment 17

[Benjamin Smedberg]

Comment on attachment 149939 [details] [diff] [review]
When launched with OS accessibility turned on, automatically use those colors in every page and choose the classic skin which adheres to the system colors

Can I get this with more context; -pu10d for example?

Comment 18

[Aaron Leventhal]

Attachment: More contextt

Comment 19

[Benjamin Smedberg]

Comment on attachment 149943 [details] [diff] [review]
More contextt

>Index: xpfe/bootstrap/nsAppRunner.cpp

>+// Install accessibility theme if necessary
>+static void CheckUseAccessibleSkin();

Don't declare static functions unless you need to. (These lines are
unnecessary).

>+static void CheckUseAccessibleSkin()

>+    if (useAccessibilityTheme) {
>+      // Use classic skin, it obey's the system's accessibility theme

spelling nit: obeys, not obey's

>Index: rdf/chrome/src/nsChromeRegistry.cpp

> nsChromeRegistry::SetProviderForPackage(const nsACString& aProvider,

>+  if (aUseProfile && !mProfileInitialized) {
>+    rv = LoadProfileDataSource();
>+    if (NS_FAILED(rv)) return rv;

use NS_ENSURE_SUCCESS(rv, rv) here, so that we get debug-spew if this fails
(failure is not expected)

Same nits apply to toolkit/xre and chrome/src.

This will massively conflict with the semi-single-profile landing in
toolkit/xre; I'll race you to the finish line ;)

Comment 20

[Aaron Leventhal]

Comment on attachment 149943 [details] [diff] [review]
More contextt

Will post new patch based on bsmedberg's comments if requested to do so.

Comment 21

[Brian Ryner (not reading)]

Comment on attachment 149943 [details] [diff] [review]
More contextt

Looks ok.  I'm taking bsmedberg's word on the chrome registry changes; I'm not
exactly sure what's going on there.

Also, I did a bit of digging on how we might do this on GNOME.	There don't
appear to be any particular indicators in the widget styles to say the theme is
high contrast.	However, the default variations on high contrast themes do all
use the "hcengine" theme engine, so it may be possible (though not scalable)
for us to check that.

Comment 22

[Aaron Leventhal]

Bill Haneman: do you think Brian Ryner's suggestion of looking for the
"hcengine" theme engine or does a flag need to be added?

Fixed for Windows. Assigning to Bill Haneman for GTK fix.

Checking in widget/public/nsILookAndFeel.h;
/cvsroot/mozilla/widget/public/nsILookAndFeel.h,v  <--  nsILookAndFeel.h
new revision: 1.40; previous revision: 1.39
done
Checking in widget/src/windows/nsLookAndFeel.cpp;
/cvsroot/mozilla/widget/src/windows/nsLookAndFeel.cpp,v  <--  nsLookAndFeel.cpp
new revision: 1.45; previous revision: 1.44
done
Checking in widget/src/xpwidgets/nsXPLookAndFeel.cpp;
/cvsroot/mozilla/widget/src/xpwidgets/nsXPLookAndFeel.cpp,v  <-- 
nsXPLookAndFeel.cpp
new revision: 1.40; previous revision: 1.39
done
Checking in layout/base/src/nsPresContext.cpp;
/cvsroot/mozilla/layout/base/src/nsPresContext.cpp,v  <--  nsPresContext.cpp
new revision: 3.263; previous revision: 3.262
done
Checking in xpfe/bootstrap/nsAppRunner.cpp;
/cvsroot/mozilla/xpfe/bootstrap/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.417; previous revision: 1.416
done
Checking in toolkit/xre/nsAppRunner.cpp;
/cvsroot/mozilla/toolkit/xre/nsAppRunner.cpp,v  <--  nsAppRunner.cpp
new revision: 1.31; previous revision: 1.30
done
Checking in rdf/chrome/src/nsChromeRegistry.cpp;
/cvsroot/mozilla/rdf/chrome/src/nsChromeRegistry.cpp,v  <--  nsChromeRegistry.cpp
new revision: 1.303; previous revision: 1.302
done
Checking in chrome/src/nsChromeRegistry.cpp;
/cvsroot/mozilla/chrome/src/nsChromeRegistry.cpp,v  <--  nsChromeRegistry.cpp
new revision: 1.301; previous revision: 1.300
done

Comment 23

[Bill Haneman]

Aaron: I don't think looking for hcengine is good (for GNOME).  My position (see
comment #4) is that this needs to be a user-configurable flag, i.e. a checkbox
etc. that the user can use to override the document-specified colors.  If the
document does NOT specify an explicit stylesheet or colors, the colors should
ALWAYS be chosen based on the user's theme (whether an "accessibility" theme or
not).  If there are strong differences with my last assertion (always use
theme-derived stylesheet if document doesn't specify one), then instead of a
checkbox for "always use system theme colors for documents", we could have a
radiobutton group with three options:

"Display document content using system theme's color scheme:"
* always
o if document doesn't specify colors
o never

Comment 24

[[e-mail]]

(In reply to comment #16)
>3) Use classic skin when launched while OS accessibility theme being used
What if I have an alternative accessible theme ;-)

Comment 25

[Aaron Leventhal]

> (In reply to comment #16)
> >3) Use classic skin when launched while OS accessibility theme being used
> What if I have an alternative accessible theme ;-)

If someone develops an alternative accessibility theme for Mozilla and want to
use the them even though they're already using the OS accessibility theme,
they'll have to set:
ui.useAccessibilityTheme   to   false
So that it doesn't override the current theme with classic.

Comment 26

[Aaron Leventhal]

(In reply to comment #23)
> "Display document content using system theme's color scheme:"
> * always
> o if document doesn't specify colors
> o never

Bill, that's a nice UI, but I don't think most users will travel very far into
the prefs. When the OS can tell us that a particular theme is used for
accessibility it's helpful so that we can alter our behavior. That way seniors
or other VI users, who may not be very advanced, can launch Mozilla and it will
be visible to them like other apps -- they'll still be able to see and set the
prefs from there.

Comment 27

[Bill Haneman]

Aaron: on gnome/gtk+ there is no way to tell whether a theme is an
"accessibility" theme or not.  That's partly by design, as there's not a clear
delineation of where "requirements" end and "preferences" begin.

I think the UI is worth providing; true, the user must know about it, and must
take action to configure, but this is a one-time user action.  As long as it's
pointed out in any accessibility guide or release notes with a distribution, I
think it's acceptable.

Whether you want to "default" to the "always use system theme" behavior based on
the name of a gtk+ engine is up to you, but it looks wrong to me, at least in
GNOME/gtk+.  In either case, the user needs a means of overriding the default -
thus the UI suggestion.

Comment 28

[Aaron Leventhal]

(In reply to comment #27)
> Aaron: on gnome/gtk+ there is no way to tell whether a theme is an
> "accessibility" theme or not.  That's partly by design, as there's not a clear
> delineation of where "requirements" end and "preferences" begin.
Okay, just one last comment before I retire on this thread :)

It's really a misnomer on windows. What the "high contrast theme" checkbox is
supposed to mean is: "Important note for apps - don't override the OS theme with
fancy graphics or hard coded colors". It's a flag to tell apps that the current
theme being by used by the end-user must be strictly adhered to.

Comment 29

[OstGote!]

Why is this a Linux-only bug if it changes Windows behavior too? I guess this
patch causes Bug 246213 (for me on WinNT 4.0).

http://msdn.microsoft.com/library/default.asp?url=/library/en-us/sysinfo/base/systemparametersinfo.asp
indicates "SPI_GETHIGHCONTRAST ... Windows NT:  This value is not supported."

Comment 30

[Aaron Leventhal]

(In reply to comment #29)
> Why is this a Linux-only bug if it changes Windows behavior too? I guess this
> patch causes Bug 246213 (for me on WinNT 4.0).
We checked it in for Windows but kept it open for a future Linux fix.

>
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/sysinfo/base/systemparametersinfo.asp
> indicates "SPI_GETHIGHCONTRAST ... Windows NT:  This value is not supported."
If getting the SPI_GETHIGHCONTRAST info fails then we just do what we normally
do, we don't override the author's colors or the theme. 

Comment 31

[Joe Sabash [:JoeS1]]

Attachment: My Win98 settings

Well, the regression timeframe fits, I'm not using accessability in Win98
and I have not been able to use a trunk build since this change has hit the
trunk. I have confirmations that Win95,WinSE,WinME and NT are all affected.
Perhaps your test for accessability theme is not valid for these OS's

Comment 32

[Eitan Isaacson [:eeejay]]

This could be remedied by looking at the GTK+ theme name, if it is called "HighContrast", "HighContrastInverse" or "LowContrast" it is an appropriately named accessibility related theme. GNOME Shell and GNOME's control panel have these names hard coded, so we should not be concerned about hard coding it too.

Comment 33

[alexander :surkov (:asurkov)]

Eitan, are you still assignee of the bug?

Comment 34

[Eitan Isaacson [:eeejay]]

(In reply to alexander :surkov from comment #33)
> Eitan, are you still assignee of the bug?

Nah, lets let this one go..

Comment 35

[:Gijs (out for now; he/him)]

This isn't a Firefox::Theme issue; it needs the GTK nsLookAndFeel and friends to implement similar things as the Windows equivalents that were implemented in the attached patch, probably using the strategy in comment #32.

Comment 38

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1527381

Comment 39

[Emilio Cobos Álvarez (:emilio)]

I fixed this in bug 1606036.