84542 - This Frame > View Page Source shows binary data if frame src is an image

Status: NEW

(SeaMonkey :: UI Design, defect)

Description

[sairuh (rarely reading bugmail)]

not sure if this should be in xp apps, the parser or even imagelib. punt where
appropriate, thx!

spun off of bug 25497. reminiscent of bug 77337, except that one only seems to
deal with text files.

to repro:
1. go to a page with images, eg, http://bugzilla.mozilla.org/
2. over the ant image bring up the context menu and select View Image. the
browser should then load only that image.
3. from the main menu, select View > Page Source.

results: a blank source window appears...as well as the download/helper-app
dialog. the dialog reads,

  You have chose to download a file of type: "JPEG Image" [image/jpeg]
  from http://bugzilla.mozilla.org/

expected: shouldn't get the download dialog [not sure about the source window,
but i don't *think* it's should be there]. shouldn't this menu item be disabled
[greyed-out]?

Comment 1

[doctor__j]

Should Moz act like NS 4.x to display all the binary crap of the image?

Comment 2

[Doron Rosenberg (IBM)]

in IE the view source is greyed out. I think showing the binary info is useless.
If you want it, save the image to disk and play with it locally.

Comment 3

[Gervase Markham [:gerv]]

The way to fix this is for View | Page Source to get the MIME type of the 
current content (however Page Info does it) and have a list of MIME types it can 
view the source of. Either that, or have the View Source window do something 
more intelligent when passed a MIME type it doesn't understand.

Gerv

Comment 4

[timeless]

viewing source of images is sometimes useful when you want to check the image 
header eg, I can't use nc4's page info to determine that the 
mozilla-banner is a GIF89a although it gives me lots of other pieces of 
information.

I'd like view-source to just display the source for all mime types it doesn't 
recognize.

Comment 5

[Boris Zbarsky [:bzbarsky]]

What we need to do then, is the following:

The view source handler should map the mime type to text/plain unless 
the type is a known type we can usefully highlight view source of.  That 
way things would work pretty.

This would also fix bug 77237.

Is this a decent approach?  ccing chak who implemented the protocol 
handler.

Comment 6

[Viswanath Ramachandran]

Not critical for this release, moving to Future. 

Comment 7

[timeless]

sounds fine w/ me.

Comment 8

[sairuh (rarely reading bugmail)]

spam: over to File Handling.

Comment 9

[Don J. Rude]

My two cents is this: When a web server claims a non-text or non-html type (EG
image) view-source should still be an option because I have run across many
borken servers where I want to 'view source' because the file turns out to
actually be text. I guess this is a broader UI issue? (cross posting to bug 92901)

Comment 10

[sairuh (rarely reading bugmail)]

mass moving open bugs pertaining to view source to [e-mail] as qa contact.

to find all bugspam pertaining to this, set your search string to
"ItsSharkeysNight".

Comment 11

[Malcolm Scott]

The following bugs seem like duplicates of this one:
bug 76253 (that's MailNews but probably the same bug)
bug 92901

Comment 12

[Jonas Jørgensen]

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

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: disable view source for images [Checkin: Comment 19]

as a temporary measure, we could just disable view source for images, which
does not work anyway. comments?

Comment 14

[Terri Preston]

sounds good to me and I'm nominating

Comment 15

[Boris Zbarsky [:bzbarsky]]

will that disable it for image/svg+xml too?  That would be poor, if so.  For one
thing, we have no trouble viewing source of such images....

I agree with the overall approach, though.  Viewing source of binary images is
not useful, imo.  That's what a hex editor is for.

Comment 16

[Jonas Jørgensen]

> I agree with the overall approach, though.  Viewing source of binary images is
> not useful, imo.  That's what a hex editor is for.

I've seen servers misconfigured so badly that they send some text or HTML
content with an image mime type. View source would be useful in that case.

What you suggested in comment 5 sounds like the best approach to me.

Maybe we should open a new bug about making view-source treat unknown types as
text/plain and make this bug and bug 77337 depend on it (or perhaps even dupe
them on it)?

Comment 17

[Christian :Biesinger (don't email me, ping me on IRC)]

bz, see nsBrowserStatusHandler.js lines 170ff:
this is the condition when view source (et al) are enabled:
    return /^text\/|\+xml$/.test(contentType) ||
           contentType == "application/x-javascript" ||
           contentType == "application/xml" ||
           contentType == "mozilla.application/cached-xul";
so anything ending in +xml should work fine.

jonasj, I did intent to make this only a temporary patch and leave the bug open
for the real patch (to make view source show the source).

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 95293 [details] [diff] [review]
disable view source for images
[Checkin: Comment 19]

sounds good.  sr=bzbarsky

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 95293 [details] [diff] [review]
disable view source for images
[Checkin: Comment 19]

checked in

Comment 20

[Daniel Clemente]

With 2002081521 on Linux, I can't 'View -> Page Source' from the menu, but if I
right click the background, there's another 'View Page Source' option that still
works.

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: oops, indeed. additional patch [Checkin: Comment 39]

Comment 22

[Daniel Clemente]

This bug is now resolved. Should anyone change its status to RESOLVED?

Comment 23

[Christopher Hoess (gone)]

Marking FIXED. Reopen if this is wrong...

Comment 24

[Boris Zbarsky [:bzbarsky]]

It's only resolved if you think the source of images should be viewable...

I don't care much one way or the other, myself.

Comment 25

[Daniel Clemente]

   Mmmm.. I'm a bit confused.... 

   I prefer not to view the source for images. Anyway, with Linux 2002100604,
the option from the menu bar can't be clicked, but the popup that appears by
rightclicking the background has an option that works (it shows the image file
as plain text).
   Should we correct this and make that both behave equally?

   PS: Ctrl+U also doesn't work

Comment 26

[Boris Zbarsky [:bzbarsky]]

Yes, I think both should behave equally.

Ctrl-u just triggers the main menu item, which is disabled.  So it can't work.  ;)

Comment 27

[Christian :Biesinger (don't email me, ping me on IRC)]

my last attachment here, created aug. 16, should do it... I'll leave it as an
exercise to the reader to chase review and super-review for it.

Comment 28

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 95549 [details] [diff] [review]
oops, indeed. additional patch
[Checkin: Comment 39]

sr=bzbarsky if you also do frame source like that... ;)

Comment 29

[Daniel Clemente]

   Well, and then, as the bug is now RESOLVED/FIXED, it will be verified by a QA
member, marked as VERIFIED/FIXED, and the patch will be checked in into the
trunk so that nightly builds would be corrected, isn't it?
  I think I'm starting to learn the life of a bug... :)

Comment 30

[Boris Zbarsky [:bzbarsky]]

bah

Comment 31

[Doron Rosenberg (IBM)]

Comment on attachment 95549 [details] [diff] [review]
oops, indeed. additional patch
[Checkin: Comment 39]

r=doron

Anyone else think that "BrowserViewSource" in a function name is stupid?  I
want a view source manager!

Comment 32

[Christian :Biesinger (don't email me, ping me on IRC)]

bz: for frame source, this will be considerably more difficult because the
isImage observer only affects the toplevel document...

Comment 33

[Boris Zbarsky [:bzbarsky]]

Yes, I realize that... ;)

Comment 34

[Christian :Biesinger (don't email me, ping me on IRC)]

very well then :)
can I check this in anyway?

if not, someone else will have to do the work... I no longer work on mozilla
frontend, as you know.

Comment 35

[Boris Zbarsky [:bzbarsky]]

Yeah, check that in anyway (sr=bzbarsky) and leave the bug open to handle the
context menu.

Comment 36

[Christian :Biesinger (don't email me, ping me on IRC)]

taking bug so I remember to check this in (tree closed atm)

also changing component to xp apps: gui features which is where the code lives.

Comment 37

[jag (Peter Annema)]

r=jag

Comment 38

[Asa Dotzler [:asa]]

Comment on attachment 95549 [details] [diff] [review]
oops, indeed. additional patch
[Checkin: Comment 39]

a=asa for checkin to 1.2beta (on behalf of drivers)

Comment 39

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 95549 [details] [diff] [review]
oops, indeed. additional patch
[Checkin: Comment 39]

checked in

leaving open for frame source.

Comment 40

[Christian :Biesinger (don't email me, ping me on IRC)]

-> default owner

Comment 41

[Samir Gehani]

Nav triage team: Need info.  Cannot reproduce using ``View Frame Source'' from
the context menu when over an image.  Please include steps to reproduce.  

Comment 42

[Daniel Clemente]

Attachment: testcase with frames

1. Open the attachment. A frame should open at the bottom, with an image on it.

2. Right click the background of the lower frame, select "This frame" -> "View
frame source"

Comment 43

[Boris Zbarsky [:bzbarsky]]

Just to be clear.  The expected behavior is that the option is completely
disabled.  Instead, it's enabled and shows some garbage binary data.

Comment 44

[Samir Gehani]

Nav triage team: nsbeta1-

Comment 45

[OstGote!]

The testcase from comment 42 still works. Adjusting summary.

Comment 46

[Serge Gautherie (:sgautherie)]

Filter "spam" on "guifeatures-nobody-20080610".

Comment 47

[Boris Zbarsky [:bzbarsky]]

Bug 384854 fixed the remaining issues here.

Comment 48

[Boris Zbarsky [:bzbarsky]]

Er, maybe not.  That bug is Firefox, and this one is seamonkey.

Comment 49

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090622 Minefield/3.6a1pre] (mozilla-central-win32-unittest/1245678366) (W2Ksp4)

No menu items.

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1pre) Gecko/20090619 SeaMonkey/2.0b1pre] (comm-1.9.1-win32-unittest/1245417812) (W2Ksp4)

Menu item, showing the source as the image, no its binary data.

Comment 50

[Tristan Miller]

Still reproducible for me with Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Firefox/60.0 SeaMonkey/2.53.5.1. Per Comment 49, This Frame -> View Page Source shows the image itself, when the expected behaviour is that the View Page Source menu item should be disabled.