Open Bug 1791024 Opened 2 years ago Updated 1 years ago

Focus blue outline should not override HCM colors

Categories

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

defect

Tracking

()

Accessibility Severity s3

People

(Reporter: sclements, Unassigned, NeedInfo)

References

(Blocks 1 open bug)

Details

(Keywords: access)

During a recent accessibility review for Firefox View, it was drawn to our attention that the focus outline in HCM on Windows has a double outline. It looks like this was part of a UX improvement in bug 1776592 but it should probably respect focus colors in HCM mode and not have that blue outline.

We have this issue tracked in bug 1786397 and even though its marked as resolved it'd be preferable if we didn't have to override this hard coded blue outline with media queries. (I don't see this issue in Mac, only in Windows.)

Blocks: 1791028

The severity field is not set for this bug.
:spohl, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(spohl.mozilla.bugs)
Keywords: access
Whiteboard: [access-s3]
Severity: -- → S3
Flags: needinfo?(spohl.mozilla.bugs)
Priority: -- → P3

I know this is just clean up and pretty low priority, but wanted to check in on status. Any updates or plans to address?

Component: Widget → Widget: Win32

Can you elaborate on what difference do you see? I'm not sure I'm following. Is the issue that we behave as outline: auto even if you specify another oitline-color? Or something else?

Flags: needinfo?(sclements)

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

Can you elaborate on what difference do you see? I'm not sure I'm following. Is the issue that we behave as outline: auto even if you specify another oitline-color? Or something else?

Hi Emilio, the issue is that the blue part of the double outline is hardcoded and overrides HCM focus colors. See Anna's comment and screenshot where this bug was originally reported: https://bugzilla.mozilla.org/show_bug.cgi?id=1786397#c2

Flags: needinfo?(sclements)

It's not hard-coded? You can set accent-color: Highlight or somesuch to override it?

Flags: needinfo?(sclements)

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

It's not hard-coded? You can set accent-color: Highlight or somesuch to override it?

We did manually override it in the style sheet, in a prefers-contrast media query to get around this. I'm curious what the a11y team thinks, but it seems like you shouldn't have to override it (especially since this change was a surprise and it happened to be caught during an accessibility review).

Flags: needinfo?(sclements) → needinfo?(mreschenberg)

(In reply to Sarah Clements [:sclements] from comment #6)

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

It's not hard-coded? You can set accent-color: Highlight or somesuch to override it?

We did manually override it in the style sheet, in a prefers-contrast media query to get around this. I'm curious what the a11y team thinks, but it seems like you shouldn't have to override it (especially since this change was a surprise and it happened to be caught during an accessibility review).

Hey Sarah, can you tell me more about what you tried here? Where did you attempt this override and what did you see as a result? Also: what platform are you testing on?

Flags: needinfo?(mreschenberg)
Flags: needinfo?(sclements)

(In reply to Morgan Reschenberg [:morgan] from comment #7)

(In reply to Sarah Clements [:sclements] from comment #6)

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

It's not hard-coded? You can set accent-color: Highlight or somesuch to override it?

We did manually override it in the style sheet, in a prefers-contrast media query to get around this. I'm curious what the a11y team thinks, but it seems like you shouldn't have to override it (especially since this change was a surprise and it happened to be caught during an accessibility review).

Hey Sarah, can you tell me more about what you tried here? Where did you attempt this override and what did you see as a result? Also: what platform are you testing on?

Hi Morgan, tgiles did some styling to fix this issue in the Firefox View style sheets and it does solve the issue in firefox view - see the patch in bug 1786397. The crux of the issue I'm trying to get it as should we have to override this? If the answer is "yes", then we can close out this bug (though maybe this should be communicated to folks on the front end?). But my impression is that this was a side-effect of bug 1776592.

Flags: needinfo?(sclements)

(In reply to Sarah Clements [:sclements] from comment #8)

Hi Morgan, tgiles did some styling to fix this issue in the Firefox View style sheets and it does solve the issue in firefox view - see the patch in bug 1786397. The crux of the issue I'm trying to get it as should we have to override this? If the answer is "yes", then we can close out this bug (though maybe this should be communicated to folks on the front end?). But my impression is that this was a side-effect of bug 1776592.

ahhh gotcha -- yeah this is a bit confusing. the patch that added the dual-outline uses accent-color (which fetches that blue). that colour doesn't get overridden when HCM is enabled the way background, foreground, and link colours do. you could argue we should be overriding accent-color, but I think that override would look different across platforms -- on mac, for example, we probably want to continue to use the system accent color when prefers-contrast is triggered. On windows, though, we might want something more like SelectedItem? Hard to say. I imagine the preferred override also depends on the context we're using accent color in.

That said, I think it's reasonable to change the code in widget/Theme.cpp to use something other than accent colour to draw the double outline when HCM is on. That feels like a localised/safe change, and I can post a patch :)

Ah looks like someone (emilio?) addressed this by bringing back ComputeFocusRectColors and adding HCM casing. I think this should "just work" without the media query overrides now.

Flags: needinfo?(sclements)
Accessibility Severity: --- → s3
Whiteboard: [access-s3]
You need to log in before you can comment on or make changes to this bug.