[webkit-reviews] review denied: [Bug 40857] Altering the CSS class of an attached SVG element causes WebKit to crash : [Attachment 60542] Updated Patch -- The previous one was the wrong file

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied  review:
Bug 40857: Altering the CSS class of an attached SVG element causes WebKit to
crash
https://bugs.webkit.org/show_bug.cgi?id=40857

Attachment 60542: Updated Patch -- The previous one was the wrong file
https://bugs.webkit.org/attachment.cgi?id=60542&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
You did not put this patch up for review, which is done by setting the review
flag to "?", but I assume you want it reviewed.

> +    if (!namedAttrMap)
> +	   attributes(false);
> +    if (hasClass)
> +	   mappedAttributes()->setClass(newClassString);
> +    else
> +	   mappedAttributes()->clearClass();

The attributes function is not meant to be called just for its side effect. If
you have determined it needs to be called, then you should use its result and
call setClass and clearClass on that. There is no need to use the
mappedAttributes function.

Also, there is no need to change the clearClass code path. In that path,
checking namedAttrMap for 0 seems the right way to go. It's only in the
setClass path that we'd want to call attributes to force the map to be created.
Something like this:

    if (hasClass)
	attributes()->setClass(newClassString);
    else {
	if (namedAttrMap)
	    namedAttrMap->clearClass();
    }

That would work well, I think.