Closed Bug 1278430 Opened 4 years ago Closed 4 years ago
The first keyframe value should be update when transition start value is replaced by Update
Start Value From Replaced Transition
58 bytes, text/x-review-board-request
No description provided.
Just for my own reference, I think the issue here is that we're updating the keyframe effect's *properties* but those properties can be overwritten from the *keyframes* any time the style context changes. It's possible that we're ok and that the checks for redundant changes mean this never happens for transitions, but I'm not sure. It's probably safer to update the keyframe values.
Review commit: https://reviewboard.mozilla.org/r/58116/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/58116/
Attachment #8760536 - Flags: review?(bbirtles)
Based upon the patch for bug 1275718.
Comment on attachment 8760536 [details] Bug 1278430 - Update the first keyframe value as well as property value when replacing transition. https://reviewboard.mozilla.org/r/58116/#review55006 r=me with comments addressed ::: layout/style/nsTransitionManager.cpp:114 (Diff revision 1) > + DebugOnly<bool> uncomputeResult = > + StyleAnimationValue::UncomputeValue(mProperties.mProperty, > + startValue, > + cssValue); > + MOZ_ASSERT(uncomputeResult, "UncomputeValue should not fail"); > + mKeyframes.mPropertyValues.mValue = cssValue; I guess we should add an assertion that mKeyframes.Length() == 2 etc. e.g. https://dxr.mozilla.org/mozilla-central/rev/1828937da9493b2cd54862b9c520b2ba5c7db92b/layout/style/nsTransitionManager.h#55 I was thinking we could just set the keyframe value then call UpdateProperties() but that seems to do a lot of unnecessary work and we probably want to avoid the call to RequestRestyle in particular. So maybe this is fine. ::: layout/style/test/file_transitions_replacement_on_busy_frame.html:84 (Diff revision 1) > + "We surely update keyframe value of the transition after the busy " + > + "in rAF callback"); We should adjust this message similar to the suggestion in bug 1275718 comment 23
Attachment #8760536 - Flags: review?(bbirtles) → review+
Comment on attachment 8760536 [details] Bug 1278430 - Update the first keyframe value as well as property value when replacing transition. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58116/diff/1-2/
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/f58943bca186 Update the first keyframe value as well as property value when replacing transition. r=birtles
You need to log in before you can comment on or make changes to this bug.