Last Comment Bug 655238 - stylesheet link header doesn't support official XSLT media type
: stylesheet link header doesn't support official XSLT media type
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: XSLT (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla7
Assigned To: Julian Reschke
:
Mentors:
http://greenbytes.de/tech/tc/httplink...
Depends on:
Blocks: 231395
  Show dependency treegraph
 
Reported: 2011-05-06 05:16 PDT by Julian Reschke
Modified: 2011-08-09 01:12 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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)
2011-05-06 07:58 PDT, Julian Reschke
no flags Details | Diff | 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)
2011-05-11 06:21 PDT, Julian Reschke
jonas: review+
Details | Diff | Review

Description Julian Reschke 2011-05-06 05:16:43 PDT
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.
Comment 2 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-05-06 07:10:14 PDT
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.
Comment 3 Julian Reschke 2011-05-06 07:21:08 PDT
(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.
Comment 4 Julian Reschke 2011-05-06 07:47:37 PDT
(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?
Comment 5 Boris Zbarsky [:bz] (Out June 25-July 6) 2011-05-06 07:49:32 PDT
Seems reasonable to me; ask Jonas to review?
Comment 6 Julian Reschke 2011-05-06 07:58:08 PDT
Created attachment 530625 [details] [diff] [review]
add "application/xslt+xml" as media type for XSLT, affects stylesheet loading both from processing instruction and HTTP Link header field
Comment 7 Jonas Sicking (:sicking) PTO Until July 5th 2011-05-09 00:38:53 PDT
> > 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.
Comment 8 Julian Reschke 2011-05-09 00:47:30 PDT
(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.
Comment 9 Julian Reschke 2011-05-09 06:39:13 PDT
(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?
Comment 10 Jonas Sicking (:sicking) PTO Until July 5th 2011-05-09 13:17:42 PDT
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.
Comment 11 Julian Reschke 2011-05-09 13:55:10 PDT
(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?
Comment 12 Jonas Sicking (:sicking) PTO Until July 5th 2011-05-09 17:44:08 PDT
(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.
Comment 13 Julian Reschke 2011-05-10 06:04:52 PDT
(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?
Comment 14 Jonas Sicking (:sicking) PTO Until July 5th 2011-05-10 09:08:12 PDT
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.
Comment 15 Julian Reschke 2011-05-10 09:41:31 PDT
(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.
Comment 16 Jonas Sicking (:sicking) PTO Until July 5th 2011-05-10 09:47:56 PDT
Sounds good
Comment 17 Julian Reschke 2011-05-11 04:46:18 PDT
(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
Comment 18 Julian Reschke 2011-05-11 06:21:38 PDT
Created attachment 531611 [details] [diff] [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
Comment 19 Dão Gottwald [:dao] 2011-05-28 05:50:33 PDT
http://hg.mozilla.org/mozilla-central/rev/1ea295d5b9c4
Comment 20 Eric Shepherd [:sheppy] 2011-08-05 13:33:23 PDT
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.
Comment 21 Julian Reschke 2011-08-05 23:33:26 PDT
"text/xslt+xml" -> "application/xslt+xml"
Comment 22 j.j. 2011-08-06 01:54:37 PDT
> "text/xslt+xml" -> "application/xslt+xml"
corrected
Comment 23 j.j. 2011-08-06 02:05:24 PDT
BTW, Julian, feel free to edit and improve the MDN docs yourself!

Note You need to log in before you can comment on or make changes to this bug.