[Webkit-unassigned] [Bug 12073] Implement setCurrentTime() and pauseAnimations() on SVGSVGElement

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=12073





--- Comment #18 from Eric Seidel <eric at webkit.org>  2011-11-21 15:53:25 PST ---
(From update of attachment 116137)
View in context: https://bugs.webkit.org/attachment.cgi?id=116137&action=review

> Source/WebCore/svg/SVGSVGElement.cpp:534
> +    if (seconds < 0)
> +        seconds = 0;

Can this and the isnan check just be rolled into a seconds = max(seconds, 0); line?

> Source/WebCore/svg/SVGSVGElement.cpp:536
> +    else if (isnan(seconds))
> +        return;

Should this throw an exception?  Do we have tests for hte behavior of element.setCurrentTime("foo")?  Seems we should.

> Source/WebCore/svg/animation/SMILTimeContainer.cpp:138
> +    Vector<SVGSMILElement*> toReset;
> +    copyToVector(m_scheduledAnimations, toReset);
> +    for (unsigned n = 0; n < toReset.size(); ++n)
> +        toReset[n]->reset();

Why do we need to copy them?  Can reset() cause arbitrary dom modifications?

> Source/WebCore/svg/animation/SMILTimeContainer.cpp:-284
> -        else if (!animation->isContributing(elapsed)) {
> -            m_scheduledAnimations.remove(animation);
> -            if (m_scheduledAnimations.isEmpty())
> -                m_savedBaseValues.clear();
> -        }

You should explain this (and other changes) in your ChangeLog.

> Source/WebCore/svg/animation/SVGSMILElement.cpp:182
> +void SVGSMILElement::reset()
> +{
> +    m_activeState = Inactive;
> +    m_isWaitingForFirstInterval = true;
> +    m_intervalBegin = SMILTime::unresolved();
> +    m_intervalEnd = SMILTime::unresolved();
> +    m_previousIntervalBegin = SMILTime::unresolved();
> +    m_lastPercent = 0;
> +    m_lastRepeat = 0;
> +    m_nextProgressTime = 0;
> +
> +    resolveFirstInterval();
> +}

We should consider using this in the constructor so that the constructor and reset() are known to take the same codepaths.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.