Closed Bug 1437272 Opened 6 years ago Closed 6 years ago

regression in behavior of CSS transitions (flicker / flash)

Categories

(Core :: DOM: Animation, defect)

Product:
Core
Component:
DOM: Animation
Version:
58 Branch
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla60
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: daniel, Assigned: hiro)

References

Details

Attachments

(5 files)

36060708-1b8b5c6a-0e4f-11e8-9b8d-d28766d01f73.png
89.21 KB, image/png
Details
Flashing when scrolling on Pontoon
6.67 MB, image/gif
Details
Bug 1437272 - Split nsTimingFunction into an independent header file.
59 bytes, text/x-review-board-request
birtles
: review+
Details
Bug 1437272 - Drop AnimationEffectReadOnly::IsActiveDurationZero().
59 bytes, text/x-review-board-request
birtles
: review+
Details
Bug 1437272 - Don't throttle finite transform animations in out-of-view element.
59 bytes, text/x-review-board-request
birtles
: review+
Details 
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 Safari/537.36

Steps to reproduce:

(copied from my colleague from https://github.com/zurb/foundation-sites/issues/10924)

Open the Orbit Docs of Foudation Sites in Firefox 58
Scroll to the example
Click the Arrows, see the "white flash"


Actual results:

There is a white flicker / flash between the animations. Its also happening with the HTML Content Examples. Its not a Image only problem.


Expected results:

There should be a seamless animation between the images.
Test case link:

https://foundation.zurb.com/sites/docs/orbit.html

Additional Information

Firefox started breaking the Orbit Animation, when they released Version 58

It was working fine in Firefox Version 57

Someone already made a Video about the Bug https://www.youtube.com/watch?v=p-xjjJAWEXw

I noticed, that when you click to the "left" direction, the Contents get properly displayed

It seems that fading with motionUI works just as expected, but sliding to the left causes problems

When you check out the demo pen (https://codepen.io/IamManchanda/pen/GmGzWY?editors=1100) it seems to work, but when you start to play around with the bullet points, its also happening
Component: Untriaged → Layout
Product: Firefox → Core
Flags: needinfo?(hikezoe)
Copying info fro the GitHub issue

I bisected the builds and the following change is the cause

2018-02-10T10:42:11: DEBUG : Using url: https://hg.mozilla.org/integration/autoland/json-pushes?changeset=fa94f7205173d34f23975c6af9cb95237b28c8b8&full=1
2018-02-10T10:42:12: DEBUG : Found commit message:
Bug 1190721 - Throttle animations that produce any transform change hint if the target element is out-of-view. r=birtles

And unthrottle them on every 200ms just like we do for transform animations on
the compositor.  To unthrottle the transform animations properly, we need to
update UpdateLastTransformSyncTime each time we compose the style for the
animations instead of updating it when we send the transform animation to the
compositor.  That's because display item for transform is built even while we
are throttling the transform animations for some reasons, so if we updated the
last transform sync time there, the time will not match what it should be.

MozReview-Commit-ID: GwMzJqUlzd2
https://hg.mozilla.org/mozilla-central/rev/fa94f7205173d34f23975c6af9cb95237b28c8b8

Related https://bugzilla.mozilla.org/show_bug.cgi?id=1424506
This issue has been mitigated by bug 1425213.  That's said, there can been seen still flickers.

If we do throttle scrolled-out transform animations only if the animation active duration is infinite, then the issue will go away.  But if we are using finite duration for throbbers on tabs, then there will be another regression on Firefox itself.
(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)
> It's finite;
> https://hg.mozilla.org/mozilla-central/file/80ca8e45becb/browser/themes/
> shared/tabs.inc.css#l191
> 
> Hope this will not cause regression on sites in the wild;
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=be131da4ff0d4818ba65452c8fd8686669250d8b

To be clear, this change certainly regresses on some situations, but I hope people will not be aware of it.
Other solutions I can think of;

- Don't throttle transform *transition* on out-of-view element
 I believe those type of slider animations use transition
- Add some amount of threshold to nsIFrame::IsScrolledOutOfView() to be able to know transform animation will be in visible region sonn.
 This way will not work well for step function cases or fast moving transform
(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)
> It's finite;
> https://hg.mozilla.org/mozilla-central/file/80ca8e45becb/browser/themes/
> shared/tabs.inc.css#l191

It's *infinite* actually. :)
Which binary can we use for testing at the moment? I'm not sure if a patch is already available.

How can we help to solve this quicker and make sure it lands in release 60?
You can get it through the links in comment 8, depending on what platform you are using.
Component: Layout → DOM: Animation
Not throttling out-of-view finite animations makes sense to me. I was wondering about this this morning actually. :)
Coincidence. :)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #8)
> Could someone try to use the binaries in comment 6?  I believe the
> regression has been solved in these binaries.
> 
> For Linux 64.
> https://queue.taskcluster.net/v1/task/FM_G0_TdTx-aKW7OBxaQ4w/runs/0/
> artifacts/public/build/target.tar.bz2
> For MacOSX.
> https://queue.taskcluster.net/v1/task/EBS3p-52SUWONDNl5BvlBw/runs/0/
> artifacts/public/build/target.dmg
> For Windows 64.
> https://queue.taskcluster.net/v1/task/X9xdeHBUQA6hZIC-rmHrjA/runs/0/
> artifacts/public/build/target.zip

This build does not have the regression anymore and looks good to me.
I can see the same issue when scrolling in Pontoon, on latest Nightly. But that actually surfaced only a few days ago, are both issue from the same origin?
See attached.
(In reply to Itiel from comment #16)
> I can see the same issue when scrolling in Pontoon, on latest Nightly. But
> that actually surfaced only a few days ago, are both issue from the same
> origin?
> See attached.

You can easily verify this with mozregression https://github.com/mozilla/mozregression/releases
(In reply to daniel from comment #18)
> You can easily verify this with mozregression
> https://github.com/mozilla/mozregression/releases

Thanks, it appears mine is another issue probably caused by bug 1436189. Will file a new bug.
Assignee: nobody → hikezoe
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8956702 [details]
Bug 1437272 - Drop AnimationEffectReadOnly::IsActiveDurationZero().

https://reviewboard.mozilla.org/r/225686/#review231916
Attachment #8956702 - Flags: review?(bbirtles) → review+
Comment on attachment 8956701 [details]
Bug 1437272 - Split nsTimingFunction into an independent header file.

https://reviewboard.mozilla.org/r/225684/#review231918

::: layout/style/nsTimingFunction.h:14
(Diff revision 2)
> +
> +#include "nsStyleConsts.h"
> +
> +struct nsTimingFunction
> +{
> +

(Nit: Perhaps we could drop this blank line while we're moving this.)
Attachment #8956701 - Flags: review?(bbirtles) → review+
Comment on attachment 8956703 [details]
Bug 1437272 - Don't throttle finite transform animations in out-of-view element.

https://reviewboard.mozilla.org/r/225688/#review231920

::: dom/animation/AnimationEffectReadOnly.h:54
(Diff revision 2)
>  
>    nsISupports* GetParentObject() const { return mDocument; }
>  
>    bool IsCurrent() const;
>    bool IsInEffect() const;
> +  bool IsFiniteActiveDuration() const

HasFiniteActiveDuration

::: dom/animation/KeyframeEffectReadOnly.cpp:1476
(Diff revision 2)
> +        // Don't throttle finite transform animation since transform animation
> +        // might move in visible area from the out-of-view area, so that
> +        // the transform animation might appear in late if it was throttled.

"Don't throttle finite transform animations since the animation might suddenly come into view and if it was throttled it will be out-of-sync."

?

::: dom/animation/test/mozilla/file_restyles.html:593
(Diff revision 2)
> +                                // This animation will move a bit but
> +                                // still out-of-view.

s/but still/but will remain/
Attachment #8956703 - Flags: review?(bbirtles) → review+
Thanks!

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

> ::: dom/animation/KeyframeEffectReadOnly.cpp:1476
> (Diff revision 2)
> > +        // Don't throttle finite transform animation since transform animation
> > +        // might move in visible area from the out-of-view area, so that
> > +        // the transform animation might appear in late if it was throttled.
> 
> "Don't throttle finite transform animations since the animation might
> suddenly come into view and if it was throttled it will be out-of-sync."

'out-of-sync' is intuitive what happens. :)
Pushed by hikezoe@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c7dc17e67663
Split nsTimingFunction into an independent header file. r=birtles
https://hg.mozilla.org/integration/autoland/rev/df6e7f27a1ab
Drop AnimationEffectReadOnly::IsActiveDurationZero(). r=birtles
https://hg.mozilla.org/integration/autoland/rev/c780ba32cec9
Don't throttle finite transform animations in out-of-view element. r=birtles
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: