Closed Bug 516906 Opened 11 years ago Closed 11 years ago

Remove XLink support from nsXMLElement

Categories

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

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: sicking, Assigned: sicking)

References

Details

Attachments

(1 file)

Attached patch Patch to fixSplinter Review
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)
Attachment #400957 - Flags: superreview?(jst)
Attachment #400957 - Flags: review?(jst)
Attachment #400957 - Flags: superreview?(jst)
Attachment #400957 - Flags: superreview+
Attachment #400957 - Flags: review?(jst)
Attachment #400957 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/2411c713a499

Checked in
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Presumably there needs to be a follow-up bug to remove XLink support in the UI?
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?
Ooh, finding more stuff. Patch coming up
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?
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.
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.
(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
Why does that code ever need to worry about XLink?  It should be using the IsLink() and company APIs, right?
It *should*, but it doesn't always. And then there is javascript which currently can't. Yes, all of this needs to be fixed
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.
Blocks: 332773
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.