Open Bug 1593449 Opened 5 years ago Updated 2 years ago

Unnecessary readability backplates are displayed in DevTools with browser.display.document_color_use=2

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

People

(Reporter: ntim, Unassigned)

References

(Regression)

Details

(Keywords: regression)

Attachments

(1 file)

Attached image Screenshot of bug (deleted) —

https://searchfox.org/mozilla-central/rev/59de675101da711520c0bb6e34a1ea2372e7ddbb/layout/generic/nsBlockFrame.cpp#6854-6855

I think this is because we're checking against PresContext()->IsChrome() instead of ShouldUseChromePrefs meaning all the logic added for DevTools in bug 1575766 isn't taken in account.

Flags: needinfo?(mreschenberg)

(Note that I couldn't check if this affects browser.display.document_color_use=0 since Firefox doesn't integrate with macOS' high contrast mode)

(In reply to Tim Nguyen :ntim from comment #0)

I think this is because we're checking against PresContext()->IsChrome() instead of ShouldUseChromePrefs meaning all the logic added for DevTools in bug 1575766 isn't taken in account.

That sounds believable.

Though, also: do you know what's making the background so dark in the right pane? Does that area have a dark background regardless of this pref-setting, or does this pref-tweak seem to give that area a dark background? (I'm concerned that if we simply remove the white backplates over there, then that text may be hard to read...)

Flags: needinfo?(ntim.bugs)

This is a known issue as dev tools doesn't seem to work as expected with (new or old) HCM (see: 1590215).

I think I'd rather go the route of adjusting dev tools for HCM than removing backplating since, as @dholbert mentioned, devtools is inaccessable without backplates currently.
Also: bug 1575766 is based off of edge comparability, and edge's devtools are HCM responsive and inherit HCM formatting (likely they're backplated, too, but the backplate renders on a same-colour background making it indistinguishable).

Flags: needinfo?(mreschenberg)

(In reply to Daniel Holbert [:dholbert] from comment #2)

Though, also: do you know what's making the background so dark in the right pane? Does that area have a dark background regardless of this pref-setting, or does this pref-tweak seem to give that area a dark background? (I'm concerned that if we simply remove the white backplates over there, then that text may be hard to read...)

That's simply the non-high contrast colors for DevTools dark theme. The devtools team has decided to disable the high contrast color adjustments (this is what bug 1575766 has done) until they have time to fix the CSS to work well with HCM, which means that right now, colors appear as authored in the CSS.

I think the backplate should only apply if we are doing the color adjustments, which is not the case here.

Flags: needinfo?(ntim.bugs)

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

I think I'd rather go the route of adjusting dev tools for HCM than removing backplating since, as @dholbert mentioned, devtools is inaccessable without backplates currently.

Ultimately yes -- and that's bug 1593449, I think.

However: right now, the backplates introduce different & perhaps-worse accessibility issues right now (in the dark theme at least). Look at the top left of the devtools panel in ntim's screenshot (the element-picker icon and the "inspector" title). Both of those are unreadable white-on-white. This happens because we're not forcing the text to be a certain color, which means we have white text and we're inserting a white backplate underneath.

So for the time being (until we've addressed bug 1593449), I think we should make the change that ntim suggested (conditioning on ShouldUseChromePrefs).

As ntim said, "I think the backplate should only apply if we are doing the color adjustments, which is not the case here." -- that's exactly correct.

I might steal this as a good-first-bug for a new hire starting today... Morgan, let me know if this^ approach sounds OK to you, and if us stealing this would collide with anything you're doing.

Flags: needinfo?(dholbert)

(In reply to Daniel Holbert [:dholbert] from comment #5)

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

I think I'd rather go the route of adjusting dev tools for HCM than removing backplating since, as @dholbert mentioned, devtools is inaccessable without backplates currently.

Ultimately yes -- and that's bug 1593449, I think.

Is that meant to be a link to this bug?

However: right now, the backplates introduce different & perhaps-worse accessibility issues right now (in the dark theme at least). Look at the top left of the devtools panel in ntim's screenshot (the element-picker icon and the "inspector" title). Both of those are unreadable white-on-white. This happens because we're not forcing the text to be a certain color, which means we have white text and we're inserting a white backplate underneath.

So for the time being (until we've addressed bug 1593449), I think we should make the change that ntim suggested (conditioning on ShouldUseChromePrefs).

Confusing bug link x2 Is this supposed to be the "HCM doesn't apply to devtools" bug?

As ntim said, "I think the backplate should only apply if we are doing the color adjustments, which is not the case here." -- that's exactly correct.

I might steal this as a good-first-bug for a new hire starting today... Morgan, let me know if this^ approach sounds OK to you, and if us stealing this would collide with anything you're doing.

Okay! Yeah I think that sounds reasonable for now; I'd like to make sure this eventually gets resolved to include our HCM a11y changes, what's the best way to track that?

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

(In reply to Daniel Holbert [:dholbert] from comment #5)

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

I think I'd rather go the route of adjusting dev tools for HCM than removing backplating since, as @dholbert mentioned, devtools is inaccessable without backplates currently.

Ultimately yes -- and that's bug 1593449, I think.

Is that meant to be a link to this bug?
[...]
Confusing bug link x2 Is this supposed to be the "HCM doesn't apply to devtools" bug?

Sorry, mispaste -- I meant bug 1590215.

I might steal this as a good-first-bug for a new hire starting today... Morgan, let me know if this^ approach sounds OK to you, and if us stealing this would collide with anything you're doing.

Okay! Yeah I think that sounds reasonable for now; I'd like to make sure this eventually gets resolved to include our HCM a11y changes, what's the best way to track that?

Actually I think I found a better bug, so this one's up for grabs after all. :D

RE the best way to track it: if this ends up being a temporary-band-aid that's replaced down the line, and you want to track the band-aid-ness, then we'd maybe want "better fix coming in bug NNN" or something like that, in a code-comment, and bug NNN would maybe want to have a mention of this bug here as a reminder to swap out the band-aid.) (If this doesn't answer your question, maybe I can advise further on slack)

Flags: needinfo?(dholbert)
Has Regression Range: --- → yes
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: