77080 - Show application/x-javascript in browser window instead of trying to download

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, enhancement)

Description

[Jesse Ruderman]

Steps to reproduce:
1. Type http://www.cs.hmc.edu/~jruderma/mozilla/proptree/proptree.js into the 
location bar and press enter.

Result: prompted to download the javascript file.

Expected result: js file should be shown in the browser window as if it had 
been sent as text/plain.

Comment 1

[Jesse Ruderman]

As Neil pointed out in bug 67647, this happens often when clicking links in the 
JS console.

Comment 2

[Boris Zbarsky [:bzbarsky]]

OK....  I have this working in my tree.  And when I created a patch and applied
it to a different tree, it did not work in the different tree.  Which is very
odd, since the trees are now identical... perhaps the problem is debug vs
optimized...  Going to work on creating a working patch for this.

Comment 3

[Boris Zbarsky [:bzbarsky]]

OK.  I have a patch. If I apply it to a tree and build debug it works. If I
build optimized it does not work.... Attaching said patch.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: patch that will work for debug builds

Comment 5

[Boris Zbarsky [:bzbarsky]]

This isn't really a networking bug... over to parser, since that's where all the
action seems to need to take place.

Comment 6

[Boris Zbarsky [:bzbarsky]]

resetting milestone

Comment 7

[Tim Powell]

Does this patch make .js files show up in the browser window? Or in a view 
source window? Your comments in bug 77337 made me a little confused about which 
it does. Personally I'd love to see all text/* (such as text/javascript, 
text/css, etc.) and application/x-javascript files show up just like text/plain 
files in both the regular browser window and view source.

Comment 8

[Boris Zbarsky [:bzbarsky]]

It does both.  I figured I might as well fix both at once.  Especially since the
JS console uses viewsource: urls to point to JS files.

Comment 9

[harishd]

CCing Jud,Rick,and Chak.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Working patch

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment 2 [details] [diff] fixes the problem for me.  It seems to require a clobber build to
take effect, though.

This patch also fixes many of the problems seen in bug 77337.

Reviews?

Comment 12

[Boris Zbarsky [:bzbarsky]]

er.. I suppose that should be "attachment 32255 [details] [diff] [review]"

Comment 13

[Keyser Sose]

2nd patch r=ksosez looks good to me.

Comment 14

[harishd]

Is it required to include mime types in contentDLF for view-source?

Comment 15

[harishd]

Boris: Will we be able to display JS,CSS,etc., in color if required? 

Comment 16

[Judson Valeski]

chak would know best about contentDLF requirements for view-source. I *doubt* 
it's required.

Comment 17

[Boris Zbarsky [:bzbarsky]]

I don't believe the contentDLF changes are required (I tried taking them out and 
things still work).  They were initially added because I was trying to make 
application/x-javascript as much like text/css (which _was_ displaying in the 
browser window) as possible.

The current setup just shows CSS/JS as plaintext even in viewsource.  If I make 
the nsViewSourceHTML parser handle them instead of the CNavDTD one, I still can't 
do anything useful with them because all that gets created is some number of 
#text nodes depending on file size.

If we can figure out a way to send the output of the CSS/JS parsers through the 
whole viewsource process we could do syntax highlighting on it.

The thing is, for JS, very often the reason we are viewing the file is that it 
has a parse error in it and the JS console told us so.  Under those 
circumstances, how reliable would the output of the parser be?

For CSS, the parser is supposed to drop "broken" rules.  Again, if we are viewing 
a CSS file this may make it difficult to discover errors since they will be 
hidden.

In either case, unless it looks like syntax highlighting for JS/CSS can be done 
in the near term, I would argue for just getting plaintext viewsource to work for 
them for now.  This is especially important for JS files, since as I said the JS 
console uses view-source: urls to point to files that errors occur in and 
currently that output is fairly useless.

It may still be worth removing the lines from CNavDTD.cpp that make it handle 
view source of CSS/JS.  I think things still work fine if that's handled in 
nsViewSourceHTML.cpp.  I will experiment a bit and attach a modified patch.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attachment: better patch

Comment 19

[Boris Zbarsky [:bzbarsky]]

OK.  Attachment 32363 [details] [diff] does not touch nsContentDLF.  It also uses
nsViewSourceHTML to display view-source: urls with JS or CSS mimetypes, so
whenever we get around to having not just #text tokens in the parse tree for
those we will be able to do highlighting.

How does that look?

Comment 20

[Boris Zbarsky [:bzbarsky]]

Er.. disregard that third attachment.  Attached wrong file.  Attaching real
patch...

Comment 21

[Boris Zbarsky [:bzbarsky]]

Attachment: better patch, for real this time

Comment 22

[harishd]

*** Bug 77499 has been marked as a duplicate of this bug. ***

Comment 23

[harishd]

Boris: I'm giving this bug to you since you have been doing all the work.

Note: Bug 77499 is not really a duplicate but should get fixed by Boris's patch.

Comment 24

[Johnny Stenback (:jst)]

Huh? Why are we attempting to be able to open .cpp and .rtf files? It's not like
we really know what to do with those anyway (yes, I know they're text files, but
if we go down this path then we'll end up having to maintain a list of all
mimtypes that are actually text files). JS and CSS files I can understand since
they're kinda relevant to mozilla. I'd say remove all references to all
variations of the text/cpp and text/rtf mimetypes here (are those even official
mimetypes?), and then check this in. sr=jst if this is OK with Harish.

Comment 25

[Boris Zbarsky [:bzbarsky]]

text/cpp can certainly go.  I was wondering about that one myself.  text/rtf is
a different issue -- apparently we _do_ handle rtf internally.  Albeit poorly. 
Harish?

Comment 26

[Simon Fraser [no longer active]]

Composer also needs to do something sensible with these kinds of files.

Comment 27

[harishd]

AFAIK, text/rtf is an offical mimetype but text/cpp is definitely not and should
be yanked. As Boris pointed out, there is some implementation, that hasn't
evolved, to handle RTF documents which IMO adds no value to Mozilla. I think we
should say "so long" to rtf mimetype and should probably remove CRTFDTD.* from
the build. Comments?

Comment 28

[Boris Zbarsky [:bzbarsky]]

I agree that we should remove handling of text/rtf.  This would have the
side-effect of fixing bug 45257, bug 45397, and bug 76757. I would leave the
CRTFDTD.* files for now, though... This way if someone wants to add support for
RTF later (eg for a mozilla-based word-processing application) they have some
place to start.

I've filed bug 78458 on the text/rtf issue.  Should we try to do it all
together, or just get the javascript and getting rid of text/cpp part of this in
and then deal with text/rtf separately?

Comment 29

[harishd]

text/rft should be handled separately.

Comment 30

[Boris Zbarsky [:bzbarsky]]

OK.  Attaching patch that just fixes this and removes text/cpp.  text/rtf to be
handled in bug 78458.  Reviews?

Comment 31

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch to just fix the JS/CSS/viewsource stuff

Comment 32

[harishd]

Boris: Why is CNavDTD still involved?

Comment 33

[Boris Zbarsky [:bzbarsky]]

CNavDTD is involved because with my changes nsViewSourceHTML::CanParse will only
return an eValidDetect if aParserContext.mParserCommand is eViewSource.  So when
we are not doing viewsource and are just loading in the browser window, CNavDTD
handles the CSS/JS as just plaintext.

I suppose we could always have nsViewSourceHTML handle those mime types... but
it that has its own problems (eg bug 77499)

Comment 34

[harishd]

patch looks good. r=harishd

Comment 35

[Boris Zbarsky [:bzbarsky]]

Bug 78568 filed on composer doing something sensible with these files.

Comment 36

[Boris Zbarsky [:bzbarsky]]

jst, does your sr still apply?

Comment 37

[Johnny Stenback (:jst)]

Yup, sure does. Harish, please please please get rid of all references to rtf in
the parser, ok?

Comment 38

[timeless]

fix checked in

Comment 39

[Boris Zbarsky [:bzbarsky]]

*** Bug 78696 has been marked as a duplicate of this bug. ***

Comment 40

[Doron Rosenberg (IBM)]

*** Bug 78886 has been marked as a duplicate of this bug. ***

Comment 41

[bsharma]

Verified on:
build: 2001-05-21-11-Mtrunk
platform: WinNT

The js file is desplayed as a text file in the browser window.