Win 8 high contrast themes are also considered to match -moz-windows-default-theme

VERIFIED FIXED in Firefox 29

Status

()

Core
Widget: Win32
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: hgk, Assigned: Gijs)

Tracking

unspecified
mozilla31
All
Windows 8
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox29+ verified, firefox30+ verified, firefox31 verified)

Details

(Whiteboard: [Australis:P3-])

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
Created attachment 8342912 [details]
thunderbird2411negativepicture.jpg

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release)
Build ID: 20131112160018

Steps to reproduce:

use windows settings black contrast
start email client
start writting an email


Actual results:

window for writting opens but parts of the window are white on white


Expected results:

window should open with all parts are black in the background and white text color.

this is espacially a problem for people who are visually handicapped. would be nice when you can change that. I an older version that problem didn't exist.
On Windows 7 I'm seeing the background black and correct contrasting text color. Are you sure this screenshot is taken from Windows 7? And are you sure you have no theme installed which changes the background.

Please check to run TB in safe mode (Hilfe/Mit deaktivierten Add-ons neu starten...).
(Reporter)

Comment 2

4 years ago
Hello!
I'm sorry that was my fault. The operating system is windows 8 (not 8.1).
I changed the operation system in the bug above. On a windows 7 machine I had a team view window to a window 8 machine open. That's why I wrote the wrong system. Sorry.
Would be nice when you can look for it again.
Thanks!
OS: Windows 7 → Windows 8
(Reporter)

Comment 3

4 years ago
In version 17.0.3 everything was fine with the contrast.
Ah yes, I see this on Win 8 too. Also Firefox shows the light blue background with the high contrast themes.

The rules in @media (-moz-windows-default-theme) are also used with HC themes -> toolkit issue.
Component: Mail Window Front End → Themes
Product: Thunderbird → Toolkit
Hardware: x86_64 → All
Summary: black on white contrast windows settings are not used everywhere → Win 8 high contrast themes are also as -moz-windows-default-theme processed
Version: 24 → unspecified
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

4 years ago
Component: Themes → Widget: Win32
Product: Toolkit → Core
Jim, do you know whats wrong?
Flags: needinfo?(jmathies)
Created attachment 8377696 [details]
Firefox on Win8 and white high contrast

The interesting is, I tried attachment 8377497 [details] on Win8 with white high contrast, and "Default Theme" isn't red. I also changed the testcase to use @media (-moz-windows-default-theme) instead of -moz-system-metric(windows-default-theme) and it was still not red.

But the toolbars where still light blue (with some toolbar-buttons on them white).

Inspector shows http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser-aero.css#25 ff. is still used.

Comment 7

3 years ago
(In reply to Richard Marti (:Paenglab) from comment #5)
> Jim, do you know whats wrong?

not really no, but from your test doesn't seem like this bug should be filed in widget.
Flags: needinfo?(jmathies)
And where should it be filed?

Comment 9

3 years ago
Dao, any thoughts on this? It looks like the media queries are working, but per comment 6 css that shouldn't be in use is.

I just checked on win7, while not perfect the blue background in the chrome area does switch to a fade black.
Flags: needinfo?(dao)
On Win7 it works. This issue is only on Win8.1. I have no Win8 at hand now but I remember it worked then.
(In reply to Jim Mathies [:jimm] from comment #9)
> Dao, any thoughts on this? It looks like the media queries are working, but
> per comment 6 css that shouldn't be in use is.

The CSS that shouldn't be in use is behind -moz-windows-default-theme, so the media query does not seem to be working as expected.
Flags: needinfo?(dao)

Comment 12

3 years ago
That gets updated here - 

http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsUXThemeData.cpp#320

which implies these high contrast themes are still being identified as a default themes.
Blocks: 986324
(Assignee)

Comment 13

3 years ago
(In reply to Jim Mathies [:jimm] (away 4/1-4/21) from comment #12)
> That gets updated here - 
> 
> http://mxr.mozilla.org/mozilla-central/source/widget/windows/nsUXThemeData.
> cpp#320
> 
> which implies these high contrast themes are still being identified as a
> default themes.

I just checked in VS's debugger, it returns AeroLite.msstyles as the theme path (above that section). This seems to match MS documentation which suggests that high contrast styles are now implemented on top of "normal" Windows theming styles, whereas before they were using classic. styles and the path lookup (GetCurrentThemeName) would fail.
(Assignee)

Comment 14

3 years ago
Created attachment 8398072 [details] [diff] [review]
high contrast themes shouldn't be considered the default theme in CSS,

This seems to fix it. Jim, does this look right to you?
Attachment #8398072 - Flags: review?(jmathies)
(Assignee)

Updated

3 years ago
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED

Comment 15

3 years ago
Comment on attachment 8398072 [details] [diff] [review]
high contrast themes shouldn't be considered the default theme in CSS,

Review of attachment 8398072 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/windows/nsUXThemeData.cpp
@@ +332,5 @@
>    if (theme == WINTHEME_UNRECOGNIZED)
>      return;
>  
> +  if ((!IsWin8OrLater() || !sIsHighContrastOn) &&
> +	  (theme == WINTHEME_AERO || theme == WINTHEME_AERO_LITE || theme == WINTHEME_LUNA)) {

Yes, this does. Please add a comment above this explaining why we do these checks.

Also, nit, on the second line you've introduced a tab.
Attachment #8398072 - Flags: review?(jmathies) → review+
General question, should we start tracking this for Australis?

Gijs, awesome that you're taking this! :)
(Assignee)

Comment 17

3 years ago
(In reply to Mike de Boer [:mikedeboer] from comment #16)
> General question, should we start tracking this for Australis?

I'm not sure. This doesn't magically fix all our high contrast bugs - not even close. It's probably helpful to have the behaviour match between different Windows versions, though.

AFAICS this fix is relatively contained, so I wouldn't feel bad asking for uplift to Aurora and Beta.

To ensure that this stays on uplift radars, I've just set it as P5.
Whiteboard: [Australis:P5]
(Assignee)

Comment 18

3 years ago
Created attachment 8398188 [details] [diff] [review]
high contrast themes shouldn't be considered the default theme in CSS,

Trees are closed and I need sleep. This is the patch w/ comments addressed.
(Assignee)

Updated

3 years ago
Attachment #8398188 - Flags: review+
Attachment #8398188 - Flags: checkin?
(Assignee)

Updated

3 years ago
Attachment #8398072 - Attachment is obsolete: true
Comment hidden (obsolete)
Oops, forgot to qpush. Here is a valid try push:
https://tbpl.mozilla.org/?tree=Try&rev=5af93d8b9c19
Comment on attachment 8398188 [details] [diff] [review]
high contrast themes shouldn't be considered the default theme in CSS,

https://hg.mozilla.org/integration/fx-team/rev/d8a36b893b2b
Attachment #8398188 - Flags: checkin? → checkin+
status-firefox29: --- → affected
status-firefox30: --- → affected
status-firefox31: --- → affected
(Assignee)

Comment 22

3 years ago
(In reply to Matthew N. [:MattN] from comment #21)
> Comment on attachment 8398188 [details] [diff] [review]
> high contrast themes shouldn't be considered the default theme in CSS,
> 
> https://hg.mozilla.org/integration/fx-team/rev/d8a36b893b2b

Thanks!
Summary: Win 8 high contrast themes are also as -moz-windows-default-theme processed → Win 8 high contrast themes are also considered to match -moz-windows-default-theme
Whiteboard: [Australis:P5] → [Australis:P5][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/d8a36b893b2b
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P5][fixed-in-fx-team] → [Australis:P5]
Target Milestone: --- → mozilla31
status-firefox31: affected → fixed
tracking-firefox29: --- → ?
tracking-firefox30: --- → ?
Matthew, could you fill the uplift request? thanks
tracking-firefox29: ? → +
tracking-firefox30: ? → +
Flags: needinfo?(MattN+bmo)
Comment on attachment 8398188 [details] [diff] [review]
high contrast themes shouldn't be considered the default theme in CSS,

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Windows 8 High Contrast Implementation
User impact if declined: Users in High Contrast (HC) mode on Windows 8+, won't get the proper styles for that mode (Other CSS will apply which shouldn't and may greatly reduce the contrast thus defeating the point of HC mode). It will help fix a few other HC bugs on Win8 due to the Win8 change.
Testing completed (on m-c, etc.): m-c although HC mode isn't tested in automation AFAIK.
Risk to taking this patch (and alternatives if risky): Tiny refactoring with a different code path for Win8+. It seems pretty straightforward so likely low risk.
String or IDL/UUID changes made by this patch: None
Attachment #8398188 - Flags: approval-mozilla-beta?
Attachment #8398188 - Flags: approval-mozilla-aurora?
Flags: needinfo?(MattN+bmo)
P3- since it blocks a P3-
Whiteboard: [Australis:P5] → [Australis:P3-]
Attachment #8398188 - Flags: approval-mozilla-beta?
Attachment #8398188 - Flags: approval-mozilla-beta+
Attachment #8398188 - Flags: approval-mozilla-aurora?
Attachment #8398188 - Flags: approval-mozilla-aurora+
Aurora: https://hg.mozilla.org/releases/mozilla-aurora/rev/e89c8841b036
Beta: https://hg.mozilla.org/releases/mozilla-beta/rev/250d63775815
status-firefox29: affected → fixed
status-firefox30: affected → fixed

Updated

3 years ago
Blocks: 963950
Keywords: verifyme
Some elements from the High Contrast Black theme, such as:
- the location bar's placeholder text,
- the google search bar's placeholder text,
- the help sub-menu,
are still not displayed properly, screenshots here: http://bit.ly/1lN3seb 

I'm not sure if the issues above should have been fixed by this as well, please confirm.

Verified on Windows 8 Pro 64-bit [1], using High Contrast Black and High Contrast White themes with:
- the latest Beta (Build ID: 20140407135746)
- the latest Aurora 2014-04-07 (Build ID: 20140407004002),
- the latest Nightly 2014-04-07 (Build ID: 20140407030203).

1. Mozilla/5.0 (Windows NT 6.2; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0
Flags: needinfo?(gijskruitbosch+bugs)
Keywords: verifyme
(Assignee)

Comment 29

3 years ago
(In reply to Andrei Vaida, QA [:avaida] from comment #28)
> Some elements from the High Contrast Black theme, such as:
> - the location bar's placeholder text,
> - the google search bar's placeholder text,
> - the help sub-menu,
> are still not displayed properly, screenshots here: http://bit.ly/1lN3seb 
> 
> I'm not sure if the issues above should have been fixed by this as well,
> please confirm.
> 
> Verified on Windows 8 Pro 64-bit [1], using High Contrast Black and High
> Contrast White themes with:
> - the latest Beta (Build ID: 20140407135746)
> - the latest Aurora 2014-04-07 (Build ID: 20140407004002),
> - the latest Nightly 2014-04-07 (Build ID: 20140407030203).
> 
> 1. Mozilla/5.0 (Windows NT 6.2; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0

I don't know anything about the graphical consequences of this change. It was a technical change; the easiest way to test is to evaluate the following in the browser console on Windows 8:

window.matchMedia("(-moz-windows-default-theme)").matches


It should be false for the Windows 8 high contrast themes, and true for the default "flat" Windows 8 theme.
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(andrei.vaida)
(In reply to :Gijs Kruitbosch from comment #29)
> I don't know anything about the graphical consequences of this change. It
> was a technical change; the easiest way to test is to evaluate the following
> in the browser console on Windows 8:
> 
> window.matchMedia("(-moz-windows-default-theme)").matches
> 
> 
> It should be false for the Windows 8 high contrast themes, and true for the
> default "flat" Windows 8 theme.

I was able to confirm the fix for this issue on the environment mentioned in Comment 28 and in accordance with Gijs' instruction (Thanks Gijs!).
Status: RESOLVED → VERIFIED
status-firefox29: fixed → verified
status-firefox30: fixed → verified
status-firefox31: fixed → verified
Flags: needinfo?(andrei.vaida)
Duplicate of this bug: 993370
(In reply to Andrei Vaida, QA [:avaida] from comment #28)
> Some elements from the High Contrast Black theme, such as:
> - the location bar's placeholder text,
> - the google search bar's placeholder text,

This is bug 986324.

> - the help sub-menu,
> are still not displayed properly, screenshots here: http://bit.ly/1lN3seb 
> 
> I'm not sure if the issues above should have been fixed by this as well,
> please confirm.

Updated

3 years ago
Duplicate of this bug: 1002863

Updated

3 years ago
Depends on: 1042090

Updated

3 years ago
No longer depends on: 1042090

Updated

3 years ago
Depends on: 1042625
You need to log in before you can comment on or make changes to this bug.