1349701 - [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%21685&authkey=AP0Dc0bV1PUZRe0.
2. Open the <select> element.


Actual results:

The background is white when opened for the first time.

The background appears when opened for the second time.

Screenshot https://1drv.ms/i/s!AgMN9yoqpWv5hS6WCfr5dv1aiOMx

Comment 1

[blinky]

I think this is a similar case http://jsfiddle.net/luiscastillo/sFx7f/.

Comment 2

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

Thanks, this URL reproduces the issue:
data:text/html,<style>select:focus { background: orange; }</style><select><option>1<option>2

Comment 3

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

Attachment: Bug 1349701 - Update the styles of <select> popups on focus.

This patch also fixes a bug in our UpdateDropDown code where we weren't computing updated styles for <select> element, as well as another bug where we weren't passing the correct number of arguments to this.populate.

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

Comment 4

[Dave Townsend [:mossop]]

Comment on attachment 8850608 [details]
Bug 1349701 - Update the styles of <select> popups on focus.

https://reviewboard.mozilla.org/r/123146/#review125528

Do we see any flickering with this approach? Is there a way to just get the computed style of the elements assuming they will be focused since I don't think there is a way to open the select without focusing it.

Comment 5

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

Comment on attachment 8850608 [details]
Bug 1349701 - Update the styles of <select> popups on focus.

https://reviewboard.mozilla.org/r/123146/#review125528

I don't see any flickering. Yes, we could lock the styles to :focus similar to what DevTools does. We're actually using that API right now for disabling the :checked style on <option> elements. I will see what that change looks like.

Comment 6

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

Comment on attachment 8850608 [details]
Bug 1349701 - Update the styles of <select> popups on focus.

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

Comment 7

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

Comment on attachment 8850608 [details]
Bug 1349701 - Update the styles of <select> popups on focus.

https://reviewboard.mozilla.org/r/123146/#review125550

::: toolkit/modules/SelectContentHelper.jsm:129
(Diff revisions 1 - 2)
>    },
>  
>    _update() {
>      // The <select> was updated while the dropdown was open.
>      // Let's send up a new list of options.
> +    DOMUtils.addPseudoClassLock(this.element, ":focus");

Technically we might not need to set this pseudo-class during _update since the element should organically have :focus. Though I put it here for belt-and-suspenders.

Comment 8

[Dave Townsend [:mossop]]

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #7)
> Comment on attachment 8850608 [details]
> Bug 1349701 - Update the styles of <select> popups on focus.
> 
> https://reviewboard.mozilla.org/r/123146/#review125550
> 
> ::: toolkit/modules/SelectContentHelper.jsm:129
> (Diff revisions 1 - 2)
> >    },
> >  
> >    _update() {
> >      // The <select> was updated while the dropdown was open.
> >      // Let's send up a new list of options.
> > +    DOMUtils.addPseudoClassLock(this.element, ":focus");
> 
> Technically we might not need to set this pseudo-class during _update since
> the element should organically have :focus. Though I put it here for
> belt-and-suspenders.

Can you add this as a comment in the code

Comment 9

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

Comment on attachment 8850608 [details]
Bug 1349701 - Update the styles of <select> popups on focus.

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

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4a98ffe94e12
Update the styles of <select> popups on focus. r=mossop

Comment 11

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

https://hg.mozilla.org/mozilla-central/rev/4a98ffe94e12

Comment 12

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

Comment on attachment 8850608 [details]
Bug 1349701 - Update the styles of <select> popups on focus.

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]: this patch must be applied on top of the one from bug 1348617
[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 13

[Gerry Chang [:gchang]]

Comment on attachment 8850608 [details]
Bug 1349701 - Update the styles of <select> popups on focus.

Fix a regression related to <select> element and was verified in comment #13 of bug 1348617. Aurora54+.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/94cfa1755988