1354196 - [e10s] The text does not show in <select> options

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[blinky]

Steps to reproduce:

1. Download and extracts the archive https://onedrive.live.com/download?cid=F96BA52A2AF70D03&resid=F96BA52A2AF70D03%211221&authkey=AITkrKeTDOkSYTM.
2. Open index.htm
3. Open the <select> element.


Actual results:

The text does not show in <select> options.

Screenshot https://1drv.ms/i/s!AgMN9yoqpWv5iUY-ejyVFOQIbJaC

Comment 1

[blinky]

Regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d66ec372454c676e487da5ba47167a79dcfea001&tochange=d02ab1fb2cbc43b425c2858d3b70f9ade88cfd65

Regressed by: bug 1335483

Comment 2

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

This data URI reproduces the bug:
data:text/html,<style>select { color: rgba(0, 0, 0, 0); text-shadow: 0 0 0 black; }</style><select><option>test<option>test<option>test<option>test

The problem here is that we display the text as transparent but we don't display the text-shadow. This data URI also has the same issue on Chrome.

The testcase attached from a real site only applies this style for Firefox (using `@-moz-document url-prefix()`). On Chrome they just use a set `color`.

We can't simply remove the alpha component of the color, and effectively treat rgba(0,0,0,0) as rgb(0,0,0). The site will not be expecting black instead of transparent and the black text may be unreadable too.

One solution here would be to forward the text-shadow property, though I'm wary of the ever-expanding set of CSS properties we will need to support.

The other solution would be to let the site find that it no longer works and then they can update their site to remove the special Firefox styling. Though there could be a long tail of sites that will be broken and either won't update timely or will never update.

Comment 3

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

Attachment: Bug 1354196 - Forward the text-shadow CSS property to the select popup for styling.

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

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8856679 [details]
Bug 1354196 - Forward the text-shadow CSS property to the select popup for styling.

https://reviewboard.mozilla.org/r/128620/#review131654

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7545a6eab53b
Forward the text-shadow CSS property to the select popup for styling. r=mossop

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7545a6eab53b

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Please request Aurora approval on this when you get a chance.

Comment 8

[Peter Chang[:pchang]]

Please request beta approval because FF 54 is beta now.

Comment 9

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

Comment on attachment 8856679 [details]
Bug 1354196 - Forward the text-shadow CSS property to the select popup for styling.

Approval Request Comment
[Feature/Bug causing the regression]: regression from bug 910022
[User impact if declined]: some <select> popups on websites may be unreadable
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no, automated test covers this
[List of other uplifts needed for the feature/fix]: yes, bug 1355078 should also be included in uplift
[Is the change risky?]: not expected
[Why is the change risky/not risky?]: The change is focused specifically on select dropdowns and is a very specific/focused fix.
[String changes made/needed]:

Comment 10

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

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #9)
> [String changes made/needed]: none

Comment 11

[Gerry Chang [:gchang]]

Comment on attachment 8856679 [details]
Bug 1354196 - Forward the text-shadow CSS property to the select popup for styling.

Fix a select dropdown issue and include test. Beta54+. Should be in 54 beta 3.

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a50aaf501b39

Comment 13

[Andrei Vaida [:avaida]]

(In reply to (Away 3 May - 5 May) Jared Wein [:jaws] (please needinfo? me) from comment #9)
> [Is this code covered by automated tests?]: yes
> [Has the fix been verified in Nightly?]: no
> [Needs manual test from QE? If yes, steps to reproduce]: no, automated test
> covers this

Setting qe-verify- based on Jared's assessment on manual testing needs and the fact that this fix has automated coverage.