Last Comment Bug 259206 - XML prettyprint broken
: XML prettyprint broken
Status: RESOLVED FIXED
: fixed1.8, regression
Product: Core
Classification: Components
Component: XML (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
:
Mentors:
http://off.net/~rgb/none.xml
: 249153 258776 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2004-09-13 19:46 PDT by Johnny Stenback (:jst, jst@mozilla.com)
Modified: 2005-10-21 07:31 PDT (History)
12 users (show)
asa: blocking1.7.11-
mtschrep: blocking1.7.13-
mtschrep: blocking1.8b5-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Override the xsl file extension in the ext helper service. (1.09 KB, patch)
2004-09-13 19:47 PDT, Johnny Stenback (:jst, jst@mozilla.com)
peterv: superreview+
Details | Diff | Review
Alternate patch (1.28 KB, patch)
2004-09-15 13:25 PDT, Peter Van der Beken [:peterv]
no flags Details | Diff | Review
Alternatest patch (1.30 KB, patch)
2004-09-15 15:01 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Review
Force local loads to load XML. (2.51 KB, patch)
2004-09-16 11:06 PDT, Johnny Stenback (:jst, jst@mozilla.com)
peterv: superreview+
asa: approval‑aviary+
Details | Diff | Review
Additional patch (1.21 KB, patch)
2004-09-17 06:26 PDT, Peter Van der Beken [:peterv]
bzbarsky: superreview+
asa: approval1.8b4+
Details | Diff | Review

Description Johnny Stenback (:jst, jst@mozilla.com) 2004-09-13 19:46:41 PDT
Using todays nightly build (and older ones too, not sure when it broke), XML
pretty printing isn't working on linux (Fedora Core 2, Gtk2). I tracked this
down to the fact that the system maps .xsl files to "text/x-xslt", and we don't
know anything about that mimetype, and thus can't apply the transformation that
we apply when doing pretty printing of XML. I've got a oneliner that overrides
the mimetype for .xsl files in Firefox that I'll attach for others to comment on.
Comment 1 Johnny Stenback (:jst, jst@mozilla.com) 2004-09-13 19:47:26 PDT
Created attachment 158827 [details] [diff] [review]
Override the xsl file extension in the ext helper service.
Comment 2 Boris Zbarsky [:bz] 2004-09-13 20:00:18 PDT
Comment on attachment 158827 [details] [diff] [review]
Override the xsl file extension in the ext helper service.

r=bzbarsky.  Peterv, want to sr?  This basically makes us not ask the OS for
the "xsl" extension -- any time we need to map that extension to a MIME type we
will just get text/xml with this patch.
Comment 3 Peter Van der Beken [:peterv] 2004-09-14 01:42:45 PDT
Comment on attachment 158827 [details] [diff] [review]
Override the xsl file extension in the ext helper service.

I'm assuming this is only for local files.
Comment 4 Christian :Biesinger (don't email me, ping me on IRC) 2004-09-14 04:06:34 PDT
> the system maps .xsl files to "text/x-xslt"

(this is actually a duplicate, of a bug that got marked invalid or wontfix or
something. bug 259100)


Wouldn't a better fix be to set a content type hint for XSLT channels, so that
the helperappservice isn't even involved?
Comment 5 Boris Zbarsky [:bz] 2004-09-14 06:43:34 PDT
That's true... If we set the type on the channel before calling Open/AsyncOpen,
it will be treated by necko as a hint to be used if the protocol itself does not
provide a type.
Comment 6 Peter Van der Beken [:peterv] 2004-09-14 08:10:58 PDT
So we want to call SetContentType(NS_LITERAL_CSTRING("text/xml")) at least
somewhere around
http://lxr.mozilla.org/seamonkey/source/extensions/transformiix/source/xslt/txMozillaStylesheetCompiler.cpp#523
Trying to think about other load paths that might want it.
Comment 7 Axel Hecht 2004-09-14 08:19:39 PDT
I actually think that the right fix is to not ask gnome for mimetypes anymore, if
they have no clue about it.
Comment 8 Anne (:annevk) 2004-09-14 08:27:18 PDT
Since text/xml will be deprecated somewhere in the future, can't we change this
to application/xml? (text/xml has some charset issues too.) If the patch is
allowed, that is.

(<http://lists.w3.org/Archives/Public/www-html/2004Jul/0004.html>)
Comment 9 Johnny Stenback (:jst, jst@mozilla.com) 2004-09-14 12:56:08 PDT
(In reply to comment #8)
> Since text/xml will be deprecated somewhere in the future, can't we change this
> to application/xml? (text/xml has some charset issues too.) If the patch is
> allowed, that is.
> 
> (<http://lists.w3.org/Archives/Public/www-html/2004Jul/0004.html>)

It really doesn't matter in this case, and few will ever see what mimetype we
internally map .xsl files to.
Comment 10 Peter Van der Beken [:peterv] 2004-09-15 13:25:36 PDT
Created attachment 159018 [details] [diff] [review]
Alternate patch

Jst: could you try out this patch? I haven't been able to reproduce this yet so
no idea if it fixes the bug.
Comment 11 Johnny Stenback (:jst, jst@mozilla.com) 2004-09-15 13:49:39 PDT
Nope, that does *not* fix the problem :-(
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2004-09-15 15:01:35 PDT
Created attachment 159026 [details] [diff] [review]
Alternatest patch

This I believe is what we want to do here.
Comment 13 Boris Zbarsky [:bz] 2004-09-15 21:38:02 PDT
Comment on attachment 159026 [details] [diff] [review]
Alternatest patch

r=bzbarsky
Comment 14 Peter Van der Beken [:peterv] 2004-09-16 00:50:44 PDT
Comment on attachment 159026 [details] [diff] [review]
Alternatest patch

Hmm, will that do the right thing? The aChannelIsSync argument when calling
nsSyncLoader::LoadDocument will be false. I think we should make
LoadLocalDocument pass true for aForceToXML.
Comment 15 Johnny Stenback (:jst, jst@mozilla.com) 2004-09-16 11:06:16 PDT
Created attachment 159114 [details] [diff] [review]
Force local loads to load XML.
Comment 16 Johnny Stenback (:jst, jst@mozilla.com) 2004-09-16 12:22:18 PDT
Comment on attachment 159114 [details] [diff] [review]
Force local loads to load XML.

This attachment is now checked in on the trunk, requesting branch approval.
Comment 17 Asa Dotzler [:asa] 2004-09-16 20:51:05 PDT
Comment on attachment 159114 [details] [diff] [review]
Force local loads to load XML.

a=asa for branch checkin. Did this also break on the 1.7 branch? If so, it
should be landed there too.
Comment 18 Peter Van der Beken [:peterv] 2004-09-17 06:26:32 PDT
Created attachment 159197 [details] [diff] [review]
Additional patch
Comment 19 Boris Zbarsky [:bz] 2004-09-17 08:16:26 PDT
Comment on attachment 159197 [details] [diff] [review]
Additional patch

r+sr=bzbarsky
Comment 20 Johnny Stenback (:jst, jst@mozilla.com) 2004-09-17 08:49:07 PDT
"Force local loads to load XML." landed on the aviary and 1.7 branches (since it
was broken on the 1.7 branch too). Marking FIXED.
Comment 21 Ali Ebrahim 2004-09-18 07:57:47 PDT
*** Bug 258776 has been marked as a duplicate of this bug. ***
Comment 22 Ali Ebrahim 2005-03-02 04:42:51 PST
*** Bug 249153 has been marked as a duplicate of this bug. ***
Comment 23 Joe Drew (not getting mail) 2005-07-26 08:35:57 PDT
Is this still an issue? Attachment 159197 [details] [diff] never got checked in to either trunk
or aviary branch, and I know that I've seen this problem on Fedora Core N, N >= 2.
Comment 24 Boris Zbarsky [:bz] 2005-07-26 08:38:20 PDT
Yeah, reopening.  We need to finish landing the patches for this bug....
Comment 25 Mike Schroepfer 2005-08-09 14:29:21 PDT
Will take patch if checked in but not holding beta for it.
Comment 26 Boris Zbarsky [:bz] 2005-08-14 18:23:46 PDT
Comment on attachment 159197 [details] [diff] [review]
Additional patch

We need to get this fix actually landed...
Comment 27 Boris Zbarsky [:bz] 2005-08-19 10:39:36 PDT
Fixed again, trunk and 1.8 branch.  Not quite fixed on 1.7, but I don't think we
want to mess with that at this point.

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