Status

()

Core
XML
RESOLVED FIXED
13 years ago
12 years ago

People

(Reporter: jst, Assigned: jst)

Tracking

({fixed1.8, regression})

Trunk
x86
Linux
fixed1.8, regression
Points:
---
Bug Flags:
blocking1.7.11 -
blocking1.7.13 -
blocking1.8b5 -

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(4 attachments, 1 obsolete attachment)

(Assignee)

Description

13 years ago
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.
(Assignee)

Comment 1

13 years ago
Created attachment 158827 [details] [diff] [review]
Override the xsl file extension in the ext helper service.
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.
Attachment #158827 - Flags: superreview?(peterv)
Attachment #158827 - Flags: review+
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.
Attachment #158827 - Flags: superreview?(peterv) → superreview+
> 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?
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.
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

13 years ago
I actually think that the right fix is to not ask gnome for mimetypes anymore, if
they have no clue about it.

Comment 8

13 years ago
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>)
(Assignee)

Comment 9

13 years ago
(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.
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.
(Assignee)

Comment 11

13 years ago
Nope, that does *not* fix the problem :-(
(Assignee)

Comment 12

13 years ago
Created attachment 159026 [details] [diff] [review]
Alternatest patch

This I believe is what we want to do here.
(Assignee)

Updated

13 years ago
Attachment #159026 - Flags: superreview?(peterv)
Attachment #159026 - Flags: review?(bzbarsky)
Comment on attachment 159026 [details] [diff] [review]
Alternatest patch

r=bzbarsky
Attachment #159026 - Flags: review?(bzbarsky) → review+
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.
(Assignee)

Comment 15

13 years ago
Created attachment 159114 [details] [diff] [review]
Force local loads to load XML.
Attachment #159114 - Flags: superreview+
Attachment #159114 - Flags: review+
(Assignee)

Comment 16

13 years ago
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.
Attachment #159114 - Flags: approval-aviary?

Comment 17

13 years ago
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.
Attachment #159114 - Flags: approval-aviary? → approval-aviary+
Attachment #159026 - Flags: superreview?(peterv)
Created attachment 159197 [details] [diff] [review]
Additional patch
Attachment #159018 - Attachment is obsolete: true
Attachment #159197 - Flags: superreview?(bzbarsky)
Attachment #159197 - Flags: review?(bzbarsky)
Comment on attachment 159197 [details] [diff] [review]
Additional patch

r+sr=bzbarsky
Attachment #159197 - Flags: superreview?(bzbarsky)
Attachment #159197 - Flags: superreview+
Attachment #159197 - Flags: review?(bzbarsky)
Attachment #159197 - Flags: review+
(Assignee)

Comment 20

13 years ago
"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.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed-aviary1.0, fixed1.7.x
Resolution: --- → FIXED

Updated

13 years ago
Keywords: regression

Comment 21

13 years ago
*** Bug 258776 has been marked as a duplicate of this bug. ***

Comment 22

12 years ago
*** Bug 249153 has been marked as a duplicate of this bug. ***
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.
Yeah, reopening.  We need to finish landing the patches for this bug....
Status: RESOLVED → REOPENED
Flags: blocking1.8b4?
Flags: blocking-aviary1.0.7?
Resolution: FIXED → ---
Component: General → XML
Flags: review+
Keywords: fixed-aviary1.0, fixed1.7.5
Product: Firefox → Core
Version: 1.0 Branch → Trunk
Flags: blocking1.7.11?

Updated

12 years ago
Flags: blocking1.7.12?
Flags: blocking1.7.11?
Flags: blocking1.7.11-

Comment 25

12 years ago
Will take patch if checked in but not holding beta for it.
Flags: blocking1.8b4?
Flags: blocking1.8b4-
Flags: blocking1.7.12?
Flags: blocking1.7.12-
Comment on attachment 159197 [details] [diff] [review]
Additional patch

We need to get this fix actually landed...
Attachment #159197 - Flags: approval1.8b4?

Updated

12 years ago
Attachment #159197 - Flags: approval1.8b4? → approval1.8b4+
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.
Status: REOPENED → RESOLVED
Last Resolved: 13 years ago12 years ago
Keywords: fixed1.8
Resolution: --- → FIXED
Flags: blocking-aviary1.0.8?
You need to log in before you can comment on or make changes to this bug.