1518802 - [Vimeo] The highlights for the options in the drop-down menus are misplaced and have incorrect color/opacity

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Timea Cernea [:tbabos][inactive]]

[Affected versions]
66.0a1 (2018-01-09)

[Affected platforms]
Windows 7/10 x64
Windows 8 x32
Mac OS 10.13

[Steps to reproduce]

1. Launch latest Firefox Nightly
2. Go to https://vimeo.com/join#_=_
3. Hover over "Features" or "Watch" to reveal the drop-down menus
4. Hover over slightly faster on all the options from the drop-down menus

[Expected result]
The options should be highlighted (with a barely visible white color) accordingly.

[Actual result]
The highlights for the options are misplaced and are displayed with different color or opacity (depending on the image from the background of the site).

[Regression-Range]
Last good revision: 2d8ce84e0107c99974201c1b67864786b22f3ff8
First bad revision: 94ecc40729d0302ee3953dbab0d280d4a6b174c4
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2d8ce84e0107c99974201c1b67864786b22f3ff8&tochange=94ecc40729d0302ee3953dbab0d280d4a6b174c4

Hiroyuki Ikezoe, can you please take a look at this?

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Thanks. That's interesting. I will check this when I have time.

Comment 3

[Hiroyuki Ikezoe (:hiro)]

So the problem here is that the animated background-color is rgba(0, 0, 0, 0) in the case where the animated value is equal to the static background-color value of the target element. To be honest I didn't know the fact at all. :/

I am not sure we need really to make the background-color be rgba(0, 0, 0, 0) case, but using the static background-color fixes the issue.

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

Comment 4

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #3)

So the problem here is that the animated background-color is rgba(0, 0, 0, 0) in the case where the animated value is equal to the static background-color value of the target element.

I was totally looking at the wrong case. :/ Actually the animated value is 'transparent', it's not related to the static background-color.

Comment 5

[Tim Spurway [:tspurway]]

Sean could you triage this one?

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Setting P3 for now since the feature regressed this is available only on nightly.

Comment 10

[Brian Birtles (:birtles)]

Might be worth bumping the priority of this (or disabling OMTA background-color temporarily). It appears to be confusing folks working on DevTools and Firefox FE occasionally (I just saw another person mentioning this in DevTools slack).

Comment 11

[Hiroyuki Ikezoe (:hiro)]

I will not have time for this for a while, I am going to disable it in nightly too.

Comment 12

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1518802 - Disable background-color animations run on the compositor on nightly. r=birtles

It causes flicker in some cases.

Comment 13

[Brian Birtles (:birtles)]

The bugs for this feature are a mess. As far as I can tell we have:

• Bug 1510030 - For Web Render only (although it would be good to make the bug title say that)
• Bug 1504065 - For non-Web Render and is marked as fixed although now we are disabling it

We should probably:

• File a meta bug for enabling background-color animations on non-WebRender
• File a separate bug for disabling it on Nightly and attach the patch to that bug
• Keep this bug open and make it block the meta bug
• Also make bug 1510120 and bug 1504065 block the new meta bug
• Add a "[WebRender]" prefix or somehow mention WebRender in the title of bug 1510030 so that we can tell it apart from bug 1504065

Comment 14

[Brian Birtles (:birtles)]

If you like, you could even have two separate bugs for enabling OMTA background-color on Nightly and on Release and make this bug block turning it back on in Nightly.

Comment 15

[Brian Birtles (:birtles)]

(In reply to Brian Birtles (:birtles) from comment #13)

The bugs for this feature are a mess. As far as I can tell we have:

• Bug 1510030 - For Web Render only (although it would be good to make the bug title say that)
• Bug 1504065 - For non-Web Render and is marked as fixed although now we are disabling it

We should probably:

• File a meta bug for enabling background-color animations on non-WebRender
• File a separate bug for disabling it on Nightly and attach the patch to that bug
• Keep this bug open and make it block the meta bug
• Also make bug 1510120 and bug 1504065 block the new meta bug
• Add a "[WebRender]" prefix or somehow mention WebRender in the title of bug 1510030 so that we can tell it apart from bug 1504065

I've gone ahead and made the the above changes.

Hiro, would you mind re-attaching this patch to bug 1535533 instead? Thanks.

Comment 16

[Hiroyuki Ikezoe (:hiro)]

Attachment: An exsily reproducible test case

Here is a test case that you can see the issue easily.

When opening this file, you will see a gray rectangle and soon after you will see the rectangle changes to black. That's the problem.

Comment 17

[Hiroyuki Ikezoe (:hiro)]

Attachment: A possible fix

(In reply to Brian Birtles (:birtles) from comment #15)

(In reply to Brian Birtles (:birtles) from comment #13)

The bugs for this feature are a mess. As far as I can tell we have:

• Bug 1510030 - For Web Render only (although it would be good to make the bug title say that)
• Bug 1504065 - For non-Web Render and is marked as fixed although now we are disabling it

We should probably:

• File a meta bug for enabling background-color animations on non-WebRender
• File a separate bug for disabling it on Nightly and attach the patch to that bug
• Keep this bug open and make it block the meta bug
• Also make bug 1510120 and bug 1504065 block the new meta bug
• Add a "[WebRender]" prefix or somehow mention WebRender in the title of bug 1510030 so that we can tell it apart from bug 1504065

I've gone ahead and made the the above changes.

Hiro, would you mind re-attaching this patch to bug 1535533 instead? Thanks.

Instead, I did a bit investigate what causes the black rectangle. That's because, as far as I can tell, nsDisplayBackgroundColor::GetOpaqueRegion returns the rectangle as an opaque region and the region is subtracted later.

:mattwoodrow, in the attaching patch we treat the background display item is not opaque if the display item has a background-color animation even if the alpha channel value is 1.0 at that moment. Does it sound the right thing to do? Is there anything we still need to care?

Comment 18

[Matt Woodrow (:mattwoodrow)]

Comment on attachment 9053106 [details] [diff] [review]
A possible fix

Review of attachment 9053106 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Hiroyuki Ikezoe (:hiro) from comment #17)
 
> :mattwoodrow, in the attaching patch we treat the background display item is not opaque if the display item has a background-color animation even if the alpha channel value is 1.0 at that moment.  Does it sound the right thing to do?  Is there anything we still need to care?

Yes that sounds correct, and should be sufficient.

Comment 19

[Hiroyuki Ikezoe (:hiro)]

Attachment: A reftest but doesn't fail without the fix unfortunately

Thank you, mattwoodrow!

I tried to write a reftest but it doesn't fail without the fix. I can actually see the black rectangle there just before the reftest harness takes a snapshot, it's gone on the snapshot, though I did fix some of issues that causes style flushes in the reftest harnes (bug 1441713, bug 1442063 and bug 1447874), it seems to there is(are) something invoking style flush.

Anyways, I will go without any test cases here.

Comment 20

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1518802 - Treat background-color animation as non-opaque even if the alpha channel is 1.0 at the moment. r?mattwoodrow

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8ead245d106b
Treat background-color animation as non-opaque even if the alpha channel is 1.0 at the moment. r=mattwoodrow

Comment 22

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/8ead245d106b

Comment 23

[Pascal Chevrel:pascalc]

Hiro, can you confirm that this is still a nightly-only feature and as such that it is disabled on 67? Thanks

Comment 24

[Hiroyuki Ikezoe (:hiro)]

Yes, this feature has been disabled on any betas and releases now.

Comment 25

[Pascal Chevrel:pascalc]

Thanks