464339 - Links to images and non-textish media should not have view-source: links

Status: RESOLVED FIXED

(Toolkit :: View Source, defect, P2)

Description

[Schuyler Duveen]

User-Agent:       Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1b2pre) Gecko/20081111 Minefield/3.1b2pre
Build Identifier: 

Awesome feature with the links, though

Reproducible: Always

Steps to Reproduce:
1. Goto view-source:http://www.mozilla.com/en-US/
2. click /img/tignish/template/mozilla-logo.png
3. Gross! binary!

Comment 1

[Nochum Sossonko [:Natch]]

Confirmed using: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081110 Minefield/3.1b2pre

Comment 2

[u88484]

Crap, I confirmed this thinking the reporter was complaining about the binary and not that images are linked.  Do you want to expand this bug to "Links to images and non-textish media show binary"

And then within this bug let the devs decide if the media should be shown or not show links for media.

Comment 3

[Schuyler Duveen]

(In reply to comment #2)
> ****, I confirmed this thinking the reporter was complaining about the binary
> and not that images are linked.  Do you want to expand this bug to "Links to
> images and non-textish media show binary"
> 
> And then within this bug let the devs decide if the media should be shown or
> not show links for media.

I was complaining that they appear as binary and not that the images are linked. 'links' in the summary is a noun :-)

I suppose one solution is not to link the images at all, but I'd prefer that they be linked, just without the view-source: protocol.

Comment 4

[Curtis Bartley [:cbartley]]

We can't reliably tell from looking at a URL whether it's pointing to an HTML file, and image, or something else -- we actually need the file's MIME type to know that.

I think the right way to go is to make view-source: URL handling smarter.  The MIME type can be consulted before the display method is chosen, and it can do the right thing at that point.  Effectively 
  view-source:http://www.example.com/picture.png 
would be displayed exactly the same way as
  http://www.example.com/picture.png
assuming of course that the MIME type for PNG really is image/png.

I believe that layout/build/nsContentDLF.cpp is the proper place for this sort of change, and in fact I've been able to modify it so that images are handled as described above.  Unfortunately, it's a complete hack at this point and I haven't had time to work out a proper solution.

Comment 5

[Curtis Bartley [:cbartley]]

Attachment: A hack to make view-source URLs for images display *as* images.

This patch contains the nsContentDLF.cpp hack I described in comment #4.  It causes view-source URLs to display files identified as images by their MIME type actually *as* images, rather than trying to interpret their binary data as text.

The change is so crude that it's not remotely suitable for check-in, however, I'm attaching it in case someone else wants to take a swing at it before I get a chance.  I think the actual changes needed here are pretty simple, and the only hard part is understanding the code in nsContentDLF.cpp well enough to understand what those changes should be.

Comment 6

[Biju]

Just curious....

Utility of view-source is many ways, there are some use which was nerver intended to.
One example is many times I use view-source:http://somesite.com/textfile.blah to see text content of a file on a server which not configured to show file with extension .blah as text.

Question, whether fixing this will affect my way of browsing web?

So can we have about:config item for user to decide whether to continue showing image as view-source text or to show as image.

 OR 

Alternatively when we do view-source if we see lot of binary non-printable data, we can have a "bar at the top of content display" prompting user what to do.
(a) continue showing as text 
(b) show as image 
(c) download as file 
etc...

Comment 7

[Biju]

(In reply to comment #6)
test case for my issue

view-source:http://www.stanford.edu/services/web/cgi/examples/printenv.pl
view-source:http://www.stanford.edu/services/web/cgi/examples/printenv.sh
at http://www.stanford.edu/services/web/cgi/examples.html

Comment 8

[Bill Gianopoulos [:WG9s]]

(In reply to comment #3)
> (In reply to comment #2)
> > ****, I confirmed this thinking the reporter was complaining about the binary
> > and not that images are linked.  Do you want to expand this bug to "Links to
> > images and non-textish media show binary"
> > 
> > And then within this bug let the devs decide if the media should be shown or
> > not show links for media.
> 
> I was complaining that they appear as binary and not that the images are
> linked. 'links' in the summary is a noun :-)
> 
> I suppose one solution is not to link the images at all, but I'd prefer that
> they be linked, just without the view-source: protocol.

Either way it is a bug.  It should either not be linked or display correctly if you click.  Trying to display binary as text is always wrong.

Comment 9

[Curtis Bartley [:cbartley]]

Attachment: Now viewing the source of an image is the same as just viewing the image.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 354245 [details] [diff] [review]
Now viewing the source of an image is the same as just viewing the image.

This looks good, but can we have a reftest for this? Also, can you do the same fix for video/audio and plugins (for which we also create special documents to view the raw content)?

Comment 11

[Curtis Bartley [:cbartley]]

(In reply to comment #10)
> (From update of attachment 354245 [details] [diff] [review])
> This looks good, but can we have a reftest for this? Also, can you do the same
> fix for video/audio and plugins (for which we also create special documents to
> view the raw content)?

I've spun out the video/audio/plugin issue to bug 472417 since I don't know when I'll be able to get to it.  I'll try to get to the reftest in the next couple of days.

(In reply to comment #10)
> (From update of attachment 354245 [details] [diff] [review])
> This looks good, but can we have a reftest for this? Also, can you do the same
> fix for video/audio and plugins (for which we also create special documents to
> view the raw content)?

Comment 12

[Curtis Bartley [:cbartley]]

Attachment: Now viewing the source of an image is the same as just viewing the image, now with reftest

The C++ changes are the same as the previous patch.  The only difference is that the patch now contains a reftest.

Comment 13

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/745c0c2aa1fe

Comment 14

[Curtis Bartley [:cbartley]]

Patch applied cleanly to 1.9.1 as of a few minutes ago.

Comment 15

[Curtis Bartley [:cbartley]]

Requesting blocking 1.9.1.  It's not a disaster if it doesn't get fixed, but it looks bad, and the functionality will be missed.  The fix is pretty straighforward and it's been baking in trunk for four or five days now.

Comment 16

[Phil Ringnalda (:philor)]

Looks like you forgot to request the easiest to get and most likely thing, approval1.9.1 on the patch.

Comment 17

[Curtis Bartley [:cbartley]]

I thought that the wanted1.9.1:? was nearly as good as a blocking1.9.1:? at the time it was made.  My sources now tell me that I sadly mistaken.(In reply to comment #16)
> Looks like you forgot to request the easiest to get and most likely thing,
> approval1.9.1 on the patch.

I thought that the wanted1.9.1:? was nearly as good as a blocking1.9.1:? at the time it was made.  My sources now tell me that I sadly mistaken.

Comment 18

[Bill Gianopoulos [:WG9s]]

No, what you want to do is select details on the patch and request approval-1.9.1.

Comment 19

[Curtis Bartley [:cbartley]]

(In reply to comment #18)
> No, what you want to do is select details on the patch and request
> approval-1.9.1.

Oh right -- forgot that the patches have flags too.

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 355884 [details] [diff] [review]
Now viewing the source of an image is the same as just viewing the image, now with reftest

(now that this is a blocker, doesn't need explicit a)

Comment 21

[Robert Strong (they/them - no direct email)]

Pushed to mozilla-1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/20c023eda85f

Comment 23

[Doug Wright]

So, this has actually broken a useful feature in my opinion - sometimes I have actually WANTED to directly view-source an image URL (e.g. when a I suspect a web framework has accidentally injected HTML before/after the an image that refuses to display). Previously, I've been able to simply view-source the corrupt image and immediately see what was wrong, but today I wasn't able to.

If Firefox isn't going to display what I (the user) asked for, it should at least tell me that somehow (e.g. by redirecting the URL to the standard non-view-source version). At the moment, this "smart" behaviour just looks broken.

Comment 24

[François Leurent]

I used to use this scheme to debug dynamic server side generated images, where i need to view the binary of it (and beeing able to F5 to reload on it was a GREAT thing)... No i just need to use another browser, or download the file each time :(.. Was this a real bug (and needing a 'fix') ? I hope so...

Comment 25

[François Leurent]

Anyway, that was for me a good occasion going deeper in the mozilla project :p. Last night,  i successfully build my :') first --enable-official-build & co with a comment on Doug Wright's patch, allowing me to see my beloved binary. 
I intend to submit a patch to toggle the binary mode (on picture) on the "about:config/view_source.syntax_highlight" value.
Is it a good idea ?

Comment 26

[François Leurent]

* Curtis Bartley's patch

Comment 27

[François Leurent]

Attachment: Toggle the behavior previous according to "view_source.syntax_highlight"

Comment 28

[Curtis Bartley [:cbartley]]

(In reply to comment #27)
> Created an attachment (id=411103) [details]
> Toggle the behavior previous according to "view_source.syntax_highlight"

Can you open a new bug, and attach the patch there?