1350670 - [e10s] The text color on select options is gray 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%21734&authkey=AKphOnh2lDX94OA.
2. Open the <select> element.


Actual results:

The text color on select options is gray when opened for the first time.

Screenshot https://1drv.ms/i/s!AgMN9yoqpWv5hV-nd1r89rDJvoo3

Comment 1

[blinky]

This is a similar case http://jsfiddle.net/luiscastillo/sFx7f/.

Comment 2

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

Attachment: Bug 1350670 - Extend the MutationObserver to cover attributes in case the page sets styles through style, class, ID or other attributes.

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

Comment 3

[Dave Townsend [:mossop]]

Comment on attachment 8852198 [details]
Bug 1350670 - Extend the MutationObserver to cover attributes in case the page sets styles through style, class, ID or other attributes.

https://reviewboard.mozilla.org/r/124434/#review126978

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/adf939cb9b0b
Extend the MutationObserver to cover attributes in case the page sets styles through style, class, ID or other attributes. r=mossop

Comment 5

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/autoland/rev/7be483e06caf for https://treeherder.mozilla.org/logviewer.html#?job_id=87139159&repo=autoland on Mac and Windows.

Comment 6

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

The test was only failing on non-e10s builds. Odd that it only failed on non-e10s, but the cause is that the binding for the elements was destroyed during _update, so `item.label` was returning undefined on non-e10s builds. The reference to the items was retrieved before the styling change, so it was pointing at elements that were out-of-date. The fix for this was to move the waitForCondition up a few lines higher so that the references to the children aren't obtained until after the _update.

Comment 7

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

Comment on attachment 8852198 [details]
Bug 1350670 - Extend the MutationObserver to cover attributes in case the page sets styles through style, class, ID or other attributes.

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

Comment 8

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

https://hg.mozilla.org/integration/autoland/rev/a76a9944a35711026010baf2bc54780552a143e3

Comment 9

[blinky]

The case in the comment 0 is fixed in bug 1350774, Please add the test for this case.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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

Comment 11

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

(In reply to blinky from comment #9)
> The case in the comment 0 is fixed in bug 1350774, Please add the test for
> this case.

It seems you're asking for a test case that confirms that `color` that is set via select:focus is inherited by the option children? It is correct that there is not a specific test case for this scenario, but the test that was included in the patch for bug 1350774 covers one point of view of this problem. Unless there is another user-visible issue regarding this I'd rather spend my time elsewhere.

Comment 12

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

Comment on attachment 8852198 [details]
Bug 1350670 - Extend the MutationObserver to cover attributes in case the page sets styles through style, class, ID or other attributes.

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 patch from bug 1350774
[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

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

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

Sorry, scratch the above. This just landed on Nightly yesterday, though after letting it sit on Nightly for a few days we should be able to uplift it.

Comment 14

[Gerry Chang [:gchang]]

Comment on attachment 8852198 [details]
Bug 1350670 - Extend the MutationObserver to cover attributes in case the page sets styles through style, class, ID or other attributes.

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

Comment 15

[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 test case from comment 0 and comment 1 and the issue is no longer reproducible on latest Nightly (55.0a1) build. 
However, I was able to reproduce the issue on an older Nightly (2017-03-26) build.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

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