1352963 - [e10s] Background on <select> options is white when opened for the first time

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[blinky]

Steps to reproduce:

1. Go to https://onedrive.live.com/download?cid=F96BA52A2AF70D03&resid=F96BA52A2AF70D03%21841&authkey=ACQf2Dqa11rX3lk.
2. Open the <select> element.


Actual results:

The background is white when opened for the first time.

The background is gray when opened for the second time.

Screenshot https://1drv.ms/i/s!AgMN9yoqpWv5hkqqY2J_e_M-hpgo

Comment 1

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

This test case reproduces it:

data:text/html,<style>* { transition: all 2s; } select:focus { background: orange; }</style><select><option>1<option>2

Comment 2

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

Attachment: Bug 1352963 - Update the styling of the select popup after transitionend.

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

Comment 3

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

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #1)
> This test case reproduces it:
> 
> data:text/html,<style>* { transition: all 2s; } select:focus { background:
> orange; }</style><select><option>1<option>2

Note that this test case uncovered another bug unrelated to this bug. I just filed bug 1353222 for the issue.

The following test case should be used to confirm/verify this specific bug:
data:text/html,<style>select { transition: all .2s; } select:focus { background: orange; }</style><select><option>1<option>2

Comment 5

[Dave Townsend [:mossop]]

Comment on attachment 8854270 [details]
Bug 1352963 - Update the styling of the select popup after transitionend.

https://reviewboard.mozilla.org/r/126198/#review128910

Looks ok but if I understand correctly we aren't going to see the transition on the select drop-down. Does Chrome support that?

Comment 6

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

(In reply to Dave Townsend [:mossop] from comment #5)
> Comment on attachment 8854270 [details]
> Bug 1352963 - Update the styling of the select popup after transitionend.
> 
> https://reviewboard.mozilla.org/r/126198/#review128910
> 
> Looks ok but if I understand correctly we aren't going to see the transition
> on the select drop-down. Does Chrome support that?

Yeah, both Chrome and Edge show the transition actually happening on the popup contents, whereas we would only update to the final state once the transition ends. This is really unfortunate, but from the front-end perspective I'm not aware of a solution to this problem. I think it underlines a major problem with how the e10s implementation of the select popup works.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf051de0d37b
Update the styling of the select popup after transitionend. r=mossop

Comment 8

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

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #6)
> (In reply to Dave Townsend [:mossop] from comment #5)
> > Comment on attachment 8854270 [details]
> > Bug 1352963 - Update the styling of the select popup after transitionend.
> > 
> > https://reviewboard.mozilla.org/r/126198/#review128910
> > 
> > Looks ok but if I understand correctly we aren't going to see the transition
> > on the select drop-down. Does Chrome support that?
> 
> Yeah, both Chrome and Edge show the transition actually happening on the
> popup contents, whereas we would only update to the final state once the
> transition ends. This is really unfortunate, but from the front-end
> perspective I'm not aware of a solution to this problem. I think it
> underlines a major problem with how the e10s implementation of the select
> popup works.

Filed bug 1353426 for this.

Comment 9

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

[Tracking Requested - why for this release]: regression from bug 910022

Comment 10

[Carsten Book [:Tomcat]]

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

Comment 11

[Gerry Chang [:gchang]]

Track 54+ as regression.

Hi :jaws,
Do you want to uplift this to Aurora54 if this patch is not too risky?

Comment 12

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

We will need to wait for bug 1355078 to get fixed first (it is waiting for review right now).

Comment 13

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

Comment on attachment 8854270 [details]
Bug 1352963 - Update the styling of the select popup after transitionend.

Approval Request Comment
[Feature/Bug causing the regression]: regression from bug 910022
[User impact if declined]: some <select> popups may be unreadable due to dependence on CSS transitions
[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, this patch should be applied before the patches from bug 1354196 and bug 1355078, in that order.
[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]: none

Comment 14

[Gerry Chang [:gchang]]

Comment on attachment 8854270 [details]
Bug 1352963 - Update the styling of the select popup after transitionend.

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

Comment 15

[Carsten Book [:Tomcat]]

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

Comment 16

[Andrei Vaida [:avaida]]

(In reply to (Away 3 May - 5 May) Jared Wein [:jaws] (please needinfo? me) from comment #13)
> [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

This fix seems to have automated coverage. Gerry, Jared, do you think manual testing would be of value here?

Comment 17

[Gerry Chang [:gchang]]

Hi Andrei,
You're right. There is no need for manual testing here. Thanks.

Comment 18

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

Yes, no need for manual testing here.