Closed Bug 655238 Opened 14 years ago Closed 14 years ago

stylesheet link header doesn't support official XSLT media type

Categories

(Core :: XSLT, defect)

Product:
Core
Component:
XSLT
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla7

People

(Reporter: julian.reschke, Assigned: julian.reschke)

References

(
URL
)

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

add "application/xslt+xml" as media type for XSLT, affects stylesheet loading both from processing instruction and HTTP Link header field
1.93 KB, patch
Details | Diff | Splinter Review
add "application/xslt+xml" as media type for XSLT, affects stylesheet loading both from processing instruction and HTTP Link header field, also adding it to the Accept header sent when fetching XSLTs
3.00 KB, patch
sicking
: review+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:2.0.1) Gecko/20100101 Firefox/4.0.1 Build Identifier: When processing an HTTP Link header field with rel=stylesheet, Firefox accepts "text/xsl" but not "application/xslt+xml". Reproducible: Always Steps to Reproduce: Run test case at <http://greenbytes.de/tech/tc/httplink/#simplexslttypeoff>. XSLT should be applied. Actual Results: But isn't.
Depends on: 231395
That's the relevant code, yes. There's no mention of "application/xslt+xml" anywhere in our codebase; as far as I can tell we treat "text/xsl" as the XSLT media type. Is the registration for this type even finalized yet? Also note that http://www.w3.org/TR/xslt20/#media-type-registration is somewhat self-contradictory: This media type registration is for XSLT stylesheet modules as described by the XSLT 2.0 specification, which is located at http://www.w3.org/TR/xslt20/. It is also appropriate to use this media type with earlier and later versions of the XSLT language. and: This new type is being registered in order to allow for the expected deployment of XSLT 2.0 on the World Wide Web, as a first class XML application. I'm having a hard time reconciling the latter with the former; in particular the latter seems to imply that the type is meant to differentiate XLST 2.0 from XSLT 1.0 but the former is sort of on the fence about that. Note that we do not implement XSLT 2.0 last I checked.
(In reply to comment #2) > That's the relevant code, yes. > > There's no mention of "application/xslt+xml" anywhere in our codebase; as > far as I can tell we treat "text/xsl" as the XSLT media type. I noticed that :-) Trying a patch right now... > Is the registration for this type even finalized yet? Yes. See <http://www.iana.org/assignments/media-types/application/index.html>. Also note that httpd's mime.types has it as default media type for the extension ".xslt". > Also note that http://www.w3.org/TR/xslt20/#media-type-registration is > somewhat self-contradictory: > > This media type registration is for XSLT stylesheet modules as described by > the XSLT 2.0 specification, which is located at > http://www.w3.org/TR/xslt20/. > It is also appropriate to use this media type with earlier and later > versions > of the XSLT language. > > and: > > This new type is being registered in order to allow for the expected > deployment of XSLT 2.0 on the World Wide Web, as a first class XML > application. I agree that the second paragraph is slightly confusing and have asked Michael K. for clarification. > I'm having a hard time reconciling the latter with the former; in particular > the latter seems to imply that the type is meant to differentiate XLST 2.0 > from XSLT 1.0 but the former is sort of on the fence about that. > > Note that we do not implement XSLT 2.0 last I checked. Understood. This is a separate problem which is slightly harder to fix than the missing media type support.
(In reply to comment #3) > (In reply to comment #2) > > That's the relevant code, yes. > > > > There's no mention of "application/xslt+xml" anywhere in our codebase; as > > far as I can tell we treat "text/xsl" as the XSLT media type. > > I noticed that :-) Trying a patch right now... > ... I can confirm that adding the type to the "if" clause fixes this issue, plus bug 231395. Should I produce a patch?
Seems reasonable to me; ask Jonas to review?
> > I'm having a hard time reconciling the latter with the former; in particular > > the latter seems to imply that the type is meant to differentiate XLST 2.0 > > from XSLT 1.0 but the former is sort of on the fence about that. > > > > Note that we do not implement XSLT 2.0 last I checked. > > Understood. This is a separate problem which is slightly harder to fix than > the missing media type support. Why is this a separate problem? If this is intended to be a differentiating mechanism between xslt 1 and xslt 2 processors, we'll break it by claiming to support the mimetype while only running a xslt 1 processor, no? In particular, right now you can start your XML document with <?xml-stylesheet href="xslt2stylesheet" type="application/xslt+xml"?> <?xml-stylesheet href="xslt1stylesheet" type="application/xml"?> And have XSLT 1 engines pick up the second PI and XSLT 1 engines the first. Also, should we add this mimetype in the Accept header here: http://mxr.mozilla.org/mozilla-central/source/content/xslt/src/xslt/txMozillaStylesheetCompiler.cpp#499 The answer to that also seems to depend on the meaning of the mimetype.
(In reply to comment #7) > Why is this a separate problem? If this is intended to be a differentiating > mechanism between xslt 1 and xslt 2 processors, we'll break it by claiming > to support the mimetype while only running a xslt 1 processor, no? It is not intended to be a differentiation mechanism. The media type is defined for both XSLT1 and XSLT2. > In particular, right now you can start your XML document with > <?xml-stylesheet href="xslt2stylesheet" type="application/xslt+xml"?> > <?xml-stylesheet href="xslt1stylesheet" type="application/xml"?> No, you can't, because it would pick xslt2stylesheet in IE9. > And have XSLT 1 engines pick up the second PI and XSLT 1 engines the first. > > Also, should we add this mimetype in the Accept header here: > http://mxr.mozilla.org/mozilla-central/source/content/xslt/src/xslt/ > txMozillaStylesheetCompiler.cpp#499 > > The answer to that also seems to depend on the meaning of the mimetype. Yes, I believe this should be added. Will update the patch.
(In reply to comment #8) > ... > > Also, should we add this mimetype in the Accept header here: > > http://mxr.mozilla.org/mozilla-central/source/content/xslt/src/xslt/ > > txMozillaStylesheetCompiler.cpp#499 > > > > The answer to that also seems to depend on the meaning of the mimetype. > > Yes, I believe this should be added. Will update the patch. > ... Wait. I thought this was the Accept header sent when fetching XSLTs. Today it doesn't send "text/xsl", so would we add "application/xslt+xml"? And if this really is for XSLTs, why does it include the XMLTL doctype? Maybe this deserves a separate bug?
The reason it doesn't include "text/xsl" is that that was never an official mimetype. In fact, we we get a response with "Content-Type:text/xsl" then that won't work. The only reason we support "text/xsl" in the PI was that at the time that was the only string that worked in IE and we wanted to make it possible to make a single set of markup work in both browsers. There might also have been a body of documents out there that we wanted to support, though we never pulled any data. At this point I would expect that there is enough of a body of documents that we couldn't pull support though. I don't have an opinion on XMLTL as I don't know what that is.
(In reply to comment #10) > The reason it doesn't include "text/xsl" is that that was never an official > mimetype. In fact, we we get a response with "Content-Type:text/xsl" then > that won't work. I'm usually the first in line to enforce the rules, but allowing text/xsl in the PI, but not allowing it in the returned Content-Type header field really is inconsistent. If we consider text/xsl to be an alias, then we should be consistent about that. > The only reason we support "text/xsl" in the PI was that at the time that > was the only string that worked in IE and we wanted to make it possible to > make a single set of markup work in both browsers. There might also have > been a body of documents out there that we wanted to support, though we > never pulled any data. At this point I would expect that there is enough of > a body of documents that we couldn't pull support though. Indeed. But I'm still not sure about why we need to enumerate this stuff in the Accept header. (I *am* aware about potential problems not sending it at all, but should't "*/*" be sufficient???) > I don't have an opinion on XMLTL as I don't know what that is. I meant to say XHTML. What does it have to do with stylesheets?
(In reply to comment #11) > (In reply to comment #10) > > The reason it doesn't include "text/xsl" is that that was never an official > > mimetype. In fact, we we get a response with "Content-Type:text/xsl" then > > that won't work. > > I'm usually the first in line to enforce the rules, but allowing text/xsl in > the PI, but not allowing it in the returned Content-Type header field really > is inconsistent. If we consider text/xsl to be an alias, then we should be > consistent about that. It's definitely a hack. The question of where to draw the line and have uglyness is subjective. Either be inconsistent about what's in the PI and what's on the network, or let the uglyness of the "text/xsl" type spread further. Personally I prefer things the way they are. I do recall getting one complaint about it back when we first implemented xslt close to 10 years ago, so given that it hasn't been more than that it doesn't seem like something that is bothering site admins. Also note that "xsl" is really something entirely different from "xslt" which would be another reason to not allow "text/xsl" as a real mimetype. > > The only reason we support "text/xsl" in the PI was that at the time that > > was the only string that worked in IE and we wanted to make it possible to > > make a single set of markup work in both browsers. There might also have > > been a body of documents out there that we wanted to support, though we > > never pulled any data. At this point I would expect that there is enough of > > a body of documents that we couldn't pull support though. > > Indeed. > > But I'm still not sure about why we need to enumerate this stuff in the > Accept header. (I *am* aware about potential problems not sending it at all, > but should't "*/*" be sufficient???) Sure, but that also doesn't seem very useful. Granted, content negotiation hasn't really been a smashing success so might be ok to save a few bytes. > > I don't have an opinion on XMLTL as I don't know what that is. > > I meant to say XHTML. What does it have to do with stylesheets? I don't remember why it's in there.
(In reply to comment #12) > It's definitely a hack. The question of where to draw the line and have > uglyness is subjective. Either be inconsistent about what's in the PI and > what's on the network, or let the uglyness of the "text/xsl" type spread > further. If we think that "text/xsl" is ugly (and I do agree) then we should allow people to use the proper type in the type parameter (which is what this bug is about). > Personally I prefer things the way they are. I do recall getting one > complaint about it back when we first implemented xslt close to 10 years > ago, so given that it hasn't been more than that it doesn't seem like > something that is bothering site admins. > > Also note that "xsl" is really something entirely different from "xslt" > which would be another reason to not allow "text/xsl" as a real mimetype. Well, most people think "xsl" = "xslt". I realize that there's XSL-FO, and of course that SHOULD have its own media type (yet another issue, I guess). > > But I'm still not sure about why we need to enumerate this stuff in the > > Accept header. (I *am* aware about potential problems not sending it at all, > > but should't "*/*" be sufficient???) > > Sure, but that also doesn't seem very useful. Granted, content negotiation > hasn't really been a smashing success so might be ok to save a few bytes. I agree that using content negotiation to select different stylesheet languages isn't useful. > I don't remember why it's in there. I collected test results at: http://lists.w3.org/Archives/Public/www-archive/2011May/0013.html ...pursue that as a separate issue?
I'm fine with changing the header to */* if the module owner is. Otherwise I think we should add the new mimetype to the header.
(In reply to comment #14) > I'm fine with changing the header to */* if the module owner is. Otherwise I > think we should add the new mimetype to the header. OK; so I'd propose to add it, and treat cleaning up the Accept header as separate issue.
(In reply to comment #15) > (In reply to comment #14) > > I'm fine with changing the header to */* if the module owner is. Otherwise I > > think we should add the new mimetype to the header. > > OK; so I'd propose to add it, and treat cleaning up the Accept header as > separate issue. ---> https://bugzilla.mozilla.org/show_bug.cgi?id=656251
Keywords: checkin-needed
Assignee: nobody → julian.reschke
http://hg.mozilla.org/mozilla-central/rev/1ea295d5b9c4
Status: UNCONFIRMED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Keywords: dev-doc-needed
Documentation updated: https://developer.mozilla.org/en/XSL_Transformations_in_Mozilla_FAQ Our XSLT documentation is kind of sketchy right now; we should find someone to clean it up at some point, but at least this is now mentioned. Also mentioned on Firefox 7 for developers.
Keywords: dev-doc-needed → dev-doc-complete
"text/xslt+xml" -> "application/xslt+xml"
> "text/xslt+xml" -> "application/xslt+xml" corrected
BTW, Julian, feel free to edit and improve the MDN docs yourself!
OS: Windows 7 → All
Hardware: x86 → All
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: