[Webkit-unassigned] [Bug 110926] Can't use fastGetAttribute with WebCore::HTMLNames::classAttr when element is a SVGStyledElement

[bugzilla-daemon at webkit.org]

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





--- Comment #42 from Eric Seidel <eric at webkit.org>  2013-04-09 01:47:32 PST ---
(From update of attachment 196067)
View in context: https://bugs.webkit.org/attachment.cgi?id=196067&action=review

>> Source/WebCore/dom/Element.h:862
>> +    return fastGetAttribute(HTMLNames::classAttr);
> 
> This is a great idea, but I believe the execution is still a little bit incorrect.
> 
> It’s a programming error to call fastGetAttribute on classAttr from any call site other than this function. Thus the fastAttributeLookupAllowed function should return false for classAttr. Thus, this line of code is wrong, and the fastAttributeLookupAllowed function also is wrong to return "true" for classAttr since it’s not allowed.
> 
> There are a number of ways to fix this. The simplest is to make a separate private inline function for the “guts” of fastGetAttribute, without the fastAttributeLookupAllowed assertion, and call that here. There are various other suitable refactoring. Even for SVG elements, there is no reason to do the styleAttr check that getAttribute does when fetching a classAttr, so that’s a case we could optimize further if desired.
> 
> Have we done performance tests on the difference in performance from calling getAttribute vs. "if (isSVG) getAttribute() else fastGetAttribute()"? If it’s not a measurable difference then we should consider that implementation.

It appears that fastAttributeLookupAllowed already returns "false" for "class" in the SVG case:
http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Element.cpp#L2609

So I'm not sure that the refactoring you suggest is necessary?  fastAttributeLookupAllowed seems to have enough context to tell if class is safe or not.

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