[Webkit-unassigned] [Bug 51490] Cleanup SVG code according to the webkit style rules 3

[bugzilla-daemon at webkit.org]

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


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #77309|                            |commit-queue-
               Flag|                            |




--- Comment #10 from Eric Seidel <eric at webkit.org>  2010-12-23 01:08:23 PST ---
(From update of attachment 77309)
View in context: https://bugs.webkit.org/attachment.cgi?id=77309&action=review

In general this looks fine, but the m_x shadowing is bad-news bears.  I'll leave kling's r+, but that definitely needs to be fixed before landing.

>> WebCore/svg/SVGSVGElement.cpp:133
>> +    double m_y = 0.0;
> 
> This looks quite wrong, m_x and m_y are member variables, and you are shadowing them here.
> Use better names please :)

We can't r+ this as-is... This would be bad news bears.

> WebCore/svg/SVGSwitchElement.cpp:53
> +    for (Node* node = firstChild(); node; node = node->nextSibling()) {
> +        if (node->isSVGElement()) {
> +            SVGElement* element = static_cast<SVGElement*>(node);
>              if (element && element->isValid())
> -                return (n == child); // Only allow this child if it's the first valid child
> +                return (node == child); // Only allow this child if it's the first valid child

Does isValid possibly run javascript or triggger any events which could invalidate the node pointer?

> WebCore/svg/animation/SVGSMILElement.h:45
> +    static bool isSMILElement(Node* node);

unneeded arg name.

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