If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Recommend filename when downloading attachment

RESOLVED FIXED in Bugzilla 2.18

Status

()

Bugzilla
Attachments & Requests
P2
enhancement
RESOLVED FIXED
17 years ago
5 years ago

People

(Reporter: timeless, Assigned: justdave)

Tracking

2.12
Bugzilla 2.18

Details

(URL)

Attachments

(1 attachment, 10 obsolete attachments)

6.57 KB, patch
gerv
: review+
gerv
: review+
Details | Diff | Splinter Review
(Reporter)

Description

17 years ago
We should recommend a filename. showattachment.cgi is a bad recommendation.
(Reporter)

Comment 1

17 years ago
Created attachment 21200 [details] [diff] [review]
This patch probably won't work

Comment 2

17 years ago
Created attachment 21201 [details] [diff] [review]
Use info from SQL query for filename, and include orig. filename
(Reporter)

Comment 3

17 years ago
Created attachment 21203 [details] [diff] [review]
Concept-Patch that won't do anything

Comment 4

17 years ago
attachments should show their file name (instead of just date/time or "id=xxx").

Comment 5

17 years ago
Created attachment 21388 [details] [diff] [review]
PATCH to suggest a filename in the form of "BUG#-FILENAME.EXT"

Comment 6

17 years ago
Example for the above patch...  When I created the file I named it 63601.dif.
If you were to try and download it (and if it weren't text/plain) it would
suggest the name of 63601-63601.dif (BUG#-FILENAME).
Keywords: patch, review

Comment 7

17 years ago
*** Bug 20383 has been marked as a duplicate of this bug. ***

Comment 8

17 years ago
Please also look at bug 20383 which I marked duplicate, because it has
a small patch attached, too.
Summary: Recommend filename → Recommend filename when downloading attachment
Target Milestone: --- → Bugzilla 2.16

Comment 9

17 years ago
An alternative technique that works on at least some web servers is to make the 
link of the form:
"/showattachment.cgi/originalfilename.txt?attach_id=12345"

The following patch does this, and also adds a GetAttachLink to centralise the 
process in a similar way to GetBugLink

Comment 10

17 years ago
Created attachment 32415 [details] [diff] [review]
Patch to tweak the form of attachment links.

Comment 11

17 years ago
*** Bug 82527 has been marked as a duplicate of this bug. ***

Comment 12

17 years ago
Bug 82527 also came with a patch (attachment 35950 [details] [diff] [review]).

Comment 13

17 years ago
*** Bug 85706 has been marked as a duplicate of this bug. ***
Priority: -- → P2
->New product Bugzilla
Assignee: tara → myk
Component: Bugzilla → Creating/Changing Bugs
Product: Webtools → Bugzilla
Version: other → 2.12
*** Bug 102140 has been marked as a duplicate of this bug. ***
Bug 102140 has a patch for this.
And this bug has several.  They all seem to take a different approach to it. 
Daniel, can you reupload your patch to this bug?

Comment 18

16 years ago
Created attachment 51499 [details] [diff] [review]
Use orginal filename (without complete path). Tested with Netscape 4.71, IE 5.0 and IE 5.5.
Comment on attachment 51499 [details] [diff] [review]
Use orginal filename (without complete path). Tested with Netscape 4.71, IE 5.0 and IE 5.5.

Looks sensible to me. I didn't know we stored the original filename in the DB.

r=gerv.

Gerv
Attachment #51499 - Flags: review+
This does not seem to work with wget. It still saves the filename improperly.
Example if I do:

wget http://bugzilla.redhat.com/bugzilla/showattachment.cgi?attach_id=33499

it properly downloads the file and places it in the current directory but the
filename itself is:

showattachment.cgi?attach_id=33499
Comment on attachment 51499 [details] [diff] [review]
Use orginal filename (without complete path). Tested with Netscape 4.71, IE 5.0 and IE 5.5.

>! $filename =~ s/^.*[\/\\]//;

This truncates a path down to a name, right? What about MacOS, which uses a ":"?

>! print qq{Content-type: $mimetype; name="$filename"\nContent-disposition: file; filename=$filename\n\n$thedata};

Does the filename need to be in quotes in Content-disposition?

Gerv
Attachment #51499 - Flags: review+
Daniel: could you respond to the review comments? Thanks :-)

Gerv
We are currently trying to wrap up Bugzilla 2.16.  We are now close enough to
release time that anything that wasn't already ranked at P1 isn't going to make
the cut.  Thus this is being retargetted at 2.18.  If you strongly disagree with
this retargetting, please comment, however, be aware that we only have about 2
weeks left to review and test anything at this point, and we intend to devote
this time to the remaining bugs that were designated as release blockers.
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
I'm looking at attachment 51499 [details] [diff] [review], "Use orginal filename (without
complete path). Tested with Netscape 4.71, IE 5.0 and IE 5.5." for
this comment.

For reference, RFC 2183 covers the Content-Disposition MIME header.

http://www.ietf.org/rfc/rfc2183.txt

Note that "file" is not a standard Content-Disposition value (the
only ones defined in the RFC are "inline" and "attachment").  However,
this non-standard value may force various versions of MSIE to actually
respect the desire of the programmer and force a save-file dialog box
for file being sent.

http://www.google.com/search?q=site%3Asupport.microsoft.com+content-disposition

Also the filename parameter should NOT be enclosed in quotes.  One of
the Microsoft knowledgebase articles mentions this.  While it is not
explicitly stated in the RFC, the BNF notation has quoted values for
the dates, but not for the filename parameter.  (The patch is correct.
I was looking at the "name" parameter for the Content-Type header.)

Finally, the patch has commented-out code and does not use the $filesize
parameter that it calculates itself.  It needs to add a "size=$filesize"
parameter to the Content-Disposition line.
I think the original filename would be a bad idea.  The filenames of things I
upload are typically useless because I expected them to never be used.  They
could also be the same between different attachments on the same or different
bugs.

I think much more useful would be attachment number or probably better bug
number-attachment number, then perhaps followed by the original filename.
(Reporter)

Comment 26

16 years ago
I agree. Please don't use original filename.  Make a field and let people set
it. And provide a default.

Comment 27

16 years ago
Other than as a candidate for a downloaded filename, is there any need to have 
the original filename in the DB?

If the current filename field was editable on the Edit Attachment screen, "bad" 
filenames could be fixed.

Then add "Upload as filename: (leave blank for same as local filename) [      ]"
onto the Create Attachment form, allowing someone to call it Bug123-Patch2.txt 
or similar.

Comment 28

16 years ago
I think the original filename (minus the path) should be kept because some
software may request a specific filename (config file for instance).
If there is a conflict on the filename, then the save dialog box should display
the overwrite confirmation. It's then up to the downloader to decide what he
wants to do (add a number, add the bug number, replace the current file,...)
Whether you want to use the orignal file name on attachments or not can be a
preference. 

If you turn the pref on, filenames would appear as:

Bug#-Attachment#-origfilename.origextension

Otherwise:

Bug#-Attachment#.origextension

Which would leave only one issue on whether someone might want to quickly toggle
this setting. Perhaps, There could be a box at the bottom of the attachment list
to temporarily toggle this setting.

Comment 30

16 years ago
I think it should default to the uploaded filename unless overridden by the
uploader.

I have run into this problem today.  I uploaded a program called program.gz to
bugzilla but the person downloading it thought it had failed because the file
size was too small and the filename was showattachment.cgi.  If the name was
program.gz, he would have understood what he had to do.

Comment 31

16 years ago
The fact that most original filenames may not be perfect is not interesting -
what is relevant is the fact that when the name _is_ important, it usually is
correct.

One typical use of our installation of Bugzilla is to create requests for
drawing graphics (for web sites). When those graphics are ready, they're
attached. Now, since the filenames for the image files are usually relevant
because they are linked to from html files, it's quite a pain trying to download
the files and give them correct names according to some info in the attachment
comments. It would be much easier to use the original file name directly.

I can't see why there should be something like bug number prefixes in the
filename: if the file isn't unique, then it isn't; browsers usually ask the save
location and file name anyway. Saving attachments is fairly rare; I don't think
name conflicts are worth extra hassle.
(Reporter)

Updated

15 years ago
Component: Creating/Changing Bugs → attachment and request management
*** Bug 161404 has been marked as a duplicate of this bug. ***
Created attachment 94297 [details] [diff] [review]
same as previous, updated for attachment.cgi, add filename editing

This patch takes the previous one and updates it for attachment.cgi (previous
one was against showattachment.cgi), and also adds the capability to edit the
filename to the attachment editing page.

This is live at http://landfill.bugzilla.org/bz63101/show_bug.cgi?id=365
(yeah, I know I screwed up the bug number when I created the landfill install,
but oh well)
Attachment #21200 - Attachment is obsolete: true
Attachment #21201 - Attachment is obsolete: true
Attachment #21203 - Attachment is obsolete: true
Attachment #21388 - Attachment is obsolete: true
Attachment #32415 - Attachment is obsolete: true
Attachment #51499 - Attachment is obsolete: true
Comment on attachment 94297 [details] [diff] [review]
same as previous, updated for attachment.cgi, add filename editing

Waht is content-disposition: file? Where is it defined?

You need to check taht this doesn't break IE - see bug 27248 comment 17 +
friends.
bbaetz: see comment #24 on this bug.

However, see also 
http://support.microsoft.com/default.aspx?scid=KB;EN-US;q182315&

MSIE less than 5.0 is broken with regards to this.  Apparently using "file" as a
disposition works in IE 4.x, but the real ones don't.

Almost everything on the planet that uses IE requires IE 5 these days so it's
probably safe to just leave it broken in IE4 and do it per the RFC (which means
changing the disposition to "attachment")
Contrary to comment 24 from ddk, I don't see anything in the RFC about a "size"
parameter to the Content-Disposition: header.  Should that be moved out to
Content-Length: ?  (or just Length: I forget which is it...)
Comment on attachment 94297 [details] [diff] [review]
same as previous, updated for attachment.cgi, add filename editing

needs-work, per previous comments.  I'll wait for a little more discussion on
the bug before I update the patch again.
Attachment #94297 - Flags: review-
Created attachment 94591 [details] [diff] [review]
Uses "attachment" per rfc, adds Content-Length header

OK, did my research in the RFC, and there is no "size" parameter to the
Content-Disposition header defined.  However, there is a Content-Length header,
which the RFC says is REQUIRED any time we actually know how much data we're
going to send (which we do in this case).

Those changes are made in this patch as well as changing the disposition to
"attachment" instead of "file".  This is correct per the RFC, but doesn't work
in IE4.  As noted in previous comments, Microsoft admits this is a bug in IE4
and claims to have fixed it in IE5.  The end result is that people using IE4
will have the same behaviour they always got from Bugzilla before we added this
header.  People using a browser that follows the standards will be politely
asked to confirm the real filename.
Attachment #94297 - Attachment is obsolete: true
Created attachment 94592 [details] [diff] [review]
fix typoes

oops.  had a blank line in the wrong place, and forgot contributors
Attachment #94591 - Attachment is obsolete: true
I tried this out on Mozilla OS X CFM 2002071608 and MSIE 5.1.1 OSX, it Does The
Right Thing on both. :)
Wasn't the problem with MSIE4 (bug 27248) that trying to _view_ a page which had
this content-disposition header in it would force a save-as dialog to appear? I
don't care if a particular browser won't use our suggested filename, but if it
means that you can't view the attachment....
OK, so the answer is, we need to browser-sniff and if it's MSIE < 5 we don't
include the headers....
Created attachment 94808 [details] [diff] [review]
adds browser-sniffing

This patch adds browser detection, and doesn't display the Content-Disposition
header if the user's browser identifies as an MSIE older than 5.0
Attachment #94592 - Attachment is obsolete: true
Comment on attachment 94808 [details] [diff] [review]
adds browser-sniffing

r=bbaetz x2, as long as someone has tested on windows ie
Attachment #94808 - Flags: review+
I can't, I don't have access to Windows.  Jouni?

Comment 46

15 years ago
I have IE 4.0.  I can test it if it's live somewhere.
it's live at http://landfill.bugzilla.org/bz63601/show_bug.cgi?id=365

(well, that whole installation, but that bug has a few attachments on it to test
with)

Comment 48

15 years ago
Looks good for IE5 on Windows 95.
In Moz 1.1b on Win2K, right clicking an attachment and doing "Save Link Target
As..." suggests attachment.cgi.html. Clicking attachments Moz can't display Does
The Right Thing; obviously, clicking ones it can display displays them.

IE 6, on the other hand, works correctly in all cases.

Gerv
gerv: bz knows about that bug - if you do save-as from teh linked page then it
works most of the time.

bz is of the opinion that fixing that to work correctly all the time in moz in
Hard, because of the way we load stuff (moz sends a HEAD request first, but
apache doesn't send the headers for HEAD modes. Taht could be a bugzilla thing
we could fix at some point, but even if we did, moz still won't always work)

However, we still need an ie < 5 test before this goes in....

Comment 51

15 years ago
On IE 4.0 on NT 4.0.

The patches display directly in the browser.  You are not prompted to save the
file.  I think this is OK.

However, the gz file saves as the file name "attachment" without a file type. 
In mozilla it suggests "AutoResponseV0.3.tar.gz".  The IE 4.0 behaviour doesn't
seem right.
So are we going with this, or not? I say we check it in, and see if anyone
complains.

Gerv
Based on comment 51, this is okay as stands.  The "suggest filename" thing is
completely broken in IE4, thus the reason we disabled it if the browser is IE4.
 The fact that you can view the contents of it at all in IE4 means this patch
didn't break usability on IE4.  (otherwise it would prompt for a Save As..
dialog whether the attachment was viewable or not, so you'd never be able to
view it in your browser window, even if it was text, which was the IE4 bug).

a= justdave, will check it in shortly, assuming it hasn't bitrotted.
Comment on attachment 94808 [details] [diff] [review]
adds browser-sniffing

oof.  it's bitrotted

I don't have time to mess with it right now, someone remind me tomorrow and
I'll refresh it.
Attachment #94808 - Flags: review+ → review-
OK, the primary conflict was the "isprivate" field getting added to the
attachments table.  New patch coming up shortyly.  Joel, you should probably
have a look at it just to double-check that I didn't break the private
attachments thing.
Assignee: myk → justdave
Created attachment 100095 [details] [diff] [review]
Merge with tip
Attachment #94808 - Attachment is obsolete: true
This is still live at http://landfill.bugzilla.org/bz63601/show_bug.cgi?id=365
fwiw.
Comment on attachment 100095 [details] [diff] [review]
Merge with tip

I'm testing this with Mozilla and the Content-Disposition field is meaning that
it pops a "download" dialog when you click the link for all file types,
including text/plain and text/html . It also pops this dialog when you go to
"edit", because you are trying to load the attachment into the iframe.

This works in 1.0 and (according to justdave) 1.1. So Mozilla is broken at the
moment. 

r=gerv. Check it in, and then we can file a bug with Landfill as the test case.
They will be more likely to fix it if they know it'll affect Bugzilla ;-)

Gerv
Attachment #100095 - Flags: review+
Checking in attachment.cgi;
/cvsroot/mozilla/webtools/bugzilla/attachment.cgi,v  <--  attachment.cgi
new revision: 1.23; previous revision: 1.22
done
Checking in checksetup.pl;
/cvsroot/mozilla/webtools/bugzilla/checksetup.pl,v  <--  checksetup.pl
new revision: 1.190; previous revision: 1.189
done
Checking in template/en/default/attachment/edit.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/attachment/edit.html.tmpl,v
 <--  edit.html.tmpl
new revision: 1.11; previous revision: 1.10
done
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 60

15 years ago
I looked over the patch.  I don't think there is a substantial risk of breaking
private attachments.  We're just both adding fields to the same list of fields
to select.
This issue is not dead.  See bug 171296.

Comment 62

15 years ago
*** Bug 175122 has been marked as a duplicate of this bug. ***

Comment 63

15 years ago
Given all the problems with content-disposition headers, etc. is there likely 
still to be any merit in tweaking the attachment links in the manner suggested 
in comment 9, and implemented in obsolete attachment 32415 [details] [diff] [review]?

I'm currently tweaking our local bugzilla 2.16 installation, and wondering 
whether to redo a rough equivalent to that patch or to go with the bugzilla 
standard.
*** Bug 207823 has been marked as a duplicate of this bug. ***
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.