User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.4) Gecko/20030624 xlink work in "general" xml documents, and not work in xhtml document, but xhtml also valid xml-document therefore xlink must work in xhtml too. Reproducible: Always Steps to Reproduce: make valid xhtml document with xlink Actual Results: xlink not work Expected Results: xlink must work

Reporter: can you give us a testcase? What content-type are you testing the document with? Mozilla doesn't always treat XHTML as XML. How does this block you from testing Mozilla?

Wrong component anyway. Also, what XLink support are you looking for?

.

Resummarizing to reflect the problem. The issue is that all the XLink magic lives on nsXMLElement, while it should live on nsGenericElement if XHTML nodes are to make use of it.

> What content-type are you testing the document with? Mozilla doesn't always treat XHTML as XML. Why? XHTML in first is XML. And in second HTML. Are you suppose that in web possible something like "street XHTML"? Also i think that doctype declaration inside document must override content-type and surely file extension (xhtml can be *.htm and *.xml) and only doctype say everything about document type. If you save Testcase as htm both link will not work (even if you insert DOCTYPE). In my application file extension is xml but content type set to "text/html" because IE 6.0 have same bug (it also ignore DOCTYPE declaration) and in case when content type set to "text/xml" IE just display xhtml-document as xml-tree.

> Are you suppose that in web possible something like "street XHTML"? Yes. Most of the web content labeled with an XHTML DOCTYPE is neither valid nor well-formed XML, and thus breaks completely when parsed with an XML parser. > Also i think that doctype declaration inside document must override > content-type and surely file extension (xhtml can be *.htm and *.xml) and > only doctype say everything about document type. That disagrees with RFC 2616, the opinion of the W3C's HTML WG, and that of our own standards people. You probably want to look into content-negotiation: the page gets served as application/xhtml+xml to browsers with that Content-Type in their Accept: header, and text/html otherwise.

arg, I hadn't realized things had changed that much. When XHTML was new, files labeled as XHTML were pretty much always XHTML. It's sad if this is no longer the case. So what happened? Have webmasters set a default content type for .htm/.html files even when they don't control the content their users produce?

well, xhtml is future of html... in this case we must concentrate all our energy to make this future extremely closer to present... maybe we can convert each old html document on fly into xhtml (not into text but directly into xhtml node-tree - it faster) and then work with him as with xhtml document?... in this case we will not work for past - only for future without dividing our energy between html and xhtml code... for example all html links we can handle as simple xlink in xhtml nodes... it will make browser code more simple and more manageable... every application sometime need total code rebuilding... i guess that birth of xml, xhtml, xlinks and xforms is good chance to make this code rebuilding for mozilla... you right understand that i also propose make similar transformation from html forms into xforms... and i have also one question - how can i help?..

The way to do this is to kill off nsIXMLContent, push those methods up to nsIContent, and move the xlink handling code to nsGenericElement.... Add some checks to make sure we don't trigger it for non-XHTML HTML nodes, and off we go.

We don't even neccesarily need to check for non-xhtml html. The html-parser/contentsink will never parse/set namespaced attributes anyway

Mental note: <xhtml:a href="foo" xlink:type="simple" xlink:href="bar" /> Should probably go to "foo", not "bar".

Yeah, if the attributes _somehow_ make their way into an HTML DOM, just obey them, doesn't make sense to explicitly check for us not to do that... And <a href=""> and <link href=""> and <area href=""> should override any xlink on the same element, btw.

There is something strange, but maybe you allready know that : Even if the XHTML element with XLink doesn't seems to be a link, if you open the contextual menu, you'll see that you can open the link in a new tab/window. And control+click works also.

Just came accross this bug when someone was asking about new windows in xhtml. I would I be right in assuming that <xhtml:a href="foo" xlink:show="new">aaa</xhtml:a> not opening in a new window is because of this bug?

Yes

Taking.

This builds, but I need to check things more thoroughly before getting review.

Don't add the mIsSimpleXLink member to nsGenericElement, we don't want to grow all elements by 4 bytes. Either use a flag (like GENERIC_ELEMENT_HAS_PROPERTIES), or simply check if the attribute is there as needed, that is probably quite fast enough and means that you don't have to keep the attribute and the flag in sync.

In the patch as it stands, mIsSimpleXLink equates to a single AttrValueIs() call, which isn't too expensive, but that's actually wrong. Before acting on mouse clicks, and before saying we're focusable, we actually need to check that we are a simple, _onRequest_ link. This requires us to check: * xlink:href="simple" * xlink:show="onRequest", or is empty, or is not set So something like: mIsSimpleXLink = AttrValueIs() && (!HasAttr() || AttrValueIs() || AttrValueIs()); This is a bit more work, and I think doing all this work in PostHandleEvent for every event, on every element along every event chain might likely hurt. Also IsFocusable should be doing the same check. How about I add a GENERIC_ELEMENT_IS_SIMPLE_ONREQUEST_XLINK flag, and add an IsSimpleOnRequestXLink() member to nsIContent? Another reason I want to use a flag is because I want to use the XLink code to handle links in SVG even when xlink:type is not set. Using a flag will allow me to override what is meant by a "simple, onrequest xlink" in nsSVGElement::SetAttr by setting the flag even if xlink:simple isn't present. I don't really want to give nsGenericElement specialist knowledge about SVG, or to be calling a modified version of nsContentUtils::IsSimpleXLink with this knowledge (since it too would then be a more expensive call).

Actually, you'd only need to GetAttrInfo() calls since you're inside nsGenericElement. And most of the time you'd only need one when the xlink:href attribute is not set. I'm not quite following what you want to do for svg. You want to use this code to implement <svg:a>? If so, couldn't you just override IsFocusable and PostHandleEvent in nsSVGAnchorElement to always behave like a link? That said, if you wanna use a flag i'm fine with that too. As long as you're dealing with all possible mutations and scripts firing in the middle of SetAttr calls.

Forgetting the topic at hand for a moment I have a couple of questions. To check that an attribute is set with a specific value, using AttrValueIs should be slightly cheaper than using GetAttrInfo, right? And FindAttrValueIn should be cheaper than GetAttrInfo for checking for a number of alternative values, right? Yes, I do want to use this (the XLink) code to implement the link traversal behaviour of <svg:a>. Overriding IsFocusable and PostHandleEvent in nsSVGAElement seems like too much bloat, since I'd just be copying the code from nsGenericElement and changing a couple of lines. The two "almost" copies would have to be kept in sync too. Using an nsContentUtils::IsSimpleXLink helper or having the respective classes set a flag correctly in their SetAttr implementations seemed like a better way to me. (In reply to comment #21) > most of the time you'd only need one when the xlink:href attribute is not set. I don't get it. What's your logic here? > As long as you're dealing with all possible mutations and scripts firing > in the middle of SetAttr calls. Hmm...I hadn't thought of that. I assume putting the stuff that sets the flag (if we go that route) after the SetAttrAndNotify call in nsGenericElement::SetAttr would do the trick? For svg:a it could be put in nsSVGAElement::AfterSetAttr so that it can then override the flag set by nsGenericElement::SetAttr.

Using GetAttrInfo should be about exactly as cheap, though more code. You should be able to use FindAttrValueIn here it looks like. You wouldn't need to duplicate hardly any code, just make nsGenericHTMLElement::PostHandleEventForAnchors a bit more generic and call into that from all callsites. That is, you can make both nsGenericElement, nsHTMLAnchorElement, nsHTMLAreaElement and nsSVGAnchorElement all call into a single anchor-event-handler method. >> most of the time you'd only need one when the xlink:href attribute is not >> set. > I don't get it. What's your logic here? Most of the time there is no xlink:href attribute so you don't even need to look for an xlink:show attribute

(In reply to comment #23) > Most of the time there is no xlink:href attribute so you don't even need to > look for an xlink:show attribute At the moment the code doesn't even check for xlink:href. If we do then we'll just be doing a HasAttr check for xlink:href before we do our AttrValueIs check for xlink:actuate. (Yeah, 'actuate'. Sorry it was me who incorrectly said 'show' first.) Hmm, okay, the HasAttr check should be cheaper, and for most cases we will just return at that point. So currently nsGenericElement::PostHandleEvent just returns NS_OK for most HTML elements. With this change every call to this function will at a minimum do: if (!(nsEventStatus_eIgnore == aVisitor.mEventStatus && HasAttr(kNameSpaceID_XLink, nsGkAtoms::href) && // most calls fail here /*...more checks...*/)) return NS_OK; Okay, hopefully the addition of an |if| statement, three logical operations and a HasAttr call in nsGenericElement::PostHandleEvent won't hit us too hard. Or maybe the HasAttr check should be done first on the basis that it will be the expressions that most rarely evaluates to true?

I think we should be checking for the event type before the HasAttr call. At least that's how it seems to me... Most events we don't care about, right?

I'd appreciate answers to the XXXjwatt questions in PostHandleEventForAnchors, if anyone knows, and eyes on the changes I've made. It would really help in refactoring to merge link handling with nsXMLElement.

> + // XXXjwatt: huh? this needs a better comment. All the comment says is that we need a prescontext to do link stuff and that not all events have prescontexts. A particular example of an event that has no prescontext is a mutation event, but there are plenty of others too. Frankly, I'm not sure how correct it is that we need a prescontext to do link stuff, but that's a separate issue. >+ // XXXjwatt: huh? isn't it the most deeply nested link that gets triggered, >+ // so why check for a link higher up? Up until the event dispatch landing, it was NOT the most deeply nested link that got triggered, actually. I don't know what we do now, but we have bugs on making this sane and removing the code in question that are dependencies of bug 234455. > + // XXXjwatt: why even check if it's a DOM event, surely just checking it's > + // trusted is quicker and will give us the same answer? I believe the idea is checking via the API we have for checking for trusted events; that lives on the DOM event interface. But really, the fact that we have nsEvent and a separate DOM event is silly; again, this is a dep of bug 234455. > + // XXXjwatt: should we set nsEventStatus_eConsumeDoDefault ?? Good question. If we had any idea what those constants actually mean in terms of behavior, we could answer it! ;) > + // XXXjwatt: do we really need this check before casting? If > + // mEvent->message is corrupt, then what chance is there for > + // mEvent->eventStructType? When creating events via DOM APIs, "message" is set by the initEvent call on an event object, while the event struct type is set by the createEvent call that creates the object. So the struct type will always correspond to the actual object type, while there are no such guarantees for the message. > Maybe the target parameter > + // should be removed from nsWebShell::OnOverLink since it's unused? Maybe, or maybe we should use it (and indicate links to something other than the current page in our UI). imho, we should leave this argument on nsILinkHandler and we should pass the right target in. The fact that XLink does something weird just indicates to me that no one really uses XLink... ;) > + // XXXjwatt: does this have to be *No*Default? nsXMLElement uses > + // *Do*Default See above on event status constants.... Maybe smaug can help here, though. Does that help?

Yes, very helpful, thanks! Especially the explaination of why we much check before the cast. I also find the justification for keeping the target argument to be compeling. > > + // XXXjwatt: why even check if it's a DOM event, surely just checking > > + // it's trusted is quicker and will give us the same answer? > > I believe the idea is checking via the API we have for checking for trusted > events; that lives on the DOM event interface. Yes. My question was why do we want to do it that way? It's slower with all the QI's than just checking if there is a dom event and then checking trusted on the nsEvent, but it should give the same result. (The DOM event API just defers to the nsEvent.) In fact why even check if it's a DOM event at all? Surely it only matters that it's trusted.

That's left over from when nsEvents could have an undefined trusted state if there was no associated DOM event, probably.

I'm going to leave the merging of nsGenericElement::PostHandleEvent and nsGenericHTMLElement::PostHandleEventForAnchors for a follow-up bug (which I'll do). Previously we allowed xlink:actuate="onLoad" links to be activated and traversed after the document had loaded (for example, as a result of a click). This patch fixes that.

I'm not going to be able to get to this for a bit, but offhand we should be checking the event type before we check for "linkness". The latter is a much more expensive check, I think...

I also don't see where GetSimpleOnRequestXLinkURI is defined/implemented...

Oops, forgot the nsContentUtil changes. I'm not quite sure how to do the event type check in a way that we don't do it twice, but I'll think about it some more later today.

See also bug 332773 ("Drop XLink support").

> > if you wanna use a flag i'm fine with that too. As long as you're > > dealing with all possible mutations and scripts firing in the middle > > of SetAttr calls. So for the purposes of having an IsXLink flag on the element I only need to care about updating it whenever the element's attributes change, right? *Attribute* mutation events are only sent out in nsGenericElement::SetAttrAndNotify, so if I put my check after the call to that function in nsGenericElement::SetAttr, it should be after any scripts have done their thing. Anyone want to disagree with this before I go and change the patch? Sicking, bz?

You should update it right next to the call to nsAttrAndChildArray::SetAttr to be sure that you're always in sync with that array. But again, i'd feel happier about not having the flag at all if possible.

Hey, previously you said you didn't mind either way. ;-) Yeah, you're right, setting the flag after the SetAttrAndNotify calls is too late. My reasoning was quite flawed. That means I can't change the criteria on which a flag would be set in the way I was hoping for SVG. :-( Heck, I shouldn't be worrying about SVG at this stage. It's just hard not to when you think you're going to have to completely rethink the approach you take once you do. :-) Now if only I could figure out why the first three links in the testcase aren't being styled as links...

The trick is that you always want to keep the flag in sync with the nsAttrAndChildArray. Putting the flag-update in SetAttrAndNotify is fine. I'm fine with either way as in I won't complain when reviewing. As long as the flag is simply a cache to avoid spending time in HasAttr/AttrValueIs calls. I'm not going to r- for it. However, I prefer not to have duplicate data if we can get away with it.

The previous patch wasn't styling XLinks on HTML elements even though they were responding as links should to mouse events etc. All styling seems correct, except that when an XLink on an HTML element is focused it doesn't get the "ants" around it. There's another issue. With the current patch, XLinks will only respond to events if the element doesn't already have a PostHandleEvent implementation (they don't defer, and I don't think we necessarily want them to). However, all HTML elements that are XLinks would be styled as a link. I'm not sure what to do about that yet.

Oops, ignore that. It was the wrong patch. Here's the correct one.

Now that I've just about fixed this, I'm slowly comming to the conclusion that this bug should be resolved as WONTFIX. *pauses to wait for Hixie's cheers to die down* I originally thought I'd fix this bug partly because I assumed we wanted to fix it, but more because the work required would clear the way to fixing the major problems with our implementation of SVG's link element. However, I now find myself forced to face up to some major unanswered questions that force me to ask myself whether we really need XLink support in XHTML? So XHTML "is XML", but that in itself is not a compelling reason to add XLink support to XHTML content. XHTML already has its own link element, <a>, and in _X_HTML there are no restrictions on what you can wrap with it. The only remotely useful think I can think of that simple XLinks can do that pure XHTML cannot, is to open a link in a new window onLoad. Personally, I'd almost consider that a bug in the language. So what compelling reasons are there to add XLink support to XHTML? I'm asking that as a serious question in case anyone thinks they have a good answer. I'm doubting anyone does, especially since XHTML and simple XLink support has been in Mozilla for years, yet there are next to no CC's on the bug and I've never heard anyone complain elsewhere. In case anyone is wondering what the questions I've not been able to answer are, they're to do with elements that already have actions to perform in response to user input. What does it mean to have an XLink on a checkbox? Should the checkbox be (un)checked and the link ignored? Or vice versa? Or should both happen? Why? And why would authors want to do what anyway? They could wrap the checkbox in an XHTML <a>. Just to clarify, I don't think we should be removing XLink support from Mozilla entirely (bug 332773), but just that we shouldn't add it to XHTML, SVG and other XML languages we may support that have adequate linking mechanisms of their own.

Does it affect the (non-working yet) XPointer support in XHTML?

The argument I can see for adding XLink to XHTML is consistency. Why should a <foo xlink:href> behave differently from <xhtml:div xlink:href>? I do see the argument that it gets confusing if you put an xlink on a checkbox and if implementation becomes hairy then I'm fine with specifically excluding namespaces like xhtml, svg, and xul from the xlink code. Basically i'd say, lets just do what is easiest. Putting an xlink on a checkbox is about as dumb as putting a checkbox inside an <a>. We can't prevent the latter, so I don't see a whole lot of harm in allowing the former if that is what's easiest.

I don't have time to work on this right now. Assigning to xml@core.bugs.

I don't see why we want to fix this bug. We need xlinks to be "clickable" links in SVG and MathML for historical reasons, but the same doesn't apply in XHTML or HTML. In fact, I believe the SVG and MathML groups are talking about using non-namespaced href attributes instead of xlink. So marking WONTFIX for now. If you disagree, feel free to reopen but include a description of why you think this should be fixed and why you can't simply use <a> elements instead.

(In reply to comment #46) > I don't see why we want to fix this bug. We need xlinks to be "clickable" links > in SVG and MathML for historical reasons, but the same doesn't apply in XHTML > or HTML. > The same applies to XHTML as XHTML is XML. It's said in first comment. > In fact, I believe the SVG and MathML groups are talking about using > non-namespaced href attributes instead of xlink. > I don't know what about other groups talk, however I cannot see why XHTML should behave differently from XML and to forbid XLink. E.g. having XHTML1 you cannot put @href to any element. You can put XLink attribute to them.

> The same applies to XHTML as XHTML is XML. Yes, but nothing defines how XLinks should work for random XML. See my posts over the years to the www-xml-linking-comments@w3.org mailing list and their (usually months to years later) responses to them. MathML and SVG explicitly define what should happen when attributes in the XLink namespace are used with them. For XHTML, nothing says they should do anything (and the xml-linking folks seem to think they should do nothing in that case). > however I cannot see why XHTML should behave differently from XML and to > forbid XLink. It doesn't behave differently anymore. We dropped support for random application of XLink to random XML, based on the feedback from xml-linking. > You can put XLink attribute to them. You can put whatever attribute you want on whatever you want, but nothing says it should do anything as a result...

(In reply to comment #47) > (In reply to comment #46) > > I don't see why we want to fix this bug. We need xlinks to be "clickable" links > > in SVG and MathML for historical reasons, but the same doesn't apply in XHTML > > or HTML. > > > The same applies to XHTML as XHTML is XML. It's said in first comment. Is there a large body of documents out there which use xlink as linking mechanism on XHTML elements? I find that surprising as a large marketshare of browsers have never supported that. Firefox and IE has never supported it and at least current versions of webkit doesn't. Don't have Opera handy to test.

In reply to comment #48) > > The same applies to XHTML as XHTML is XML. > > Yes, but nothing defines how XLinks should work for random XML. See my > posts > over the years to the www-xml-linking-comments@w3.org mailing list and their > (usually months to years later) responses to them. MathML and SVG > explicitly > define what should happen when attributes in the XLink namespace are used > with > them. For XHTML, nothing says they should do anything (and the xml-linking > folks seem to think they should do nothing in that case). > Because XLink is general framework. I know it's not well specified and lot of statements are should and fuzzy. However at least the simple type with no fancy actuate is clear and useful as this is the only way how to link generic XML documents. > > however I cannot see why XHTML should behave differently from XML and to > > forbid XLink. > > It doesn't behave differently anymore. We dropped support for random > application of XLink to random XML, based on the feedback from xml-linking. > When I see `legacy SVG and MathML' in the drop-XLink bug report I start thinking nobody likes XML. And I thought XML could heal the Web.

(In reply to comment #49) > Is there a large body of documents out there which use xlink as linking > mechanism on XHTML elements? Because of that:? > I find that surprising as a large marketshare of > browsers have never supported that. Firefox and IE has never supported it and > at least current versions of webkit doesn't. XHTML2 allows @href's on any element. It's just a step to XLink. Why to have linking mechanism specific to each language if we can have one independent? Nevertheless you removed XLink from Gecko, thus this report lost its meaning.

> However at least the simple type with no fancy actuate is clear No, it's not. Please do go read those xml-linking threads. In particular, trying to implement it the way we had it implemented is actually incompatible with how SVG uses XLink. Seriously, go read the existing discussion instead of repeating it here. > Because of that:? It doesn't matter _why_ XLink is not used in general. The point was that we want support it in SVG and MathML because there is somewhat extensive existing content out there using it in those contexts. There is no existing XHTML content using XLink, so there is no compatibility argument for supporting it. And given that and the feedback from the XML Linking working group that support for "generic XLink" is wrong, the generic support was removed.