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 UpdateStartValueFromReplacedTransition

Categories

(Core :: DOM: Animation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: hiro, Unassigned)

References

Details

Attachments

(1 file)

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.
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[0].mProperty,
> +                                            startValue,
> +                                            cssValue);
> +      MOZ_ASSERT(uncomputeResult, "UncomputeValue should not fail");
> +      mKeyframes[0].mPropertyValues[0].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 hiikezoe@mozilla-japan.org:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f58943bca186
Update the first keyframe value as well as property value when replacing transition. r=birtles
https://hg.mozilla.org/mozilla-central/rev/f58943bca186
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.