[Webkit-unassigned] [Bug 61368] SVGAnimation should use direct unit animation for SVGLength

[bugzilla-daemon at webkit.org]

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





--- Comment #2 from Nikolas Zimmermann <zimmermann at kde.org>  2011-05-24 11:02:14 PST ---
(From update of attachment 94624)
View in context: https://bugs.webkit.org/attachment.cgi?id=94624&action=review

Looks good to me.

> Source/WebCore/svg/SVGAnimateElement.cpp:186
> +    case AnimatedNumberList:
> +    case AnimatedLengthList:

Why this reorering? You should keep it in alphabetical order, no?

> Source/WebCore/svg/SVGAnimateElement.cpp:545
> +        m_animatedType->length() = new SVGLength(lengthModeForAnimatedLengthAttribute(attributeName()), baseString);

This looks weird...

> Source/WebCore/svg/SVGAnimatedLengthAnimator.h:42
> +        type->length() = new SVGLength(m_lengthMode, string);

This looks weird...

> Source/WebCore/svg/SVGAnimatedType.h:39
> +    SVGLength*& length()

Why don't you return SVGLength& here? aka. return *m_data.length;

> Source/WebCore/svg/SVGAnimationElement.cpp:163
> +        for (unsigned i = 0; i < m_values.size(); ++i)
> +            m_values[i] = m_values[i].stripWhiteSpace();

Looks unrelated.

> Source/WebCore/svg/SVGAnimator.h:34
> +        return adoptPtr(new SVGAnimatedLengthAnimator(contextElement));

you should still add the switch here, and only implement case AnimatedLength/default: return adoptPtr(new SVGAnimatedLengthAnimator...

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