please try the sample page: https://rapidrabb.it/files/svg-image-bug.xhtml description: I have an svg image tag with an empty xlink:href attribute and the broken image icon is shown. then i use JavaScript to set the display attribute and the xlink:href at the same time in three functions as follows: - set a real image url and display to inherit. - set an empty image url and display to none. - remove the xlink:href attribute and set display to none. Either the image should be shown (first case) or nothing should be visible. However, in some cases the broken image icon reappears.
Created attachment 31168 [details] example of the bug with short description
forgot to mention: this bug seems to be new since Safari 4 Beta.
Created attachment 31244 [details] First attempt This fixes the bug :) I tried to emulate the two problems in two separate testcases. Cheers, Rob.
Comment on attachment 31244 [details] First attempt > + * svg/SVGElement.cpp: > + (WebCore::SVGElement::setXmlbase): I don’t see SVGElement.cpp diffs in the patch.
Comment on attachment 31244 [details] First attempt SVGLoad events would be a better solution than using timeouts.
Created attachment 31257 [details] Fixed testcases I think Eric meant this with using SVGLoad. Also removed setXmlBase stuff. Cheers, Rob.
Comment on attachment 31257 [details] Fixed testcases I don't understand this clause: 95 if (isURIAttribute) 96 m_imageLoader.updateFromElementIgnoringPreviousError(); 97 else if (!renderer()) 98 return; Why only return when ! isURIAttribute && !renderer()? That seems wrong, given all the other code below. How does HTML handle this case? This patch doesn't seem right.
Created attachment 31598 [details] Different approach I think this approach is clearer, the image loading is part of the general attribute change notification and does not depend on the renderer being there. If any svg attribute changes still a setNeedsLayout is called, when there is a renderer. HTMLImageElement also loads image on src changes in parseMappedAttribute. Cheers, Rob.
Comment on attachment 31598 [details] Different approach Does this still work if you set href.baseVal.value= "newurl.png"? You're removing the m_imageLoader.updateFromElementIgnoringPreviousError(); update from svgAttributeChanged, which makes me think that this will break updates which are driven by JavaScript property changes. Yes, it sucks that JS doesn't just call parseMappedAttribute. Eventually we need to fix that. I think this broke href.baseVal, so r-
Created attachment 31666 [details] Back to fixing SVGImageElement::svgAttributeChanged These changes were made: - I went back to the original fix. Since svgAttributeChanged is there for SVG and parseMappedAttribute is not called for the JS bindings, I tried to fix up SVGImageElement::svgAttributeChanged. Basically the xlink:href handling is singled out first and done independent of renderer() being there or not. The rest is as before, I hope the code is more readable than in the first patch. I guess in the long run svgAttributeChanged should go (is there a bug for that?). - I added a test for the href.baseValue setting alternative, which does not end up in parseMappedAttribute. - I updated the ChangeLogs because it also fixes bug 26392. Cheers, Rob.
Comment on attachment 31666 [details] Back to fixing SVGImageElement::svgAttributeChanged Looks OK.
Landed in r45095.