1694384 - Poor contrast on hover/active with proton appmenu with dark system theme.

Status: VERIFIED FIXED

(Firefox :: Menus, defect, P1)

Description

[Emilio Cobos Álvarez (:emilio)]

Attachment: image.png

https://searchfox.org/mozilla-central/rev/f47a4b67643b3048ef9a2e2ac0c34edf6d1ebff3/browser/themes/shared/customizableui/panelUI.inc.css#64-67 is specifying a hardcoded background but inheriting a system color. That's not great if the system color is light :)

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1694384 - Fix contrast issues with GTK dark themes on the proton app menu. r=mconley

Not sure if you want to take something like this or do something else,
really your call, but I did this so I may as well post it.

The opacity change helps with the regular dark theme as well.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1691152

Comment 3

[:Gijs (he/him)]

This is assigned, but it looks like a straight dupe of bug 1693010?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Yeah, I had a patch but quoting Mike:

The Proton dark theme is still being defined, and we should have a clearer path forward for how to apply it later this week. I'm going to resign from this review until then.

So I decided to abandon the revision from now, we can revive it later if needed.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1694384 - Separate Proton AppMenu colors into a separate rule block in an include file. r?ntim!

This should, I think, offer some relief to folks using the current built-in
dark theme.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Hey ntim, is this the right idea? What I've got here is roughly:

:root {
  (proven colors that we've shipped before for things)
}

@supports -moz-bool-pref("browser.proton.enabled") {
  :root {
    (non color things)
  }

  :root:not(:-moz-lwtheme) {
    (Proton default theme color values)
  }
}

This seems to improve the AppMenu for the built-in themes. Am I on the right track here?

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Speaking with ntim, I believe he's recommending that I do the following:

1. In browser/themes/linux/customizableui/panelUI.css, make sure that the colors set on the :root are from the "known proven set" of colours that work on Linux.
2. Inside of panelUI.inc.css, wrap the :root:not(:-moz-lwtheme) collection of colours with a @media not (prefers-contrast) query

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/172ab28afcf1
Separate Proton AppMenu colors into a separate rule block in an include file. r=harry

Comment 9

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/172ab28afcf1

Comment 13

[Alin Ilea, Desktop QA]

Verified - Fixed in release 88, beta 89.0b5 and latest Nightly 90.0a1 (04-28-2021) using Ubuntu 20.04 and Windows 10 with dark system theme.