1429288 - [Linux] addon permissions alert is unreadable when using dark-background themes

Status: RESOLVED FIXED

(Firefox :: Theme, defect, P3)

Description

[Paul Bone [:pbone]]

There was an orange symbol on my Firefox's menu, usually where Nightly puts a green symbol when there's a new version available.  I clicked it, in the menu there was an new item that I couldn't read.  A friend read it for me and said it was "Gecko profiler requires new permissions".

It looks like the menu item is painted with a grey or white forground colour but a yellow background, even people with normal eyesight struggled to read it from the screenshot (attached).

I'm using linux mint and my desktop has a dark-ish theme.  I am using the default Firefox theme.  I have different settings to improve the contrast on web pages, I have those documented here: http://paul.bone.id.au/2017/11/26/how-i-see-the-web/

Thanks.

Comment 1

[Paul Bone [:pbone]]

Attachment: screenshot-orange.png

Here's the screenshot showing the problem.

Comment 2

[Jared Wein [:jaws] (please needinfo? me)]

The theming that you have applied to the Firefox menu is not supported and thus this notice isn't properly adjusted to account for a dark background/light text.

This should be fixed by https://bugzilla.mozilla.org/show_bug.cgi?id=1417880 (and later by https://bugzilla.mozilla.org/show_bug.cgi?id=1408121).

Comment 3

[Paul Bone [:pbone]]

I don't understand why this is not supported.

I chose a desktop theme in Linux Mint, I installed Firefox, I made the configuration changes (in the preferences pannel, not about:config!) as described in my blog post, and I don't think these affect the menu anyway, and I couldn't read this one menu item.  What could be unsupported there.  If Firefox won't support my desktop theme then could it please tell me so?!  And if I had picked a firefox theme how would that change the support?

All it needs to do is paint the text in black when the background is yellow.  I didn't theme the background to be yellow, But my desktop may have set the default forground to be white.

All you need to do is when using a non-default colour (diverging from eg a desktop theme) for either forground or background, then ALSO specify the colour of the other (background or forground).  This alert won't "fit in" with the general theme, but at least it'll be more readable then WHITE ON YELLOW.

Comment 4

[:Gijs (he/him)]

Yeah, this is just a bug in the theme that is hard to notice because these banners almost never appear, and the only place where the menu will have a dark background without high contrast mode enabled is Linux. I would also not be surprised if color is set somewhere but -moz-appearance and/or use of !important in the toolkit styles overrides it.

Comment 5

[Dão Gottwald [:dao]]

Attachment: Bug 1429288 - Fix up app menu warning stripe colors.

Review commit: https://reviewboard.mozilla.org/r/212058/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/212058/

Comment 6

[:Gijs (he/him)]

Comment on attachment 8941813 [details]
Bug 1429288 - Fix up app menu warning stripe colors.

https://reviewboard.mozilla.org/r/212058/#review217780

r=me, thanks for picking this up so quickly.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5acf096b4c7c
Fix up app menu warning stripe colors. r=Gijs

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/5acf096b4c7c

Comment 9

[Paul Bone [:pbone]]

(In reply to :Gijs from comment #4)
> Yeah, this is just a bug in the theme that is hard to notice because these
> banners almost never appear, and the only place where the menu will have a
> dark background without high contrast mode enabled is Linux. I would also
> not be surprised if color is set somewhere but -moz-appearance and/or use of
> !important in the toolkit styles overrides it.

Thanks.

How are the themes developed. Do theme authors get to see a mock-up of every widget they are themeing?  There are also tools for calculating whether something has approprite contrast (light or dark).  These are just ideas/feedback.

Comment 10

[Paul Bone [:pbone]]

(In reply to :Gijs from comment #6)
> Comment on attachment 8941813 [details]
> Bug 1429288 - Fix up app menu warning stripe colors.
> 
> https://reviewboard.mozilla.org/r/212058/#review217780
> 
> r=me, thanks for picking this up so quickly.

Yes, thank you Dão.

Comment 11

[:Gijs (he/him)]

(In reply to Paul Bone [:pbone] from comment #9)
> (In reply to :Gijs from comment #4)
> > Yeah, this is just a bug in the theme that is hard to notice because these
> > banners almost never appear, and the only place where the menu will have a
> > dark background without high contrast mode enabled is Linux. I would also
> > not be surprised if color is set somewhere but -moz-appearance and/or use of
> > !important in the toolkit styles overrides it.
> 
> Thanks.
> 
> How are the themes developed.

I assume you mean the default theme, and the light/dark themes we ship with Firefox? By writing CSS in a text editor and checking the result, submitting patches, getting review. Much like the rest of Firefox's codebase.

> Do theme authors get to see a mock-up of every widget they are themeing?

I think there's some kind of misconception here. We are the theme authors (in terms of code) - Firefox frontend CSS is written, by and large, by the same people who write the rest of the frontend code (JS, XUL, etc.). We're not "theming" something that already exists, we're just implementing UI as a whole.

A substantial portion of the CSS is shared across the 3 main OSes Firefox runs on (Linux, Mac, Windows). Some of the rest is OS-specific. We rely on system colours in some places, and on hardcoded colours in other places (because OSes don't generally offer system colours for attention or other states). The UI needs to look acceptable on all these OSes, "fit in" but not stop being Firefox (e.g. tabs couldn't be in the titlebar on Mac/Windows (and soon Linux) if we were more stringent about OS look'n'feel). 

All of those OSes have their quirks. Windows has high contrast mode, Windows 7 has classic, aero basic, and translucent aero glass, plus windows 8+ where high contrast themes aren't identified in the same way as on Windows 7, and Windows 10 where we have to mess around with window backgrounds differently to Windows 7/8; macOS now has a 'dark mode' as well as a weird version of translucency (so the app that is in the background affects the background colour of certain parts of the UI); Linux is basically a jungle - some window managers don't support partial transparency, some gtk themes do dumb things in that certain system colour combinations don't look right even though they should; the new "dark" mode in gtk messes with everything in a slightly different way, our gtk layer for system colours is sometimes buggy, etc. etc.

Then there's the fact that sometimes background/foreground colours aren't explicit in the CSS but part of the widget code we ship and its interpretation of a particular -moz-appearance value.

This particular banner would most likely have looked fine to people testing it manually on Linux, because only a minority of GTK themes make menus (including ours) be light-text-on-dark-background, and as noted before, these banners rarely (if ever) show up.

Showing a "mock-up" doesn't really make sense - there's no way to predict what values system colours are, what the effect of semitransparency/translucency or other OS-specific colours of background items is going to be, or how other style rules may or may not interfere with the specific code you're writing right now, or what things will look like on specific Linux themes if you're writing the patch on $otherOS. And even if there was, there'd be so many different configurations that checking all the results manually would be madness. The best way to check things manually when adding new things is to just run the browser, and apply logic while reading the CSS.

We now have some automated testing called mozscreenshots that produces screenshots of the browser in various configurations, and does automatic comparisons to alert us when things change so we can verify changes are expected. But it turns out there's a lot of UI in the browser, and not all of the UI has configurations yet, so there's likely no mozscreenshots screenshot that shows this particular bit of UI and would flag up anything. It's not clear that it would be feasible to create and compare such screenshots for every part of the UI (it'd basically require duplicating most of the automated testing with the sole purpose of making screenshots - it'd be a massive amount of work and likely generate terabytes of screenshot data per run).

If anything, we could consider a code lint that complained about rules that specify a background without specifying a foreground, but that would have a *lot* of false positives, and not work for the -moz-appearance stuff, or elements that don't have text in them. Plus you'd likely not want to do it for :hover/:active styling because it can likely just use the same foreground as the non-hover/active case, except in some cases it can't so then you miss those... Same thing for semitransparent background colours or cases where you deliberately inherit foreground colours even when changing background colours.

This is just a hard problem to solve in categorical fashion.

>  There are also tools for calculating whether
> something has approprite contrast (light or dark).  These are just
> ideas/feedback.

Yes, I'm aware of the relevant WCAG standards and various contrast checking tools, as are the UX designers. The relevant CSS to set a foreground colour here was simply missed, because in the common (99%+ of users / developer machines) case, no extra CSS was needed. Bugs happen.

Comment 12

[Paul Bone [:pbone]]

(In reply to :Gijs from comment #11)
> (In reply to Paul Bone [:pbone] from comment #9)
> > (In reply to :Gijs from comment #4)
> > > Yeah, this is just a bug in the theme that is hard to notice because these
> > > banners almost never appear, and the only place where the menu will have a
> > > dark background without high contrast mode enabled is Linux. I would also
> > > not be surprised if color is set somewhere but -moz-appearance and/or use of
> > > !important in the toolkit styles overrides it.
> > 
> > Thanks.
> > 
> > How are the themes developed.
> 
> I assume you mean the default theme, and the light/dark themes we ship with
> Firefox? By writing CSS in a text editor and checking the result, submitting
> patches, getting review. Much like the rest of Firefox's codebase.

The default themes (the oens that ship with Firefox) and user contributed themes.

> > Do theme authors get to see a mock-up of every widget they are themeing?
> 
> I think there's some kind of misconception here. We are the theme authors
> (in terms of code) - Firefox frontend CSS is written, by and large, by the
> same people who write the rest of the frontend code (JS, XUL, etc.). We're
> not "theming" something that already exists, we're just implementing UI as a
> whole.

Sorry, I was attempting to include the user contributed themes as well as the default ones, and picked phrasing that mislead.

> ...

That all makes sense, some of it was as I expected but I also assumed that things were a bit more magical on Windows, there was some "accessability flag" that Firefox queried or something.

> macOS now has a 'dark mode'

Ohhh, that was one of the main reasons I went back to Linux.  I'm pleased they've added that!

> Linux is basically a jungle - some
> window managers don't support partial transparency, some gtk themes do dumb
> things in that certain system colour combinations don't look right even
> though they should; the new "dark" mode in gtk messes with everything in a
> slightly different way, our gtk layer for system colours is sometimes buggy,
> etc. etc.

That's what I was worried about.

> Then there's the fact that sometimes background/foreground colours aren't
> explicit in the CSS but part of the widget code we ship and its
> interpretation of a particular -moz-appearance value.
> 
> This particular banner would most likely have looked fine to people testing
> it manually on Linux, because only a minority of GTK themes make menus
> (including ours) be light-text-on-dark-background, and as noted before,
> these banners rarely (if ever) show up.

Okay, that's what I thought happened.  My advice (which was mainly for other not-Firefox apps in the past, and websites) is whenever something specifies one of the two fore/back colours, then it should specify the other.  But that'd mess with transperancy...

> Showing a "mock-up" doesn't really make sense - there's no way to predict
> what values system colours are, what the effect of
> semitransparency/translucency or other OS-specific colours of background
> items is going to be, or how other style rules may or may not interfere with
> the specific code you're writing right now, or what things will look like on
> specific Linux themes if you're writing the patch on $otherOS. And even if
> there was, there'd be so many different configurations that checking all the
> results manually would be madness. The best way to check things manually
> when adding new things is to just run the browser, and apply logic while
> reading the CSS.

Hrm, a lot more complex than I was expecting ( things often are :-( ).

> We now have some automated testing called mozscreenshots that produces
> screenshots of the browser in various configurations, and does automatic
> comparisons to alert us when things change so we can verify changes are
> expected. But it turns out there's a lot of UI in the browser, and not all
> of the UI has configurations yet, so there's likely no mozscreenshots
> screenshot that shows this particular bit of UI and would flag up anything.
> It's not clear that it would be feasible to create and compare such
> screenshots for every part of the UI (it'd basically require duplicating
> most of the automated testing with the sole purpose of making screenshots -
> it'd be a massive amount of work and likely generate terabytes of screenshot
> data per run).

particularly for each distro/desktop environment.

> ...
> 
> This is just a hard problem to solve in categorical fashion.

:-(

> >  There are also tools for calculating whether
> > something has approprite contrast (light or dark).  These are just
> > ideas/feedback.
> 
> Yes, I'm aware of the relevant WCAG standards and various contrast checking
> tools, as are the UX designers. The relevant CSS to set a foreground colour
> here was simply missed, because in the common (99%+ of users / developer
> machines) case, no extra CSS was needed. Bugs happen.

It's cool.  It's why I took the time to file it.  And now I feel validated that I did.  But I'm still confused about the role of theme detection.

So what else does the browser do when it detects a dark theme?  Should we add an option in about:prefereces where users say "My desktop has a dark/light theme" overriding the detection?

Comment 13

[:Gijs (he/him)]

(In reply to Paul Bone [:pbone] from comment #12)
> So what else does the browser do when it detects a dark theme?

We don't specifically detect dark themes as opposed to light themes. There are 2 things that influence appearance (besides the obvious "whatever the theme CSS says"):

- system colour/appearance. This is basically what the OS theme claims a specific part of UI (window, toolbar, scrollbar, button, ...) should look like, mediated through both our widget layer (which translates that to CSS colour and font constants, as well as through -moz-appearance values) and CSS (which uses the colour/font constants and -moz-appearance values in hopefully the right/sensible places). The same -moz-appearance value has different effects on different OSes, and the same colour/font constants have different values depending on the OS and theme in use. Bugs here appear through all layers of this stack, from the OS themes themselves through to our widget and CSS layers.
- high contrast mode on Windows will (with default pref settings) cause web content colours, images and background images to be overridden by the user- or system-specified defaults. We don't currently implement high contrast detection to do the same thing on Linux, see bug 239914 and bug 1064164, or Mac (high contrast is kinda weird on Mac anyway and it's questionable whether doing that would even be the right behaviour because I don't think Safari does it, either; in any case I'm not aware we have a bug on file for doing work on that)

>   Should we
> add an option in about:prefereces where users say "My desktop has a
> dark/light theme" overriding the detection?

So this doesn't match the implementation as we don't detect "dark" vs "light" themes separately (generally speaking; there's some voodoo magic about Windows titlebar colours in various places but that's an edgecase that's not really relevant here). There is already a pref to *always* override system colours, which based on your blogpost you already know about. We could try to fix the "detect high contrast on linux" bug but that wouldn't have helped this bug.

This bug is not really related to all of the high contrast detection. It turned out to be a bug in our CSS. I know there are other bugs in our widget/gtk layer about how we get various system colours, and there are also likely more CSS bugs lurking in places. The gtk2->gtk3 change also changed a few things. Speaking generally, those can all individually be fixed, but I'm not aware of an overarching "we don't get any of the right colours" bug.

Comment 14

[Paul Bone [:pbone]]

(In reply to :Gijs from comment #13)
> (In reply to Paul Bone [:pbone] from comment #12)
> > So what else does the browser do when it detects a dark theme?
> 
> - high contrast mode on Windows will (with default pref settings) cause web
> content colours, images and background images to be overridden by the user-
> or system-specified defaults. We don't currently implement high contrast
> detection to do the same thing on Linux, see bug 239914 and bug 1064164, or
> Mac (high contrast is kinda weird on Mac anyway and it's questionable
> whether doing that would even be the right behaviour because I don't think
> Safari does it, either; in any case I'm not aware we have a bug on file for
> doing work on that)

That sounds like what I was thinking of after discussing it so far.

So does this override web colours by setting specific fore/background and link colours and then always using those?  Does it prevent CSS from setting a background image (which removes some icons? because some sites use background images to implement their buttons).

I don't know what it _should_ do, I have no good answers mostly because each person with a disability is unique and will require something different, plus they may just have different preferences!  But I'm curious.

> >   Should we
> > add an option in about:prefereces where users say "My desktop has a
> > dark/light theme" overriding the detection?
> 
> So this doesn't match the implementation as we don't detect "dark" vs
> "light" themes separately (generally speaking; there's some voodoo magic
> about Windows titlebar colours in various places but that's an edgecase
> that's not really relevant here). There is already a pref to *always*
> override system colours, which based on your blogpost you already know
> about. We could try to fix the "detect high contrast on linux" bug but that
> wouldn't have helped this bug.

Okay, same question s/dark/high-contrast.  But I guess that is approximately the "always override colours" option.

> This bug is not really related to all of the high contrast detection.

Okay, that's something I was wondering, whether it would not appear in windows because of the high contrast detection.  (or simply different GUI themes and code in windows).

> It
> turned out to be a bug in our CSS. I know there are other bugs in our
> widget/gtk layer about how we get various system colours, and there are also
> likely more CSS bugs lurking in places. The gtk2->gtk3 change also changed a
> few things. Speaking generally, those can all individually be fixed, but I'm
> not aware of an overarching "we don't get any of the right colours" bug.

I'll keep an eye out.

Thanks.

(I'm very happy with these clarifications.  Thanks again.)

Comment 15

[Paul Bone [:pbone]]

(In reply to Paul Bone [:pbone] from comment #14)
> (In reply to :Gijs from comment #13)
> > (In reply to Paul Bone [:pbone] from comment #12)
> > > So what else does the browser do when it detects a dark theme?
> > 
> > - high contrast mode on Windows will (with default pref settings) cause web
> > content colours, images and background images to be overridden by the user-
> > or system-specified defaults. We don't currently implement high contrast
> > detection to do the same thing on Linux, see bug 239914 and bug 1064164, or
> > Mac (high contrast is kinda weird on Mac anyway and it's questionable
> > whether doing that would even be the right behaviour because I don't think
> > Safari does it, either; in any case I'm not aware we have a bug on file for
> > doing work on that)
> 
> That sounds like what I was thinking of after discussing it so far.
> 
> So does this override web colours by setting specific fore/background and
> link colours and then always using those?  Does it prevent CSS from setting
> a background image (which removes some icons? because some sites use
> background images to implement their buttons).

That information is in Bug 239914, thanks for that link.  All my curiousity is now resolved ;-).