1348617 - [e10s] Background on <select> options is white

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[blinky]

Steps to reproduce:

1. Go to http://jsfiddle.net/mynameisdonald/dXdq6/.
2. Open the <select> element.


Actual results:

The background is white.

Comment 1

[Loic]

[Tracking Requested - why for this release]:

https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=aa279848f2133b82f83649478ee0a57f69e7d263&tochange=2801d6d63979781b6a807de78cf65a1ac2502dcb

Comment 2

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

So it looks like what Chrome is doing is setting a white background on the popup, then applying the translucent color provided by content on top of the white background. Since 1347089 we're dropping the alpha channel, which is why we're getting solid black in the attached testcase.

Comment 3

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

Attachment: Bug 1348617 - Use the alpha channel of custom styled select backgrounds by applying the requested color on top of the system's background.

This matches parity with Google Chrome Canary Version 59.0.3046.0 (Official Build) canary (64-bit).

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

Comment 4

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

Comment on attachment 8849562 [details]
Bug 1348617 - Use the alpha channel of custom styled select backgrounds by applying the requested color on top of the system's background.

https://reviewboard.mozilla.org/r/122344/#review124450

::: toolkit/modules/SelectContentHelper.jsm:145
(Diff revision 1)
>    // This is used to skip applying the custom color if it matches
>    // the user agent values.
>    _calculateUAColors() {
>      let dummyOption = this.element.ownerDocument.createElement("option");
>      dummyOption.style.setProperty("color", "-moz-comboboxtext", "important");
> -    dummyOption.style.setProperty("backgroundColor", "-moz-combobox", "important");
> +    dummyOption.style.setProperty("background-color", "-moz-combobox", "important");

Note that this line change and the one below are strictly errors since this API expects the hyphen-style of CSS property names, not hte camel-case style that we were using previously.

Comment 5

[Dave Townsend [:mossop]]

Comment on attachment 8849562 [details]
Bug 1348617 - Use the alpha channel of custom styled select backgrounds by applying the requested color on top of the system's background.

https://reviewboard.mozilla.org/r/122344/#review124568

::: browser/base/content/test/forms/browser_selectpopup_colors.js:166
(Diff revision 1)
> +    if (topCoat == "none") {
> +      is(`rgb(${bR}, ${bG}, ${bB})`, options.selectBgColor,
> +        "popup has expected background color");
> +    } else {
> +      let [/* unused */, tR, tG, tB, /* unused */, tA] =
> +        topCoat.match(/rgba?\((\d+), (\d+), (\d+)(, (0\.\d+))?\)/);

You can get rid of the second unused by using ?: at the start of the parenthesized block that you don't care about.

::: browser/base/content/test/forms/browser_selectpopup_colors.js:166
(Diff revision 1)
> +    if (topCoat == "none") {
> +      is(`rgb(${bR}, ${bG}, ${bB})`, options.selectBgColor,
> +        "popup has expected background color");
> +    } else {
> +      let [/* unused */, tR, tG, tB, /* unused */, tA] =
> +        topCoat.match(/rgba?\((\d+), (\d+), (\d+)(, (0\.\d+))?\)/);

Please make this regexp more fully match the string. You should be able to wrap the rgba in parenthesis and then use a backreference to verify it appears twice.

Comment 6

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

Comment on attachment 8849562 [details]
Bug 1348617 - Use the alpha channel of custom styled select backgrounds by applying the requested color on top of the system's background.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/122344/diff/1-2/

Comment 7

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e65e6b9e8849974c1ea5ee1b40c4203ebefc57dd

Comment 8

[Dave Townsend [:mossop]]

Comment on attachment 8849562 [details]
Bug 1348617 - Use the alpha channel of custom styled select backgrounds by applying the requested color on top of the system's background.

https://reviewboard.mozilla.org/r/122344/#review124594

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc61bbc304d7
Use the alpha channel of custom styled select backgrounds by applying the requested color on top of the system's background. r=mossop

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fc61bbc304d7

Comment 11

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

Comment on attachment 8849562 [details]
Bug 1348617 - Use the alpha channel of custom styled select backgrounds by applying the requested color on top of the system's background.

Approval Request Comment
[Feature/Bug causing the regression]: regression from bug 910022
[User impact if declined]: <select> menus may look broken on some webpages
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not "verified" but it has been on nightly for some time now with no bugs filed against it and the bug reporter has done an amazing job hunting for bugs with the feature
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: no
[Is the change risky?]: no
[Why is the change risky/not risky?]: limited to just select popups, this is following other patches that have been uplifted to for bug 910022
[String changes made/needed]: none

Comment 12

[Gerry Chang [:gchang]]

Hi Brindusa, could you help find someone to verify if this issue and bug 1350670, bug 1350774, bug 1350662, and 1349701 were fixed as expected on a latest Nightly build? Thanks!

Comment 13

[Cosmin Muntean [:cmuntean], Ecosystem QA]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
Firefox: 55.0a1, Build ID: 20170404030204

I have tested this issue using the provided STR from comment 0 using the latest Nightly (55.0a1, Build ID: 20170404030204) build and the issue is no longer reproducible. 
However, I was able to reproduce the issue with an older Nightly (2017-03-19) build. 

I have also tested the issues mentioned in comment 12, and are no longer reproducible on the latest Nightly (55.0a1, Build ID: 20170404030204) build. I will add a comment on each bug.

Comment 14

[Gerry Chang [:gchang]]

Comment on attachment 8849562 [details]
Bug 1348617 - Use the alpha channel of custom styled select backgrounds by applying the requested color on top of the system's background.

Fix a regression related to <select> menu and was verified. Aurora54+.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b305ba0b489b

Comment 16

[Ciprian Georgiu, Desktop QA]

I've managed to reproduce this issue using STR from comment 0, on an old Nightly.

This issue is verified fixed on 54 beta 2 (20170424215451) across platforms:
- Windows 10 x64
- Mac OS X 10.11.5
- Ubuntu 16.04 x64 LTS