[webkit-reviews] review denied: [Bug 22357] Crash when setting className via SVG className.baseVal : [Attachment 26211] Possible patch to issue 22357

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Dec 22 13:00:38 PST 2008

Darin Adler <darin at apple.com> has denied Glenn Wilson <gwilson at google.com>'s
request for review:
Bug 22357: Crash when setting className via SVG className.baseVal

Attachment 26211: Possible patch to issue 22357

------- Additional Comments from Darin Adler <darin at apple.com>
=> -	if (namedAttrMap) {
> -	   if (i < length)
> -	       mappedAttributes()->setClass(newClassString);
> -	   else
> -	       mappedAttributes()->clearClass();
> -    }
> +    if (i < length) {
> +	   if (!namedAttrMap)
> +	       createAttributeMap();
> +	   mappedAttributes()->setClass(newClassString);
> +    } else
> +	   mappedAttributes()->clearClass();

It's not correct to create an attribute map here. The map is supposed to be
created on demand in functions like Element::attributes, and a map should not
be created just because the class attribute was set on an element.

I need more information about when we're going to need the attribute map later,
and we should consider putting the call to create it at that site instead of
here. Or maybe when it's created the class isn't set up correctly in it? I need
more info about how the test case is failing to understand what the right fix

This patch also introduces a null-dereferencing crash: The clearClass could
dereference 0 if namedAttrMap is 0.

More information about the webkit-reviews mailing list