Summary: | parseMappedAttribute inconsistency | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rob Buis <rwlbuis> | ||||
Component: | SVG | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | ||||||
Priority: | P2 | ||||||
Version: | 523.x (Safari 3) | ||||||
Hardware: | Mac | ||||||
OS: | OS X 10.4 | ||||||
Attachments: |
|
Description
Rob Buis
2007-05-05 03:01:14 PDT
Created attachment 14350 [details]
First attempt
Should be straightforward patch. Would be nice if somebody could benchmark what the change brings us :)
Cheers,
Rob.
Comment on attachment 14350 [details]
First attempt
*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
Comment on attachment 14350 [details]
First attempt
Ignore last comment, seems like this patch matches the style elsewhere
|