[e10s] select popups on Linux lack sufficient contrast (when dom.forms.select.customstyling is enabled)

NEW
Unassigned

Status

()

defect
2 years ago
3 months ago

People

(Reporter: jaws, Unassigned)

Tracking

(Blocks 2 bugs, {regression})

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54- disabled)

Details

(URL)

Attachments

(4 attachments, 1 obsolete attachment)

Since bug 910022 and its follow-ups have landed, we are lacking sufficient contrast in the popups on Linux.

Bug 910022 implemented the ability for pages to style the <select> popup. The style is calculated by getting the computed styles of the <select> and its children. The styles are then forwarded to the parent process and used on the <menupopup> and its  children, respectively. We also make an attempt to calculate the default color values for these elements and only apply the colors if the computed value differs from the default colors.

This approach appears to work fine for Windows and OSX, however on some GTK themes (maybe all?), this approach doesn't work. For example, the <select> button on Ubuntu 16.10 has a light grey background with dark grey text. But the <select> popup by default has a dark grey background with a light grey text. Our computation of the default <select> background returns the light grey background, likely because it is using the button's colors instead of the popup's colors. This button's colors are forwarded to the parent process and applied to the <menupopup>. The <option> colors that are computed match the default user agent styling and are not applied to the menuitem's in the parent process, but the parent process has a default styling of using color:MenuText which is light by default in these same themes.

This ends up giving the result of a light grey background for the popup and a light grey text (just slightly darker than the background).

Dao, do you have any ideas what we can do here? You can see this code in /toolkit/modules/SelectContentHelper.jsm (look for _calculateUAColors) and /toolkit/modules/SelectParentHelper.jsm (look for multiple uses of sheet.insertRule).
Flags: needinfo?(dao+bmo)
Flags: needinfo?(dao+bmo)
I had been wondering what broke select popups. Interestingly they appear to be working again in the current nightly. Has this feature been disabled on Linux?

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #0)
> This approach appears to work fine for Windows and OSX,

Have you tested High Contrast and Win7 / Classic themes too?

> Our computation of the default <select> background returns the light
> grey background, likely because it is using the button's colors instead of
> the popup's colors.

I hope you're not actually using the button to get that color, because it seems obvious that this couldn't possibly do the right thing when the button and the popup don't use matching colors.

Assuming you're trying to get the background color from the menupopup rather than from the button, what I think is happening is that you don't get the actual color being rendered, because that comes from -moz-appearance. toolkit/themes/linux/global/popup.css does not even set a background-color.

Can you use only the text color? I.e. assume a dark background for light text and vice versa, and maybe fall back on background-color when the text color lightness is in some middle range.
Flags: needinfo?(jaws)
(In reply to Dão Gottwald [:dao] from comment #1)
> I had been wondering what broke select popups. Interestingly they appear to
> be working again in the current nightly. Has this feature been disabled on
> Linux?

No the feature wasn't disabled. Bug 1338219 just landed in time for today's Nightly so that's why it looks to be fixed.

> (In reply to Jared Wein [:jaws] (please needinfo? me) from comment #0)
> > This approach appears to work fine for Windows and OSX,
> 
> Have you tested High Contrast and Win7 / Classic themes too?

I just tested with all of the High Contrast options on Windows 10 and it works fine there.

> > Our computation of the default <select> background returns the light
> > grey background, likely because it is using the button's colors instead of
> > the popup's colors.
> 
> I hope you're not actually using the button to get that color, because it
> seems obvious that this couldn't possibly do the right thing when the button
> and the popup don't use matching colors.

We are creating a <select> element and calling getComputedStyle on it. GTK seems to be the only platform that uses different colors for the button vs the popup. 

> Assuming you're trying to get the background color from the menupopup rather
> than from the button, what I think is happening is that you don't get the
> actual color being rendered, because that comes from -moz-appearance.
> toolkit/themes/linux/global/popup.css does not even set a background-color.

Yeah, this is problematic. getComputedStyle gets us the various CSS colors used at different stages, but not the composited color that is actually displayed on screen. We would have to use something like -moz-element combined with context2d.drawWindow to get the colors of the individual pixels.

> Can you use only the text color? I.e. assume a dark background for light
> text and vice versa, and maybe fall back on background-color when the text
> color lightness is in some middle range.

I don't know how we could use text color to infer background-color, though this route does seem like it would be the most durable. getComputedStyle for color seems to always return a usable color, though if it returned 'transparent' then we would be SOL. Using just text color to infer background-color means that we might end up using background-colors that don't match the platform styling. Perhaps I'm not thinking of it the way you are though, does the code at http://searchfox.org/mozilla-central/source/toolkit/modules/SelectContentHelper.jsm#139-154 seem reasonable to you?
Flags: needinfo?(jaws) → needinfo?(dao+bmo)
(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #2)
> (In reply to Dão Gottwald [:dao] from comment #1)
> > I had been wondering what broke select popups. Interestingly they appear to
> > be working again in the current nightly. Has this feature been disabled on
> > Linux?
> 
> No the feature wasn't disabled. Bug 1338219 just landed in time for today's
> Nightly so that's why it looks to be fixed.

I take that back, it's broken again. Requesting tracking because we can't ship like this.

> > Can you use only the text color? I.e. assume a dark background for light
> > text and vice versa, and maybe fall back on background-color when the text
> > color lightness is in some middle range.
> 
> I don't know how we could use text color to infer background-color, though
> this route does seem like it would be the most durable.

You can try to infer whether the background is light or dark. If you need the actual color, this doesn't work.

> getComputedStyle for
> color seems to always return a usable color, though if it returned
> 'transparent' then we would be SOL.

It should never be transparent.

> does the code at
> http://searchfox.org/mozilla-central/source/toolkit/modules/
> SelectContentHelper.jsm#139-154 seem reasonable to you?

It assumes that the popup uses certain CSS platform colors which is just not true. (Also, if you're just interested in what -moz-comboboxtext, -moz-combobox, -moz-fieldtext, -moz-field compute to, you might as well use <span> instead of <select> and <option>.)
Flags: needinfo?(dao+bmo)
Tracking 54+ for this issue.
Depends on: 1335483
Here's a screenshot showing what this looks like right now (in the Bugzilla attachment "details" page), on Ubuntu 16.10, in Nightly 54.0a1 (2017-02-13) (64-bit)
In the interim, this will be solved by bug 1339966 which will disable this feature for Linux.
[Tracking Requested - why for this release]: I propose that we stop tracking this for 54 since it has been disabled by bug 1339966 on Linux.
Linux is Tier 1 though, and supporting only a sub set of web features there doesn't seem right. This is also completely our fault, not an inherent limitation of Linux. We should at least have a plan to get this fixed.
Untracking for 54, but it seems like there should be a bug tracking re-enabling this feature on linux?
uh, nevermind, I guess this is that bug.
Blocks: e10s-select
Keywords: regression
Summary: select popups on Linux lack sufficient contrast → [e10s] select popups on Linux lack sufficient contrast
Blocks: 1406865
(In reply to Julien Cristau [:jcristau] from comment #11)
> Untracking for 54, but it seems like there should be a bug tracking
> re-enabling this feature on linux?
(In reply to Julien Cristau [:jcristau] from comment #12)
> uh, nevermind, I guess this is that bug.

No -- IIUC, this bug tracks the contrast issues *that happen when the feature is enabled*. (the issues that need to be fixed before the feature can be re-enabled).

IMO we should leave this bug focusing on the same contrast issues that it was initially filed for, and we can track the pref-flip re-enabling in a separate bug. (I'll co-opt bug 1406865 for that, which was filed recently on the fact that the feature is disabled.)
Summary: [e10s] select popups on Linux lack sufficient contrast → [e10s] select popups on Linux lack sufficient contrast (when dom.forms.select.customstyling is enabled)
Blocks: e10s-select-styling
No longer blocks: e10s-select
Duplicate of this bug: 1421898
Is this on the roadmap at all?  Per comment 10, it seems like we should aim to get the issues here fixed, so we can reenable custom styling on Linux (bug 1406865).  Right now, it's effectively a linux-only regression from e10s.

(FWIW: I just retested this to be sure it's still an issue, and it is.  Specifically, if I enable the customstyling pref, then bugzilla's attachment dropdown-menus still render with barely-visible text, as shown in attachment 8836934 [details].)

Comment 16

5 months ago
What's the status with this bug? Having disabled dom.forms.select.customstyling is pretty annoying. On the other hand, having it enabled, Firefox lacks some styling. It's one of the bugs that are really annoying and version after version they are not fixed. I will attach screenshots.

Comment 17

5 months ago
Posted image Ff-dropdown.png
This one shows how Firefox renders the drop down with dom.forms.select.customstyling enabled.

Comment 18

5 months ago
Posted image Chr-dropdown.png
This one shows how a Chromium based browser (Vivaldi in this case) renders the drop down, which is correct.

Comment 19

5 months ago
This one shows how Firefox renders the drop down with dom.forms.select.customstyling disabled. Extremely ugly (no colors, ugly font, no font sizes or style).
You need to log in before you can comment on or make changes to this bug.