458376 - Bugzilla should let me directly view application/x-javascript attachments

Status: NEW

(Bugzilla :: Attachments & Requests, enhancement)

Description

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

In the "Details" view of an attachment, Bugzilla doesn't directly show application/x-javascript  files. Instead it offers me to download it. But since at least Firefox can directly display those files as text, bugzilla should let me view these attachments.

Comment 1

[Max Kanat-Alexander]

Does it show them when you mark them text/javascript?

Comment 2

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

Yeah, that's the workaround I'm using.

Comment 3

[Max Kanat-Alexander]

Okay. In that case we should probably show application/x-javascript in the window, then, and any other of the javascript MIME types.

Comment 4

[Frédéric Buclin]

(In reply to comment #0)
> application/x-javascript

Where does this MIME type come from? Entered manually or auto-determined by the browser?

Comment 5

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

I don't know, it wasn't my attachment (Bug 327765). CC'ing the attacher.

Comment 6

[Bjarne (:bjarne)]

The MIME type was auto-detected. The environment from which I attached the file is a 64-bit Ubuntu 8.04.

Comment 7

[Frédéric Buclin]

Bugzilla::Attachment->is_viewable looks at the values returned by $cgi->Accept() to know if a MIME type is accepted by the browser or not. Using Firefox 3.1b2, this list is:

*/*,application/xhtml+xml,application/xml,text/html

As application/x-javascript (or application/javascript when I let Fx3.1 decide what MIME type the file is) is not explicitly listed there, is_viewable() returns false and so the UI offers to download the file instead of displaying it.

As I have no idea if non-Gecko browsers can display them correctly or not, we could add this MIME type to the existing

    return 1 if (($contenttype =~ /^application\/vnd\.mozilla\./)
                 && ($cgi->user_agent() =~ /Gecko\//));

code.

Comment 8

[Ges Seger]

I was about ready to tackle this bug and I'm glad I looked it over again.  Your proposed fix is a lot cleaner than the one I was considering (override the content type in attachment.cgi)

Related issue: is there a chance that a javascript MIME-type like application/x-ecmascript or application/ecmascript will get stored by Bugzilla?

Comment 9

[Max Kanat-Alexander]

Here's the question, really--if a file is application/x-javascript, does that mean that you expect us to execute it, or you expect us to display it? Will this require more complexity than it's worth?

Comment 10

[Ges Seger]

My personal preference is displayed, not executed.  It's safer.

Comment 11

[Frédéric Buclin]

(In reply to comment #10)
> My personal preference is displayed, not executed.  It's safer.

Well, the question is not be safe or not. It's like saying "display the source code of text/html files to avoid executing the JS code in it". :)

Comment 12

[Ges Seger]

Attachment: v1

Based on comments made today for this bug.  Should allow display of files with MIME types application/x-javascript, application/x-ecmascript, application/javascript, and application/ecmascript.

Comment 13

[Max Kanat-Alexander]

Comment on attachment 354752 [details] [diff] [review]
v1

I'd like to see this actually tested on other browsers before limiting it to Gecko.

Comment 14

[Ges Seger]

Apologies for not getting back to this sooner.  I've been dealing with sick family members while trying to stand my own Bugzilla server up on my home network.

Anyway, I put a test bug into my test server with two attachments: a javascript file with no file extension (showed up as application/text), and a javascript file with a .js extension (showed up as application/x-javascript).  I then baselined existing behavior as follows:

Firefox v3 (Mac OS X) - asked to download application/text, displayed application/x-javascript without complaining
Safari v3 (Mac OS X) - same as Firefox 3
Firefox v3 (Windows XP) - same as Firefox for Mac OS X
IE v 7 (Windows XP) - asked to download both attachments

I then applied the patch from attachment 354752 [details] [diff] [review] -- with the removal of the test for Gecko-based browsers as Max recommended on 12/29 -- to my home server and attempted to view the bug's attachments in IE v7 on my Windows XP system as before.  IE continued to ask to download both attachments.

The original javascript files were prepared on a Mac OS X and uploaded to my test server from it.  My copy of Bugzilla is running on a FreeBSD v5.3 server.

Comment 15

[Frédéric Buclin]

Comment on attachment 354752 [details] [diff] [review]
v1

>+    return 1 if (($contenttype =~ /^application\/(x-)?(ecma|java)script/)
>+                 && ($cgi->user_agent() =~ /Gecko\//));

From my own testing: Firefox 3.1b3 doesn't like application/x-ecmascript; it tries to download it in all cases. Same for Konqueror 4.1.3. Opera is always happy and display them all. IE6 and IE8 are never happy and try to download them in all cases because they sniff the content of the attachments.

So we must exclude Internet Explorer (all versions) but not restrict to Gecko only as Konqueror (KHTML) and Safari (WebKit) are both able to display them. We must also not include x-ecmascript in the list as it causes trouble with Fx3.1 and Konqueror (no idea about Safari).

Comment 16

[Ges Seger]

OK, straightforward enough.

Instead of just tacking on extra conditionals to the existing if(), I'm leaning toward a minor rewrite to improve future maintenance.  Something like:

if ($contenttype =~ /^application\/(x-)?javascript/) {
  return 1 if ($cgi->user_agent() =~ /Gecko\//);
  return 1 if ($cgi->user_agent() =~ /KHTML/);
  ...
}

Should make it easier to include future allowed browser engines.  Thoughts?

Comment 17

[Max Kanat-Alexander]

Honestly, I'd think that the server should just be obeying the Accept header sent by the browser, no?

Comment 18

[Frédéric Buclin]

(In reply to comment #16)
> Should make it easier to include future allowed browser engines.  Thoughts?

What bothers me with this is that we are going to add more and more stuff to is_viewable() because such or such browsers are able to display it inline. If the browser is unable to tell the server whether it accepts such or such MIME types or not, I would tend to say it's a problem with the browser.

To keep things simple, we should only care about application\/(x-)?javascript and let (x-)?ecmascript alone for now. Then the question is: do we want a whitelist (with Gecko, KHTML, WebKit, Presto, etc...) in it, or a blacklist (with IE only)? This is going to be a pain to maintain if old versions of a given browser do not support it and new versions do. What's best practice here?

Comment 19

[Frédéric Buclin]

We no longer accept new features for Bugzilla 3.6. Retargetting to 3.8.