Closed Bug 77337 Opened 24 years ago Closed 22 years ago

[FIX]view-source: does not allow viewing of many text files

Categories

(Core :: DOM: HTML Parser, defect, P2)

Product:
Core
Component:
DOM: HTML Parser
Platform:
x86
Other
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.2beta

People

(Reporter: tpowellmoz, Assigned: bzbarsky)

References

Details

(Keywords: arch)

Attachments

(1 file, 1 obsolete file)

Minor tweak to handle servers sending us application/x-view-source content (why??)
22.50 KB, patch
jst
: review+
darin.moz
: superreview+
Details | Diff | Splinter Review 
On the 2001042304 WinNT build, you can add view-source: in front of URLs to 
attempt to view the source of a page. However, it apparently only supports 
viewing of HTML files. JavaScript files (.js), Cascading Style Sheets (.css), 
XML and other files that have a mime type of text/something get prompted to 
download. This severely limits the usefulness of view source.

Expected Result:
view-source: of a text file should be shown.
Actual Result:
Mozilla prompts for download and does not show the source.
Cc:ing Vidur, Jud and Rick for thier input.
Blocks: 76671
bug 76671 is about viewing source of text/sgml specifically.

I just played with this. XML files workforme -- I get the source.  We _do_ fail
for text/css and text/plain.  As for .js, that should be served as
application/x-javascript.... so it's not really a text type.  But we could try
to show it anyway, I suppose....

The relevant code seems to be at
http://lxr.mozilla.org/seamonkey/source/htmlparser/src/nsViewSourceHTML.cpp#390

and

http://lxr.mozilla.org/seamonkey/source/htmlparser/src/nsParser.cpp#1065

Reading that, it _looks_ like CSS and text/plain should work, being valid
(though not primary) detects....
In Communicator view-source: allowed you to view ANYTHING as text.

Of course, it would still attempt to highlight HTML tags :-)
Just to clarify, I tried to view an XSL file. This caused Mozilla to prompt for 
download and reported that the mimetype was text/xml. So I couldn't view this 
type of xml file. Regular xml files may be okay.

JavaScript as application/x-javascript? I've seen it as text/javascript quite a 
bit. In either case, it's a text file and should be viewable.

I'm sure there's other types that should also be supported. I just picked the 
main ones that I miss.

Added 4xp keyword. As Neil points out, this worked in communicator 4 with 
pretty much any text file. Proposing for mozilla 0.9.1 since the lack of this 
feature makes it more difficult for web developers.
Keywords: 4xp, mozilla0.9.1
I think this should be fixed soon too.  Is it well-assigned?  Maybe someone
could debug a bit and see where things go awry?  I'll try to get to it after the
0.9 madness ends.

/be
This should be easy to fix. Will take a look.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.1
*** Bug 77413 has been marked as a duplicate of this bug. ***
getting the downloading/helper app dialog also occurs when trying to view source
of image files [might be a different bug?].
Severity: normal → major
Keywords: regression
OS: other → All
Hardware: PC → All
The docshell is failing to create a content viewer for 
conten-type:application/x-javascript.

Rick?
Severity: major → normal
Keywords: regression
OS: All → other
Hardware: All → PC
I think the reason the "regression" keyword was there is that text/plain at
least used to work and no longer does.
Keywords: regression
I have a patch attached to bug 77080 that allows loading of text/plain (again),
text/css, text/javascript, and application/x-javascript in view source. 
However, that only works in a debug build.  In an optimized build it fails for
some reason for application/x-javascript.

Note that a fix that will work for all text/* types will have to be a little
more clever than what we do now.
Boris: Do you think I should mark this bug a dup of 77080?
Well, to me this bug sounded like an rfe to let viewsource of _all_ text/*
types...  tpowell@databeam.com, is that the case?  If not, then it's a dup --
bug 77080 implements viewing text/plain, various xml/xul types, JS, and CSS in
viewsource.
Boris: Yes, this bug is about view-source support for all text/* files. XSL 
files (text/xml) are not currently shown, for example. View-source should 
support pretty much any text file, such as program source code (perl .pl, .pm, 
C++ .cpp, .c, etc.) Of course, most of these should also display in the browser 
as plain text, so view-source should be basically the same thing.
Adding dependecy.  Should be looked at once 77080 is fixed.
Depends on: 77080
Target Milestone: mozilla0.9.1 → mozilla0.9.2
OK.  bug 77080 is fixed now.  What sort of files do we currently _not_ load in
viewsource that we should?
*** Bug 82369 has been marked as a duplicate of this bug. ***
*** Bug 82182 has been marked as a duplicate of this bug. ***
Priority: -- → P2
*** Bug 86090 has been marked as a duplicate of this bug. ***
Target Milestone: mozilla0.9.2 → mozilla1.0
I don't know the best place to mention it but I would like the view source
window to work for mail messages as well. I tried changing line 446 of
mailCommands.js from getBrowserURL() to
"chrome://navigator/content/viewSource.xul" but I just got a wierd invalid MIME
type dialog - Mozilla didn't have a clue :-(
neil, what mime type did it come up with for the message?
*** Bug 91666 has been marked as a duplicate of this bug. ***
*** Bug 92187 has been marked as a duplicate of this bug. ***
Boris: message/rfc822; x-view-type=view-source
Another example: bugzilla buglists! e.g.

http://bugzilla.mozilla.org/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&email1=&emailtype1=substring&emailassigned_to1=1&email2=greenrd%40hotmail.com&emailtype2=substring&emailreporter2=1&bugidtype=include&bug_id=&changedin=&votes=&chfieldfrom=&chfieldto=Now&chfieldvalue=&short_desc=&short_desc_type=allwordssubstr&long_desc=&long_desc_type=allwordssubstr&bug_file_loc=&bug_file_loc_type=allwordssubstr&status_whiteboard=&status_whiteboard_type=allwordssubstr&keywords=&keywords_type=anywords&field0-0-0=noop&type0-0-0=noop&value0-0-0=&cmdtype=doit&order=Reuse+same+sort+as+last+time

lynx reports headers as:

HTTP/1.0 200 OK
Date: Mon, 20 Aug 2001 23:08:05 GMT
Server: Apache/1.3.19 (Unix) mod_throttle/3.1.2
Content-disposition: inline; filename=bugzilla_bug_list.html
Set-Cookie:
LASTORDER=bugs.bug_status%2C%20bugs.priority%2C%20map_assigned_to.login_name%2C%20bugs.bug_id
; path=/; expires=Sun, 30-Jun-2029 00:00:00 GMT
Set-Cookie: BUGLIST=95941:95942:96131:91968:96124:81772:96120:89309:76799:92329
Content-Type: text/html
X-Cache: MISS from wwwcache.lancs.ac.uk
X-Cache-Lookup: MISS from wwwcache.lancs.ac.uk:8080
Proxy-Connection: close

Content-type is text/html so it may be necessary to reopen bug 86090 or file a
new bug for this case. However, I'm no expert - please tell me what to do.
adding keyword mostfreq
Keywords: mostfreq
Robin: probably bug 76816 is what you're seeing.
QA Contact: bsharma → moied
The original bug is a dup of bug #56898.
Cannot view source of eBay auction item pages, such as:

   <http://cgi.ebay.com/aw-cgi/eBayISAPI.dll?ViewItem&item=1654096985>

Type is:

   application/x-unknown-content-type

Is this the same bug?
the reason that this is failing is because the content type is *not*
"allication/x-unknown-content-type" it is  "application/x-unknown-content-type;
x-view-type=view-source"

unfortunately, we *only* have a content converted registered to handle
x-unk-content-type *not* with the 'x-view-type' extension :-(

So, somehow we need to educate the stream converter service about this
content-type syntax.  

-- rick


Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
Blocks: 64933
When I try to view source of a message whose message-id ends in .com, then
choose view-source from the context menu, Mozilla asks me to save the .com file :-)
*** Bug 118096 has been marked as a duplicate of this bug. ***
As this was AutoMoved (tm) to a post 1.0 milestone, and Brendan Eich thought (8
months ago) it should be fixed "soon", I think this should be considered for
retargeting. (Like, 0.9.8 :)
Removing regression keyword since the regression was fixed in bug 77080.

Note that Brendan asked whether this bug is well-assigned.  It is not, in my 
opinion.  The problem is that the current architecture requires us to list every 
type that we have an internal handler for in nsContentDLF.cpp.  For example, 
consider a file at url "http://foo" served as text/x-csrc.  We can't handle this 
type internally, so it's not listed in nsContentDLF.cpp.  The request in this 
bug is that a link to "view-source:http://foo" (the only way to view source of 
such a page that I can find) do something reasonable.  This is only possible if 
we change the type set on the channel in nsContentDLF to text/plain for 
text/* types for the view-source command (I'll try this next week and see if 
it works) or if we change the way the whole content handler setup works.

We could probably just add "message/rfc822; x-view-type=view-source" to the list 
 and let the parser know about it and it would work (again, I'll try this next 
week).

I'm guessing that Brendan was asking for this to be fixed "soon" because doing 
it right will take some rearchitecting...
Keywords: regression → arch
On the ebay URL posted in comment 29, I notice the server sends no
Content-Length or Content-Type (and even more screwed up, sends Server: and
Date: twice). It does set Content-Type via a meta, though:

<!--A http-equiv="Content-Type" content="text/html; charset=UTF-1-->
<META HTTP-EQUIV="Content-Type" CONTENT="text/html; CHARSET=ISO8859-1">
<meta http-equiv="Expires" content="Sat, 05 Jan 2002, 05:14:19 GMT">

(With an invalid charset... wow, ebay web monkeys are pretty lame). Is it
possible to use the meta header to set the Content-Type for whereever view
source reads it from. Not a final fix, but would eliminate some cases of this bug.
> Is it possible to use the meta header to set the Content-Type

Not unless we decide that the page is text/html and parse it as such.  :)  At 
the moment, this decision is made by the unknown content decoder (for 
display purposes) which only kicks in for application/x-unknown-content-type.  
Then comment #30 takes effect.  The unknown decoder needs to know about view 
source in the current setup....
bzbarsky, do you want to take this bug?

/be
Frankly, I have neither the time nor the knowledge of the code required to fix 
this in full generality... I can do things a mimetype at a time like bug 77080, 
but I would be unable to work on a general solution till well into summer, if 
then.
What would happen if in line 230 of nsContentDLF.cpp the channel's content type
was set to text/plain instead of the previous content type? Would we then lose
out on syntax highlighting?
Also I think resetting the content type in line 231 is a bit strange, is this
just to avoid duplicating the code in line 238?
> Would we then lose out on syntax highlighting?

Yes.  In fact the only reason the type is not text/plain to start with in the
view-source case is so that syntax highlighting will work.

So one possible solution is to scan for the type in our "list of known types" in
the view-source case and then just set the type to text/plain if the type is not
found.  This will break things like view-source:view-source:http://foo, but that
does not work anyway, so....  Seem like a reasonable course of action?
Blocks: 86835
Target Milestone: mozilla1.0.1 → mozilla1.0
> Seem like a reasonable course of action?

To me, it does. But what do I know...
*** Bug 126154 has been marked as a duplicate of this bug. ***
Another side-effect of this bug: because MailNews can't use a view source window
it actually opens a browser window. Turn off the pref to hide the tab bar when
only one tab is open, then do a view source of a message :-)
Looks like we do handle .js/.css/.xml/.html. That covers majority of the used
cases.
Keywords: nsbeta1 → nsbeta1-
Target Milestone: mozilla1.0 → mozilla1.2
Is this also the bug that prevents view source from working on HTML files over
file:? That's pretty severe for page development and the nsbeta1 might need to
be reconsidered if that needs to be taken into account.
Jeremy, that falls under comment 30.   Those issues are better tracked by bug
86835 which is more clealy focused on the unknown content decoder.
Soooo, this would be fixed, or at least workable around, by either of the following:
* an option to turn off syntax highlighting, and just see everything as text/plain
* an option to pass 'view source' to an external text/plain viewer or editor
(see bug #8589).

Since either of these would also provide a workaround for a large collection of
other bugs, maybe I'll take a pass at implementation of the latter... looks like
there aren't too many code paths to learn...


> * an option to turn off syntax highlighting, and just see everything as
> text/plain

The option to turn off highlighting already exists and even has UI.  Making it 
just view as text/plain is simple, though it would screw up HTML pages that 
depend on <meta> tags for charset info.

> * an option to pass 'view source' to an external text/plain viewer or editor
> (see bug #8589).

This is what I've been thinking of too...  One option is to change how the 
uriloader works a bit.  Right now, it uses "major/minor; options" as a key to 
look up a handler (the MIME type is the key).  It may be a thought to instead 
try:

major/minor; options
major/*; options
*/*; options

in that order.

That way, for example, we could define "*/*; x-view-type=view-source" as being 
handled internally (to fix this bug) or as being handled by a helper (to fix bug 
8589).
Win 95 OSR2 Moz 0.9.9+ Nightly 2002.04.05

This bug is classified "Browser", but I Have something similar with "Mail &
Newsgroups".

When I read a newsgroup message off-line, View/Message source doen't display
anything.

When on-line, the source is normally displayed.
Do you accept this bug, or does it required to open a new number ?


It must be a new bug because message source doesn't use view-source.
*** Bug 140979 has been marked as a duplicate of this bug. ***
Blocks: 92187
Target Milestone: mozilla1.2alpha → mozilla1.2beta
Blocks: 125723
Attached patch Proposed fix (obsolete) — — Splinter Review 
This fixes all the testcases I have been able to get my hands on, from this and
duplicate bugs, namely:

ftp://ftp.mozilla.org/Welcome
http://bugzilla.mozilla.org/attachment.cgi?id=81518&action=view
news://news.mozilla.org:119/3C34FF3D.2010309@netscape.com
http://wp.netscape.com/h.js
http://cgi.ebay.com/aw-cgi/eBayISAPI.dll?ViewItem&item=1247292051
http://bugzilla.mozilla.org/buglist.cgi?bug_id=77337

(this last is "fixed" in that it loads source, but bug 76816 should maybe stay
open for doing a better job showing that source).

I backed out the patch for bug 86835 because it's no longer needed with these
changes (note that the UnknownDecoder still knows about view source).
taking, too.
Assignee: harishd → bzbarsky
Status: ASSIGNED → NEW
Summary: view-source: does not allow viewing of many text files → [FIX]view-source: does not allow viewing of many text files
Keywords: patch
Comment on attachment 97635 [details] [diff] [review]
Minor tweak to handle servers sending us application/x-view-source content (why??)

sr=darin (looks good to me)
Attachment #97635 - Flags: superreview+
Comment on attachment 97635 [details] [diff] [review]
Minor tweak to handle servers sending us application/x-view-source content (why??)

r=jst
Attachment #97635 - Flags: review+
Fixed.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
so one could view source of an image/gif now, right?
in that case you should remove the observes="isImage" from the view source
menuitem...
One _could_ do it (it would be shown as plaintext and be kinda useless).  I
think one _shouldn't_ do it, however.
Why not? How is timeless (bug 84542 comment 4) otherwise going to find out that
the Mozilla banner is GIF89a?
Because viewing source on a 100kb image will take up a good chunk of CPU time
and memory... just try it.  And images come a hell of a lot larger than that all
the time.

Timeless can use a friggin' image manipulation program if he wants data like
that on the image, imo.

Again, these are my opinions.  The UI module owner should have the final say on
what the front end does.
Mozilla has way too many geek features already, timeless can cope otherwise. 
Nuff said!

/be
neil, at the time I made that comment no nightly with the patch was available,
and I had no easy way to test the patch.
Just tried that view-source: URL I gave earlier: Yes, it is GIF89a :-)
Status: RESOLVED → VERIFIED
*** Bug 173620 has been marked as a duplicate of this bug. ***
*** Bug 173620 has been marked as a duplicate of this bug. ***
*** Bug 56898 has been marked as a duplicate of this bug. ***
*** Bug 135624 has been marked as a duplicate of this bug. ***
Apologies if I've missed something, but line 328 in nsNetModule looks a little suspicious:

static PRUint32 g_StreamConverterCount = sizeof(g_StreamConverterCount)/sizeof(const char*);

Surely the formula should be sizeof(g_StreamConverterArray)/sizeof(const char*);?

I'm trying to debug why FF isn't starting on a new platform, it's something to do with static member initialisation in libnecko so I came across this from my lxr searching.

I have no idea how this patch worked for all the test cases though, as it looks to me like g_StreamConverterCount will be 1 and most of the converters will not be registered.
> line 328 in nsNetModule looks a little suspicious

See bug 315159.

> I have no idea how this patch worked for all the test cases though

The converters are registered as contracts; just not in the graph-making code.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: