Closed
Bug 1437272
Opened 7 years ago
Closed 7 years ago
regression in behavior of CSS transitions (flicker / flash)
Categories
(Core :: DOM: Animation, defect)
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox58 | --- | wontfix |
firefox59 | --- | wontfix |
firefox60 | --- | fixed |
People
(Reporter: daniel, Assigned: hiro)
References
Details
Attachments
(5 files)
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
Comment 2•7 years ago
|
||
Blocking bug 1190721 per the regression range in https://github.com/zurb/foundation-sites/issues/10924.
Blocks: 1190721
Updated•7 years ago
|
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
Assignee | ||
Comment 5•7 years ago
|
||
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.
Assignee | ||
Comment 6•7 years ago
|
||
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
Flags: needinfo?(hikezoe)
Assignee | ||
Comment 7•7 years ago
|
||
(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.
Updated•7 years ago
|
status-firefox58:
--- → wontfix
status-firefox59:
--- → fix-optional
status-firefox60:
--- → affected
status-firefox-esr52:
--- → unaffected
Assignee | ||
Comment 8•7 years ago
|
||
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
Assignee | ||
Comment 9•7 years ago
|
||
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
Assignee | ||
Comment 10•7 years ago
|
||
(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. :)
Reporter | ||
Comment 11•7 years ago
|
||
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?
Assignee | ||
Comment 12•7 years ago
|
||
You can get it through the links in comment 8, depending on what platform you are using.
Assignee | ||
Updated•7 years ago
|
Component: Layout → DOM: Animation
Comment 13•7 years ago
|
||
Not throttling out-of-view finite animations makes sense to me. I was wondering about this this morning actually. :)
Assignee | ||
Comment 14•7 years ago
|
||
Coincidence. :)
Reporter | ||
Comment 15•7 years ago
|
||
(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.
Comment 16•7 years ago
|
||
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.
Comment 17•7 years ago
|
||
Reporter | ||
Comment 18•7 years ago
|
||
(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
Comment 19•7 years ago
|
||
(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 | ||
Comment 21•7 years ago
|
||
Rebased and modified a bit. https://treeherder.mozilla.org/#/jobs?repo=try&revision=ca9e33c75d75705b8bdf95349ddc79fa3f8e6223
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → hikezoe
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 29•7 years ago
|
||
mozreview-review |
Comment on attachment 8956702 [details]
Bug 1437272 - Drop AnimationEffectReadOnly::IsActiveDurationZero().
https://reviewboard.mozilla.org/r/225686/#review231916
Attachment #8956702 -
Flags: review?(bbirtles) → review+
Comment 30•7 years ago
|
||
mozreview-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 31•7 years ago
|
||
mozreview-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+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 35•7 years ago
|
||
A final try;
https://treeherder.mozilla.org/#/jobs?repo=try&revision=153a1cfbd77396a93fac38b994d581aaee9b3051
L10N failure seems to be an infra issue.
https://treeherder.mozilla.org/#/jobs?repo=try&fromchange=a10ccae09b0fe730840f5c282ae25ca357ccd4c9&selectedJob=166700159
Assignee | ||
Comment 36•7 years ago
|
||
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. :)
Comment 37•7 years ago
|
||
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
Comment 38•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c7dc17e67663
https://hg.mozilla.org/mozilla-central/rev/df6e7f27a1ab
https://hg.mozilla.org/mozilla-central/rev/c780ba32cec9
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Updated•6 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•