Attached test case shows a bug in Webkit (Safari 4). The <use> points at a <text> element that is defined later in the document. Webkit does not dislay the black outline text behind the orange. NOTE that if you put the <use> element after the <text> it shows up.
Created attachment 29793 [details] Test Case showing the problem
Created attachment 32278 [details] First attempt The default SVGElement:insertedIntoDocument is not enough in this case, since the text content may not have been appended to the <text> element yet, therefore do something extra in childrenChanged. Cheers, Rob.
Comment on attachment 32278 [details] First attempt Isn't: 8 // Invalidate all SVGElementInstances associated with us 139 SVGElementInstance::invalidateAllInstancesOfElement(this); needed in the root SVGElement childrenChanged method? How do we handle updating <use> elements when other subtrees change?
Hi Eric, (In reply to comment #3) > (From update of attachment 32278 [details]) > Isn't: > 8 // Invalidate all SVGElementInstances associated with us > 139 SVGElementInstance:: invalidateAllInstancesOfElement(this); > needed in the root SVGElement childrenChanged method? How do we handle > updating <use> elements when other subtrees change? I think <use> only references styled elements, so the hooks are only there, see SVGStyledElement::childrenChanged and SVGStyledElement::svgAttributeChanged. However that childrenChanged only calls invalidateAllInstancesOfElement after parsing is over, so that is not enough in the use forward references to text, hence my patch overwriting it. Can you think of scenarios/testcases where the patch fails? Any other forward referencing cases that fail? Cheers, Rob.
Created attachment 33019 [details] More efificent This version is slightly more efficient since it skips SVGStyledElement::invalidateAllInstancesOfElement. I still think the patch is correct so putting it up for review again. Cheers, Rob.
Comment on attachment 33019 [details] More efificent Please make your test dumpAsText, no need to use a full rendering test. Why shouldn't this just be on SVGElement: 134 void SVGTextElement::childrenChanged(bool changedByParser, Node* beforeChange, Node* afterChange, int childCountDelta) 135 { 136 SVGElement::childrenChanged(changedByParser, beforeChange, afterChange, childCountDelta); 137 138 // Invalidate all SVGElementInstances associated with us 139 SVGElementInstance::invalidateAllInstancesOfElement(this); 140 } 141 How do non-text nodes get around needing to make this call to update <use> stuff?
Fixed in ToT, closing bug.