Closed Bug 1404803 Opened 2 years ago Closed 2 years ago

When using discrete animation with an "empty" nsSMILCSSValueType value the zero-value fixup never happens


(Core :: SVG, defect, P3)




Tracking Status
firefox57 --- wontfix
firefox58 --- fixed


(Reporter: birtles, Assigned: birtles)




(3 files, 1 obsolete file)

Attached patch Mochitest (obsolete) — Splinter Review
Attached patch MochitestSplinter Review
(Accidentally left the begin="indefinite" in from another test)
Attachment #8914211 - Attachment is obsolete: true
Attached patch PatchSplinter Review
This patch fixes the bug but it's awefully complicated for something that, as far as I can tell, only ever happens with a discrete non-additive SMIL by-animation. That also makes it really hard to test the code here so I'm a bit uncomfortable about landing something as complicated as this given how unlikely it is to be exercised.

Perhaps I can find other ways to test/simplify it.
Priority: -- → P3
I'm not sure if this patch is worth it honestly. It fixes the behavior but it introduces a fair bit of code for a very rare case.
Comment on attachment 8915023 [details]
Bug 1404803 - Convert empty values to suitable zero values even when using discrete interpolation;

::: dom/smil/nsSMILAnimationFunction.cpp:491
(Diff revision 1)
> +        // We have currently only ever encountered this case for the first
> +        // value of a by-animation (which has two values) and since we have no
> +        // way of testing other cases we just skip them (but assert if we
> +        // ever do encounter them so that we can add code to handle them).
> +        if (index + 1 >= aValues.Length()) {
> +          MOZ_ASSERT(static_cast<void*>(aResult.mU.mPtr),

Is this static_cast necessary?
Attachment #8915023 - Flags: review?(hikezoe) → review+
Pushed by
Convert empty values to suitable zero values even when using discrete interpolation; r=hiro
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.