Closed Bug 1518802 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Layout, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox-esr60 --- unaffected
firefox64 --- unaffected
firefox65 --- unaffected
firefox66 --- disabled
firefox67 --- disabled
firefox68 --- fixed

People

(Reporter: tbabos, Assigned: hiro)

References

Details

(Keywords: regression)

Attachments

(4 files, 1 obsolete file)

[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?

Flags: needinfo?(hikezoe)

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

Blocks: 1504065
Summary: [Vimeo} The highlights for the options in the drop-down menus are misplaced and have incorrect color/opacity → [Vimeo] The highlights for the options in the drop-down menus are misplaced and have incorrect color/opacity
Duplicate of this bug: 1513522

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

Flags: needinfo?(hikezoe)

(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.

Sean could you triage this one?

Flags: needinfo?(svoisen)

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

Flags: needinfo?(svoisen)
Priority: -- → P3
Duplicate of this bug: 1525203

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).

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

Assignee: nobody → hikezoe
Status: NEW → ASSIGNED

It causes flicker in some cases.

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

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.

Blocks: 1535532
No longer blocks: 1504065

(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.

Flags: needinfo?(hikezoe)

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.

Attachment #9044791 - Attachment is obsolete: true
Attached patch A possible fixSplinter Review

(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?

Flags: needinfo?(hikezoe)
Attachment #9053106 - Flags: feedback?(matt.woodrow)
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.
Attachment #9053106 - Flags: feedback?(matt.woodrow) → feedback+

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.

Pushed by hikezoe@mozilla.com:
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
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

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

Flags: needinfo?(hikezoe)

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

Flags: needinfo?(hikezoe)
You need to log in before you can comment on or make changes to this bug.