[webkit-reviews] review denied: [Bug 13592] parseMappedAttribute inconsistency : [Attachment 14350] First attempt

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat May 5 03:58:35 PDT 2007


Oliver Hunt <oliver at apple.com> has denied Rob Buis <rwlbuis at gmail.com>'s
request for review:
Bug 13592: parseMappedAttribute inconsistency
http://bugs.webkit.org/show_bug.cgi?id=13592

Attachment 14350: First  attempt
http://bugs.webkit.org/attachment.cgi?id=14350&action=edit

------- Additional Comments from Oliver Hunt <oliver at apple.com>
*SVGAnimateMotionElement::parseMappedAttribute

I'd lift add const String& value = attr-value() after the first name check.  It
seem icky to query multiple times.

*SVGAnimateTransformElement::parseMappedAttribute
I don't think this gains anything.

*SVGAnimationElement::parseMappedAttribute
in those cases where you access attr->value() multiple times i think you should
just declare a var.  eg. the attributeTypeAttr case.

*SVGCircleElement::parseMappedAttribute
*SVGClipPathElement::parseMappedAttribute
*SVGCursorElement::parseMappedAttribute
*SVGEllipseElement::parseMappedAttribute
I'd consider a local const& to hold the name, but otherwise looks good

*SVGExternalResourcesRequired::parseMappedAttribute
looks good

*SVGGradientElement::parseMappedAttribute
local for the attribute name, and maybe use locals in those branches that query
the value multiple times.

*SVGImageElement::parseMappedAttribute
*SVGLineElement::parseMappedAttribute
*SVGLinearGradientElement::parseMappedAttribute
*SVGMarkerElement::parseMappedAttribute
*SVGMaskElement::parseMappedAttribute
*SVGPathElement::parseMappedAttribute
local for the name again

*SVGPatternElement::parseMappedAttribute
local for name, local for value on those branches that access the value
multiple times

*SVGRadialGradientElement::parseMappedAttribute
*SVGRectElement::parseMappedAttribute
local for the name

*SVGSVGElement::parseMappedAttribute
local for name, local for value those places it's used multiple times

*SVGScriptElement::parseMappedAttribute
ooh, good *and* fixed indenting

*SVGStopElement::parseMappedAttribute
*SVGTests::parseMappedAttribute
*SVGTextContentElement::parseMappedAttribute
*SVGTextPositioningElement::parseMappedAttribute
*SVGUseElement::parseMappedAttribute
local for name if it's used multiple imes

*SVGViewElement::parseMappedAttribute
good

*SVGZoomAndPan::parseMappedAttribute
local for value when it's used multiple times



More information about the webkit-reviews mailing list