[Webkit-unassigned] [Bug 54538] symbol display <use> at wrong scale

[bugzilla-daemon at webkit.org]

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


Nikolas Zimmermann <zimmermann at kde.org> changed:

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




--- Comment #4 from Nikolas Zimmermann <zimmermann at kde.org>  2011-02-22 01:43:23 PST ---
(From update of attachment 83280)
View in context: https://bugs.webkit.org/attachment.cgi?id=83280&action=review

Great work, r- because we need more test coverage:

> Source/WebCore/svg/SVGElementInstance.h:63
> -    void clearUseElement() { m_useElement = 0; }
> +    void clearCorrespondingUseElement() { m_correspondingUseElement = 0; }
> +    void clearDirectUseElement() { m_directUseElement = 0; }

I'd leave clearUseElement() as is, and let it clear both m_correspdondingUseElement and directUseElement, as you always want to clear both of them in parallel.

> Source/WebCore/svg/SVGUseElement.cpp:395
> +    String directUseElementName = directUseElement ? directUseElement->nodeName() : "null";

This is fine, as it's just debugging code.

> Source/WebCore/svg/SVGUseElement.cpp:551
> +    m_targetElementInstance->setDirectUseElement(this);

I'd remove this line, and directly initialize m_directUseElement to 'this' in the SVGElementInstance constructor, by using the first constructor param, just like it's done for m_correspondingUseElement.

> Source/WebCore/svg/SVGUseElement.cpp:756
> +    newInstancePtr->setDirectUseElement(static_cast<SVGUseElement*>(target));

Very good catch!

I guess correspondingUseElement should stay as is:
"
The corresponding ‘use’ element to which this SVGElementInstance object belongs. When ‘use’ elements are nested (e.g., a ‘use’ references another ‘use’ which references a graphics element such as a ‘rect’), then the correspondingUseElement is the outermost ‘use’ (i.e., the one which indirectly references the ‘rect’, not the one with the direct reference)."

Can you please extend the testcase, to check that the generated SVGElementInstance is just like SVG describes it here?

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