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)
:
: Andrew Overholt [:overholt]
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 | Splinter Review
Alternate patch (1.28 KB, patch)
2004-09-15 13:25 PDT, Peter Van der Beken [:peterv]
no flags Details | Diff | Splinter Review
Alternatest patch (1.30 KB, patch)
2004-09-15 15:01 PDT, Johnny Stenback (:jst, jst@mozilla.com)
no flags Details | Diff | Splinter 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 | Splinter 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 | Splinter Review

Description User image 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 User image 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 User image Boris Zbarsky [:bz] (still a bit busy) 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 User image 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 User image 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 User image Boris Zbarsky [:bz] (still a bit busy) 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 User image 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 User image 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 User image 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 User image 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 User image 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 User image Johnny Stenback (:jst, jst@mozilla.com) 2004-09-15 13:49:39 PDT
Nope, that does *not* fix the problem :-(
Comment 12 User image 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 User image Boris Zbarsky [:bz] (still a bit busy) 2004-09-15 21:38:02 PDT
Comment on attachment 159026 [details] [diff] [review]
Alternatest patch

r=bzbarsky
Comment 14 User image 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 User image 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 User image 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 User image 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 User image Peter Van der Beken [:peterv] 2004-09-17 06:26:32 PDT
Created attachment 159197 [details] [diff] [review]
Additional patch
Comment 19 User image Boris Zbarsky [:bz] (still a bit busy) 2004-09-17 08:16:26 PDT
Comment on attachment 159197 [details] [diff] [review]
Additional patch

r+sr=bzbarsky
Comment 20 User image 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 User image Ali Ebrahim 2004-09-18 07:57:47 PDT
*** Bug 258776 has been marked as a duplicate of this bug. ***
Comment 22 User image Ali Ebrahim 2005-03-02 04:42:51 PST
*** Bug 249153 has been marked as a duplicate of this bug. ***
Comment 23 User image 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 User image Boris Zbarsky [:bz] (still a bit busy) 2005-07-26 08:38:20 PDT
Yeah, reopening.  We need to finish landing the patches for this bug....
Comment 25 User image Mike Schroepfer 2005-08-09 14:29:21 PDT
Will take patch if checked in but not holding beta for it.
Comment 26 User image Boris Zbarsky [:bz] (still a bit busy) 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 User image Boris Zbarsky [:bz] (still a bit busy) 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.