516906 - Remove XLink support from nsXMLElement

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

Our XLink support in nsXMLElement has issues:

1. It's not actually working as defined by spec as I understand it. XLink just defines that a link exist, not what type of link (i.e. is the link describing where the site-map is, is the link to a stylesheet to be used to style the document, is the link a clickable navigation link).

2. I don't think it's used enough to warrant the code needed. Especially for autolinks.

3. Its current implementation means that whenever we add explicit classes for new namespaces, those namespaces loose the ability to use xlink. This happened to MathML for example. (granted, given the very limited number of element classes we have, this doesn't seem like a big deal)

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

http://hg.mozilla.org/mozilla-central/rev/2411c713a499

Checked in

Comment 2

[[e-mail]]

Presumably there needs to be a follow-up bug to remove XLink support in the UI?

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Where is it used? All I could find in the tree was 453278-frame.xml and the files in content/xml/tests. Is there more?

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Ooh, finding more stuff. Patch coming up

Comment 5

[Boris Zbarsky [:bzbarsky]]

Did that patch ever happen?

Doesn't the nsXMLContentSink comment need fixing?

What about the nsGenericElement code that deals with xlink:href?  Is it still needed?

Can we get a followup bug filed on the bogosity in nsXBLPrototypeHandler::DispatchXBLCommand?

Comment 6

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I wrote a bunch of this patch but lost it in a harddrive failure.

There's actually not that much code that can be removed, but a lot of it needs to be changed so that it only honors xlink:href for MathML elements.

It would be great if there was some way to use an nsILink-like interface from javascript.

Comment 7

[Boris Zbarsky [:bzbarsky]]

We could add a scriptable interface for that sort of thing, sure.

> but a lot of it needs to be changed so that it only honors xlink:href for
> MathML elements.

I thought we were going to just push that down into the mathml element class, at least with shawn's patches.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

(In reply to comment #7)
> > but a lot of it needs to be changed so that it only honors xlink:href for
> > MathML elements.
> 
> I thought we were going to just push that down into the mathml element class,
> at least with shawn's patches.

Yes, but that'll only help with a small part of the remaining code. Most of the remaining changes lives in code that inspects elements from the outside

Comment 9

[Boris Zbarsky [:bzbarsky]]

Why does that code ever need to worry about XLink?  It should be using the IsLink() and company APIs, right?

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

It *should*, but it doesn't always. And then there is javascript which currently can't. Yes, all of this needs to be fixed

Comment 11

[Boris Zbarsky [:bzbarsky]]

Can we at least get bugs filed on the remaining issues?

I think we can get some noticeable perf improvements here, if we push the visited/unvisited stuff entirely into elements and so forth; hence my concern with this.