Closed
Bug 77080
Opened 23 years ago
Closed 23 years ago
Show application/x-javascript in browser window instead of trying to download
Categories
(Core :: DOM: HTML Parser, enhancement)
Core
DOM: HTML Parser
Tracking
()
VERIFIED
FIXED
mozilla0.9.1
People
(Reporter: jruderman, Assigned: bzbarsky)
References
()
Details
Attachments
(5 files)
2.50 KB,
patch
|
Details | Diff | Splinter Review | |
4.92 KB,
patch
|
Details | Diff | Splinter Review | |
1.60 KB,
patch
|
Details | Diff | Splinter Review | |
4.18 KB,
patch
|
Details | Diff | Splinter Review | |
5.09 KB,
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•23 years ago
|
||
As Neil pointed out in bug 67647, this happens often when clicking links in the JS console.
Assignee | ||
Comment 2•23 years ago
|
||
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.
OS: Windows 98 → All
Hardware: PC → All
Assignee | ||
Comment 3•23 years ago
|
||
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.
Assignee | ||
Comment 4•23 years ago
|
||
Assignee | ||
Comment 5•23 years ago
|
||
This isn't really a networking bug... over to parser, since that's where all the action seems to need to take place.
Assignee: neeti → harishd
Component: Networking → Parser
QA Contact: tever → bsharma
Comment 7•23 years ago
|
||
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.
Assignee | ||
Comment 8•23 years ago
|
||
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.
Assignee | ||
Comment 10•23 years ago
|
||
Assignee | ||
Comment 11•23 years ago
|
||
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?
Assignee | ||
Comment 12•23 years ago
|
||
er.. I suppose that should be "attachment 32255 [details] [diff] [review]"
Comment 13•23 years ago
|
||
2nd patch r=ksosez looks good to me.
Comment 14•23 years ago
|
||
Is it required to include mime types in contentDLF for view-source?
Comment 15•23 years ago
|
||
Boris: Will we be able to display JS,CSS,etc., in color if required?
Comment 16•23 years ago
|
||
chak would know best about contentDLF requirements for view-source. I *doubt* it's required.
Assignee | ||
Comment 17•23 years ago
|
||
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.
Assignee | ||
Comment 18•23 years ago
|
||
Assignee | ||
Comment 19•23 years ago
|
||
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?
Assignee | ||
Comment 20•23 years ago
|
||
Er.. disregard that third attachment. Attached wrong file. Attaching real patch...
Assignee | ||
Comment 21•23 years ago
|
||
Comment 22•23 years ago
|
||
*** Bug 77499 has been marked as a duplicate of this bug. ***
Comment 23•23 years ago
|
||
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.
Assignee: harishd → bzbarsky
Status: ASSIGNED → NEW
Assignee | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.1
Comment 24•23 years ago
|
||
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.
Assignee | ||
Comment 25•23 years ago
|
||
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•23 years ago
|
||
Composer also needs to do something sensible with these kinds of files.
Comment 27•23 years ago
|
||
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?
Assignee | ||
Comment 28•23 years ago
|
||
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•23 years ago
|
||
text/rft should be handled separately.
Assignee | ||
Comment 30•23 years ago
|
||
OK. Attaching patch that just fixes this and removes text/cpp. text/rtf to be handled in bug 78458. Reviews?
Assignee | ||
Comment 31•23 years ago
|
||
Comment 32•23 years ago
|
||
Boris: Why is CNavDTD still involved?
Assignee | ||
Comment 33•23 years ago
|
||
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•23 years ago
|
||
patch looks good. r=harishd
Assignee | ||
Comment 35•23 years ago
|
||
Bug 78568 filed on composer doing something sensible with these files.
Assignee | ||
Comment 36•23 years ago
|
||
jst, does your sr still apply?
Comment 37•23 years ago
|
||
Yup, sure does. Harish, please please please get rid of all references to rtf in the parser, ok?
Comment 38•23 years ago
|
||
fix checked in
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Keywords: mozilla0.9.1,
review
Resolution: --- → FIXED
Assignee | ||
Comment 39•23 years ago
|
||
*** Bug 78696 has been marked as a duplicate of this bug. ***
Comment 40•23 years ago
|
||
*** Bug 78886 has been marked as a duplicate of this bug. ***
Comment 41•23 years ago
|
||
Verified on: build: 2001-05-21-11-Mtrunk platform: WinNT The js file is desplayed as a text file in the browser window.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•