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

Categories

(Core :: SVG, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: birtles, Assigned: birtles)

References

Details

Attachments

(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;

https://reviewboard.mozilla.org/r/186292/#review191616

::: 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 bbirtles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d8974272e3e3
Convert empty values to suitable zero values even when using discrete interpolation; r=hiro
https://hg.mozilla.org/mozilla-central/rev/d8974272e3e3
Status: ASSIGNED → RESOLVED
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.