User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a5) Gecko/20041102 Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8a5) Gecko/20041102 Any link when clicked on causes a new window to launch. http://www.peepo.co.uk/launch/love.svg This is specific to the first window opened when the application launches Reproducible: Always Steps to Reproduce: 1.launch application 2.visit uri 3. click on graphic link Actual Results: new window opened Expected Results: location of current window should have changed

This is because the binding for <svg:a> was erroneously changed to use "content" instead of "_content". But I have to ask... why are we handling the link behavior via the binding (poorly, I must add)? If the issue is that the back end doesn't assume type="simple", then maybe we need to make that assumption in the back end? If that's a bad assumption for non-svg, then we either special-case it for svg or we change the binding to set type="simple" in the constructor (well, off a timer, I suppose).

I think Alex would be the person most likely to know why we handle the link behavior via a binding.

(In reply to comment #1) >(poorly, I must add)? Why was it using _content? Also IMHO the event should be click, not mousedown.

Those are parts of "poorly", yes. Plus bug 267664, the fact that it doesn't check our new-window prefs like the XLink code in nsXMLElement does, the lack of support for actuate="onload", the fact that it puts text in the status bar with a different priority than other links... That sort of thing.

This should IMHO be a blocker before SVG is enabled in release builds.

Could we at least fix this to use the "click" event while we're here? And perhaps address the other issues with this code? I'd really prefer we just fix it to work right instead of wallpapering over some issues and leaving others...

I'm just doing tweaks to the binding because I'm not sure of how to do the full fix, but would like something better than our current behavior.

Comment on attachment 172975 [details] [diff] [review] as above, using click I agree this should be implemented properly, but until someone volunteers do to a proper implementation, I guess there's no harm in fixing the current implementation.

This would work great, I think, if bug 211916 (in its SVG variant) did not exist.

This makes use of an nsXMLElement, since xlink works on those... Doesn't fix all the deps of this bug (not sure why it doesn't fix the mouseout thing), but fixes this bug and bug 273197.

Why isn't this simply implemented in C++? The HandleDOMEvent code could be moved up from nsGenericHTMLElement to nsGenericElement, or to some utility class. And then called by <html:a>, <html:area>, <xml xlink=> and <svg:a>.

Comment on attachment 172987 [details] [diff] [review] Nasty hack You don't need the bogus namespace, since the svg content factory creates generic xml elements for unknown tags.

I want the bogus namespace so I can restrict the styling to that element. And yes, sicking, this should be implemented in C++. And xlink should work on HTML elements too. All of which is on my "to do sometime" list, but is not 1.8 material at this point, imo....

*** Bug 287701 has been marked as a duplicate of this bug. ***

Could the patch that changes "content" to "_content" be checked in perhaps?

Comment on attachment 172987 [details] [diff] [review] Nasty hack I'm wondering what the intent of the stylesheet is... if you just want a nested element you could use <content> <xlink:xlink xbl:inherits="xlink:href" xlink:type="simple"> <svg:g xbl:inherits=" ... "> <children/> </svg:g> </xlink:xlink> </content> (OK, xlink:xlink is just as bogus)

Aren't kids inserted via <children> styled by the scoped sheet? So what I was aiming for was having something that really wouldn't appear in <children> there...

I realized you needed the scoped stylesheet to attach the binding to the bogus node, but as that binding only appeared to exist to provide a second anonymous node I didn't understand why the original binding couldn't specify both nodes.

Oh, I see. That was because I'm not very good with XBL? ;)

That approach doesn't work because the xlink:xlink causes an nsInlineFrame to be constructed, which SVG skips over during painting because it isn't a SVG child frame.

(In reply to comment #23) > Created an attachment (id=183410) [edit] > Less simplified hack This doesn't work because the frames aren't being constructed for the link children.

More details on it not working. First, svgBindings.xml is now in chrome://global/content/svg (-moz-binding). Second, when that is corrected the following assert happens: ###!!! ASSERTION: Unable to locate an XBL binding.: 'protoBinding', file /home/tor/src/moz-trunk/mozilla/content/xbl/src/nsXBLService.cpp, line 886 Break: at file /home/tor/src/moz-trunk/mozilla/content/xbl/src/nsXBLService.cpp, line 886

Comment on attachment 183410 [details] [diff] [review] Less simplified hack Duh, yeah, I meant #xlink instead of #svg :-[

Ok, that makes it a bit happier, though it still doesn't work. There's still a Inline(xlink) frame in the heirarchy which cause the svg painting to ignore the <a> content. Heirarchy is now: SVGGenericContainer(a) Inline(xlink) SVGG(g) SVGEllipse(ellipse)

Comment on attachment 183410 [details] [diff] [review] Less simplified hack Would you mind trying extends="svg:generic" instead of display="svg:generic" ?

Yes, that worked. Attaching corrected version of your patch.

Comment on attachment 183440 [details] [diff] [review] patched version of "less simplified hack" Don't forget that I've been attaching -w versions for easy review, so you don't see the indent on the svg:g or the children.

Comment on attachment 183526 [details] [diff] [review] version with indent If it works, r=bzbarsky

Comment on attachment 183526 [details] [diff] [review] version with indent a=asa

Checked in. Thanks for your help with this, Neil.