Closed Bug 1723938 Opened 3 years ago Closed 3 years ago

Certificate error pages warning title is unreadable in HCM

Categories

(Core :: Layout, defect)

Firefox 92
All
Windows 10
defect
Not set
normal

Tracking

()

VERIFIED FIXED
92 Branch
Accessibility Severity s2
Tracking Status
firefox-esr78 --- unaffected
firefox-esr91 --- unaffected
firefox90 --- unaffected
firefox91 --- unaffected
firefox92 --- verified

People

(Reporter: phorea, Assigned: emilio)

References

(Blocks 2 open bugs, Regression)

Details

(Keywords: access, regression, Whiteboard: [fidefe-mr11-cleanup] [hcm-2021-h2])

Attachments

(2 files)

Attached image HCM Black VS HCM#2.png

[Affected versions]:

  • Nightly 92.0a1 2021-08-03

[Affected platforms]:

  • Win 10 64-bit

[Preconditions]:

  • Turn ON High Contrast Mode at system level - with High Contrast Black theme
  • Reproducible with all Firefox Themes

[Steps to reproduce]:

  1. Open a certificate error page in Firefox (eg. https://expired.badssl.com/)
  2. Check that all elements from page are visible in HCM

[Expected result]:

  • All elements are readable while in HCM

[Actual result]:

  • Warning: Potential Security Risk Ahead label is not readable, it is covered by a white box. The text has different colors in other HCM themes, please see the comparative screenshot.

[Regression range]:

  • Found commit message:
    Bug 1720012: Respect system colors checkbox independent of HCM

[Additional notes]:

Hey Morgan, do you know what's happening here? If we can fix on the CSS side we can obviously do that, but off-hand I'm not sure why this would have broken as a result of your changes. :-)

Flags: needinfo?(mreschenberg)
Whiteboard: [fidefe-mr11-cleanup]

It looks like setting background-color: Window on the h1 that is unreadable here works, but I do not understand why.

The default background-color value for the h1 is transparent, and not changed, and that should work here. Emilio or Morgan, do you know why it doesn't?

(Adding in Micah and Molly for visibility as they're about to start work on a mentored project involving HCM)

Flags: needinfo?(emilio)

So what's going on is that the warning icon has a background-image like this:

  background-image: url("chrome://global/skin/icons/warning.svg");

So when we end up finding a color to backplate, we end up here and choose the default background color, which since the previous bug no longer uses the system colors... But since the system color in use for the text is white, it ends up being unreadable.

So in this case we could either change the break above by a continue (to find the "upper" background-color) or go back to use the system colors in HCM... But probably the first solution is better.

Flags: needinfo?(emilio)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

So what's going on is that the warning icon has a background-image like this:

  background-image: url("chrome://global/skin/icons/warning.svg");

So when we end up finding a color to backplate, we end up here and choose the default background color, which since the previous bug no longer uses the system colors... But since the system color in use for the text is white, it ends up being unreadable.

So in this case we could either change the break above by a continue (to find the "upper" background-color) or go back to use the system colors in HCM... But probably the first solution is better.

OK, so that might work here, I'll give it a shot, though I'm unsure what else would break...

However, reading this code I have a related question: this loop is informed by the background image, but in this case (ie in about:certerror and friends) there is no visual overlap between the text we're backplating and the background image's size + position. Can/should we detect that case? I suspect it is not uncommon, and it sounds like we could skip based on that, too? (I skimmed nsCSSRendering::DetermineBackgroundColor and it didn't appear to do this either, but perhaps I just missed it?)

Flags: needinfo?(emilio)

This is a problem in preferences and a few other places too, lemme find the bug for that.

Flags: needinfo?(mreschenberg)
See Also: → 1723742
See Also: → 1724045

(In reply to :Gijs (he/him) from comment #4)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #3)

So what's going on is that the warning icon has a background-image like this:

  background-image: url("chrome://global/skin/icons/warning.svg");

So when we end up finding a color to backplate, we end up here and choose the default background color, which since the previous bug no longer uses the system colors... But since the system color in use for the text is white, it ends up being unreadable.

So in this case we could either change the break above by a continue (to find the "upper" background-color) or go back to use the system colors in HCM... But probably the first solution is better.

OK, so that might work here, I'll give it a shot, though I'm unsure what else would break...

I don't think it should break much. I think the code was relying on system colors not being honored, which changed along the way, so the foreground color would be the default color.

However, reading this code I have a related question: this loop is informed by the background image, but in this case (ie in about:certerror and friends) there is no visual overlap between the text we're backplating and the background image's size + position. Can/should we detect that case? I suspect it is not uncommon, and it sounds like we could skip based on that, too? (I skimmed nsCSSRendering::DetermineBackgroundColor and it didn't appear to do this either, but perhaps I just missed it?)

Yeah, this seems a bit tricky to do. At best we can do it based on background-size/repeat/position, but seems overcomplicated because the text may be anywhere in the subtree, or there may be e.g. an abspos deep down below which overlaps with the background.

Flags: needinfo?(emilio)
Assignee: nobody → emilio
Status: NEW → ASSIGNED
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/db4aa060f966
Keep looking for background-colors when finding a background-image with transparent background-color. r=morgan
Whiteboard: [fidefe-mr11-cleanup] → [fidefe-mr11-cleanup] [access-s2]
Flags: needinfo?(emilio)
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/77571a2d6d5b
Keep looking for background-colors when finding a background-image with transparent background-color. r=morgan
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch

Reproduced the issue on the affected Nightly 92.0a1 (2021-08-03) on Windows 10 x64 and can confirm it as verified-fixed on the latest Nightly 92.0a1 2021-08-09.

Status: RESOLVED → VERIFIED
Whiteboard: [fidefe-mr11-cleanup] [access-s2] → [fidefe-mr11-cleanup] [access-s2][hcm-2021-h2]
Has Regression Range: --- → yes
See Also: 1724045
Accessibility Severity: --- → s2
Whiteboard: [fidefe-mr11-cleanup] [access-s2][hcm-2021-h2] → [fidefe-mr11-cleanup] [hcm-2021-h2]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: