Suppose you've got the following SVG: <rect id="firstrect" x="100" y="100" width="100" height="100" fill="red" /> <rect id="secondrect" x="100" y="250" width="100" height="100" fill="red" /> The "fill='red'" in both objects will be mapped to the same attribute. Calling getPresentationAttribute('fill') on either rectangle will return this shared attribute reference. Modifying the returned presentation attribute will modify both rectangles. Even more fun, the change is permanent until WebKit is restarted. So executing this code: function onLoad() { var rect = document.getElementById("firstrect"); var fill = rect.getPresentationAttribute("fill"); fill.setRGBColor("green"); } ...means that every SVG object that's loaded from then on with the attribute fill="red" will be drawn as green. Verified on Windows XP with the latest service packs installed, but I've no reason to think the bug is platform-specific.
Created attachment 20556 [details] Repro case
Confirmed that this happens in the Mac OS X build. Results in other browsers: o Firefox 3.0b5 doesn't appear to support getPresentationAttribute(), or if it does, doesn't support changing the returned value. Both squares appear red. o IE 7 doesn't support SVG, period. o Opera 9.27 appears to work. The top square turns green, the bottom square remains red
Yeah, I guess we can't just wrap the style object. Need to make modifications to the inline style decl: spec: http://www.w3.org/TR/SVG/types.html#InterfaceSVGStylable original bug: https://bugs.webkit.org/show_bug.cgi?id=13976
I don't think that "every attribute which is loaded from then on" will have red. Rather any sibling element which happens to be sharing the RenderStyle with this element will suddenly turn red.
Try it. Load the repro case in Safari and then without restarting, visit an SVG document with fill="red". LayoutTests/svg/custom/viewport-clip.svg will do.
Style sharing for HTML actually looks at the mapped attributes, and if they don't match it avoids sharing. I don't know if this is helpful or not. It sounds similar.
Created attachment 20694 [details] First attempt This small patch fixes the problem. However I wonder if there is a nicer way to do it. I guess there is nothing wrong with sharing the style declaration for this attribute though, since in the testcase the values are equal and we do not know up front that we are going to use getPresentationAttribute, which is used very rarely anyway. So I really can't see another way, though maybe the unsharing can be done with less statements or some helper method in StyledElement. I'll do the testcase later, I think the original testcase in the bug is a good base for an automated test. Cheers, Rob.
Created attachment 20696 [details] Now with testcase Just added a testcase, code is the same. Cheers, Rob.
Comment on attachment 20696 [details] Now with testcase Is it possible that the style will not be shared at the time you call getPresentationAttribute() but a later addition to the DOM will make it shared?
Comment on attachment 20696 [details] Now with testcase + if (cssSVGAttr->style()->refCount() > 1) { The hasOneRef() function was made for uses like this one. I think it'd be better to use that. r=me
Committed revision 34448.