Closed Bug 464339 Opened 16 years ago Closed 16 years ago

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

Categories

(Toolkit :: View Source, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9.2a1

People

(Reporter: sky, Assigned: cbartley)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Keywords: fixed1.9.1)

Attachments

(2 files, 2 obsolete files)

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!
Confirmed using: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081110 Minefield/3.1b2pre
OS: Linux → All
Version: unspecified → Trunk
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
(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.
Flags: wanted1.9.1?
Hardware: PC → All
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.
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.
Blocks: 17612
Assignee: nobody → cbartley
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...
(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.
Attachment #347807 - Attachment is obsolete: true
Attachment #354245 - Flags: review?
Attachment #354245 - Flags: review? → review?(roc)
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)?
Attachment #354245 - Flags: approval1.9.1?
Depends on: 472417
(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)?
No longer depends on: 472417
Depends on: 472417
The C++ changes are the same as the previous patch. The only difference is that the patch now contains a reftest.
Attachment #354245 - Attachment is obsolete: true
Attachment #355884 - Flags: review?
Attachment #354245 - Flags: approval1.9.1?
Attachment #355884 - Flags: review? → review?(roc)
Keywords: checkin-needed
Blocks: 472846
Keywords: checkin-needed
Target Milestone: --- → mozilla1.9.2a1
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Patch applied cleanly to 1.9.1 as of a few minutes ago.
Flags: blocking1.9.1?
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.
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.(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.
No, what you want to do is select details on the patch and request approval-1.9.1.
Attachment #355884 - Flags: approval1.9.1?
(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.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
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)
Attachment #355884 - Flags: approval1.9.1?
Flags: blocking1.9.1+ → blocking1.9.1?
Keywords: checkin-needed
Priority: P2 → --
Whiteboard: [needs 191 landing]
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Blocks: 476361
No longer blocks: 476361
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.
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...
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 ?
* Curtis Bartley's patch
(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?
Attachment #411103 - Flags: review?(cab) → review-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: