Content-Disposition is "inline" instead of "attachment" in response to Download Diff

[Brett Randall]

Hi,

RB 1.7.21.

I've noticed that the response to Download Diff includes a Content-Disposition: inline:

GET /r/1234/diff/raw/ HTTP/1.1

Content-Type: text/x-patch

...

Content-Disposition: inline; filename=some.patch

Shouldn't that be:

Content-Disposition: attachment; filename=some.patch

 ?  Current Chrome version sees text/ MIME type and displays the content inline (in the current window) as suggested, instead of raising a download dialog.  I image most if not all folks clicking "Download Diff" want the patch as a file, otherwise they would click "View Diff".

Has this come up before?  Raise a bug?

Thanks

Brett

[Christian Hammond]

Hi Brett,

It's actually never come up before, that I know of. It may be more useful to do that, but I think it's arguable. Certainly, I sometimes view the diff and am happy looking at it right in the browser, instead of having to save it somewhere and then open it.

I'm interested in hearing what others have to say.

Christian

--
Christian Hammond - chi...@chipx86.com
Review Board - http://www.reviewboard.org

Beanbag, Inc. - http://www.beanbaginc.com

--
Get the Review Board Power Pack at http://www.reviewboard.org/powerpack/
---
Sign up for Review Board hosting at RBCommons: https://rbcommons.com/
---
Happy user? Let us know at http://www.reviewboard.org/users/
---
You received this message because you are subscribed to the Google Groups "reviewboard" group.
To unsubscribe from this group and stop receiving emails from it, send an email to reviewboard...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[David Trowbridge]

Personally, if I click "Download X" and it displays in the browser, I get annoyed.

-David

[Steven MacLeod]

Most sites seem to do the opposite of what I want about 50% of the time. I think in this case though since Review Board gives you a diff viewer, you'd generally want to download the diff when clicking "Download Diff", so I vote for changing it.

As an aside, here is a firefox add-on I use for dealing with the annoying opposite case where things try to download and I want to view them in the browser: https://addons.mozilla.org/en-US/firefox/addon/open-in-browser/

[Brett Randall]

Thanks Christian.  I ran blame to find the commit where this was added, and commented:

https://github.com/reviewboard/reviewboard/commit/ed3e2a4382aeaf4155dabe67a2d259ca72362096

So I think the drive here was to ensure that the patch filename is retained, but "attachment" might be more appropriate than "inline" for raw diff requests.

Thanks

Brett

[Christian Hammond]

Okay, I'm happy switching it over then. Would you mind filing a bug (or a review request, if you want to just make the switch)? We can get it in for a 2.0.x.

Christian

--
Christian Hammond - chi...@chipx86.com
Review Board - http://www.reviewboard.org

Beanbag, Inc. - http://www.beanbaginc.com

[Brett Randall]

Thanks, done, https://code.google.com/p/reviewboard/issues/detail?id=3384 .