<tref> element not implemented http://www.w3.org/TR/SVG/text.html#TRefElement Should be quite simple.
Created attachment 7757 [details] Possible fix. Possible fix. Only does local references. I am up for suggestions on what test case(s) to add. Cheers, Rob.
Comment on attachment 7757 [details] Possible fix. A couple comments: 1. This violates the style guidelines: + if (SVGURIReference::parseMappedAttribute(attr)) return; (the return should be on its own line) + DeprecatedString ref = String(href()->baseVal()).deprecatedString(); is unnecessarily complicated. If you really need a deprecatedString, I believe you can ask the AtomicString directly. If you're going to ignore the exception you might call this variable "ignored" + ExceptionCode ec = 0; unless there is already a version of createTextNode which ignores exceptions. New code should have the * close to the type not the variable name. Generally we try not to use closeRenderer, but rather respond to dynamic updates. In this case we should update the text children whenever the href property changes. Probably a call to setTextContent would work well for that. Are you sure you should return RenderInline and not RenderSVGText? I'd have to go stare at both to decide myself. I think there are enough minor issues here to prevent landing.
Created attachment 7773 [details] Better patch This patch addresses some of the issues. I do not really see how to prevent the deprecatedString and I think using RenderInline is correct. Cheers, Rob.
Comment on attachment 7773 [details] Better patch attach() is still the wrong way to do this. As this won't handle dynamic updates. I think this would be "ok" to land as is, but it's better to just fix it right the first time. Also, ideally your patch would contain a diff of the test results you've changed, including possibly a dynamicly updating test to show that you got href changes working correctly (which is what my previous attach() complaint is about). Other than that, this looks great. Wonderful work as always Rob.
Created attachment 7807 [details] Better patch yet This patch should fix the problem with using attach(). I think since handling of attributes is not guarenteed, we need to check for the referenced text both in attributeChanged and parseMappedAttribute. I chose for a private helper method, I have no idea whether a non-member(hidden) method is better, but I'll probably soon know. Finally the patch lacks testcases, I just commit the patch to find out whether the code part is correct now. Cheers, Rob.
Created attachment 7820 [details] Tref tests + changed test results See description :) Cheers, Rob.
Comment on attachment 7807 [details] Better patch yet Fabulous. r=me
I landed. However I did not update all of the pixel results, given that I am landing from an intel machine and we have outstanding issues wrt pixel results on mac intel boxes.