[Webkit-unassigned] [Bug 53442] SVGAnimateElement needs information about the animated attribute type

[bugzilla-daemon at webkit.org]

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


Nikolas Zimmermann <zimmermann at kde.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #80757|review?                     |review-
               Flag|                            |




--- Comment #5 from Nikolas Zimmermann <zimmermann at kde.org>  2011-02-01 07:29:53 PST ---
(From update of attachment 80757)
View in context: https://bugs.webkit.org/attachment.cgi?id=80757&action=review

Almost there, still r- though.

> Source/WebCore/svg/SVGAnimateElement.cpp:91
> +    AnimatedAttributeType type = targetElement()->animatedPropertyTypeForAttribute(QualifiedName(nullAtom, attribute, nullAtom));
>      // FIXME: We need a full property table for figuring this out reliably.
> -    if (hasTagName(SVGNames::animateColorTag))
> +    if (hasTagName(SVGNames::animateColorTag) || type == AnimatedColor)
>          return ColorProperty;

You don't need to ask the targetElement(), if hasTagNames(SVGNames::animateColorTag) is true. Make it a early exit shortcut.

> Source/WebCore/svg/SVGCircleElement.cpp:139
> +    AttributeToPropertyTypeMap& animatablePropertyMap = this->animatablePropertyMap();

s/animatablePropertyMap/attributeToPropertyTypeMap/ ?

> Source/WebCore/svg/SVGElement.h:140
> +    AttributeToPropertyTypeMap m_animatedAttributeMap;

Obsolete, please remove.

> Source/WebCore/svg/SVGElementRareData.h:60
> +    HashMap<QualifiedName, AnimatedAttributeType>& animatedAttributeMap() { return m_animatedAttributeMap; }

Use the typdef.

> Source/WebCore/svg/SVGElementRareData.h:73
> +    HashMap<QualifiedName, AnimatedAttributeType> m_animatedAttributeMap;

Ditto.

> Source/WebCore/svg/SVGGlyphElement.cpp:83
> +void SVGGlyphElement::svgAttributeChanged(const QualifiedName& attrName)
> +{
> +    if (attrName == SVGNames::dAttr)
> +        invalidateGlyphCache();
> +}

This seems unrelated.

> Source/WebCore/svg/SVGGlyphElement.h:120
> +    virtual void svgAttributeChanged(const QualifiedName&);

Ditto.

> Source/WebCore/svg/SVGStyledElement.cpp:204
> +AttributeToPropertyTypeMap& SVGStyledElement::cssPropertyToTypeMap()

Make it a static inline.

> Source/WebCore/svg/SVGStyledElement.h:86
> +    AttributeToPropertyTypeMap& cssPropertyToTypeMap();

No need for this, if you convert it to be a static inline.

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