Open Bug 291837 Opened 20 years ago Updated 4 years ago

Content-Disposition is not used when drag&dropping an image from a PHP file

Categories

(Core :: DOM: Copy & Paste and Drag & Drop, defect, P5)

defect

Tracking

()

People

(Reporter: fishos, Unassigned)

References

(Blocks 1 open bug, )

Details

Attachments

(1 obsolete file)

User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.7) Gecko/20050414 Firefox/1.0.3 See Bug 224209 When you "Save As..." an image from a php file (i.e. "attachment.php&postid=163252") it shows you the real filename (i.e. "mypicture.gif"). That's good. But when you're dragging the image to desktop you get the filename "attachment.gif". Reproducible: Always Steps to Reproduce:
Marking dependent on bug 224209 for now, since the arch work there may make this easier...
Depends on: 224209
Status: UNCONFIRMED → NEW
Ever confirmed: true
Not just PHP, of course, but any sort of "dynamic image-serving scripts" (Bugzilla attachment CGIs)....
OS: Windows XP → All
Hardware: PC → All
This behaviour is still occuring. Would a bugfix for this need intricate knowledge of firefox internals or is this something a person with a couple years of general C++ experience could fix in a few days?
I would have thought someone with that experience would be able to answer the question by having a quick dig into the source. My guess is that it would be fairly simple by comparing what Save As does to get the correct filename.
Blocks: 449588
A good testcase can be found here: http://forum.magicball.net/attachment.php?s=&postid=163252 It has: > Content-disposition: inline; filename*=utf-8''cfdesktop.jpg So it is named "cfdesktop.jpg" when content disposition is used, attachment.php otherwise.
Blocks: 491208
Bug 440930 and bug 449588 are dupes of this bug, as I wrote there. Would somebody mark them as dupes, please?
As I said in Comment #4 of bug 449588, it isn't a dupe because solving this bug is only one of the possibilities to fix bug 449588. The other being *not* to use content-disposition in File > Save as. This other solution might be easier to implement and the inconsistency could be fixed faster. It will always be possible to revert once this bug is solved. Please feel free to reopen it if you agree.
We're not going to stop using content-disposition in File > Save as.
QA Contact: drag-drop
Comment on attachment 414866 [details] [diff] [review] Draft Patch Patch itself is fairly straightforward, though the code is mostly duped from helper functions that go along with nsExternalAppHelperService. It would be cool if we could somehow expose those functions more widely, maybe in nsNetUtil? I also can't see an easy way to test this.
Attachment #414866 - Flags: review?(bzbarsky)
Attachment #414866 - Flags: review?(bzbarsky) → review?(Olli.Pettay)
Any chance for some mochitests?
Comment on attachment 414866 [details] [diff] [review] Draft Patch >diff --git a/content/base/src/nsContentAreaDragDrop.cpp b/content/base/src/nsContentAreaDragDrop.cpp >--- a/content/base/src/nsContentAreaDragDrop.cpp >+++ b/content/base/src/nsContentAreaDragDrop.cpp >@@ -86,17 +86,19 @@ > #include "nsIPresShell.h" > #include "nsPresContext.h" > #include "nsIDocShellTreeItem.h" > #include "nsIFrame.h" > #include "nsRange.h" > #include "nsIWebBrowserPersist.h" > #include "nsEscape.h" > #include "nsContentUtils.h" >+#include "nsIMIMEHeaderParam.h" > #include "nsIMIMEService.h" >+#include "imgICache.h" > #include "imgIContainer.h" > #include "imgIRequest.h" > #include "nsContentCID.h" > #include "nsDOMDataTransfer.h" > #include "nsISelectionController.h" > #include "nsFrameSelection.h" > #include "nsIDOMEventTarget.h" > #include "nsWidgetsCID.h" >@@ -816,16 +818,17 @@ nsTransferableFactory::GetNodeString(nsI > > > nsresult > nsTransferableFactory::Produce(nsDOMDataTransfer* aDataTransfer, > PRBool* aCanDrag, > PRBool* aDragSelection, > nsIContent** aDragNode) > { >+ nsresult rv; > NS_PRECONDITION(aCanDrag && aDragSelection && aDataTransfer && aDragNode, > "null pointer passed to Produce"); > NS_ASSERTION(mWindow, "window not set"); > NS_ASSERTION(mSelectionTargetNode, "selection target node should have been set"); > > *aDragNode = nsnull; > > nsIContent* dragNode = nsnull; >@@ -974,22 +977,66 @@ nsTransferableFactory::Produce(nsDOMData > > // grab the image data, and its request. > nsCOMPtr<imgIContainer> img = > nsContentUtils::GetImageFromContent(image, > getter_AddRefs(imgRequest)); > > nsCOMPtr<nsIMIMEService> mimeService = > do_GetService("@mozilla.org/mime;1"); Do you actually need this if content-disposition is used? >+ if (NS_SUCCEEDED(rv) && !mImageDestFileName.IsEmpty()) { >+ mImageDestFileName.ReplaceChar(FILE_PATH_SEPARATOR >+ FILE_ILLEGAL_CHARACTERS, '-'); >+ mImage = img; >+ imgUri = nsnull; >+ } >+ } >+ } >+ } >+ // If we can't use the cache to get the correct filename, at least >+ // fix up the file's extension >+ if (imgUri && mimeService) { Setting imgURI to nsnull is a bit ugly, IMO. Could you just have some PRBool variable to indicate if file's extension needs to be fixed up?
Comment on attachment 414866 [details] [diff] [review] Draft Patch So if you could update the patch. I'll re-review
Attachment #414866 - Flags: review?(Olli.Pettay) → review-
Comment on attachment 414866 [details] [diff] [review] Draft Patch Thanks, it'll be a few days before I have time to update the patch. Any thoughts on avoiding the code duplication? (Most of it is copy pasted from nsExternalAppHelperService.cpp where all of the content-disposition helper functions live)
Attachment #414866 - Attachment is obsolete: true
Yeah, it would be good to use the same code as what nsExternalAppService uses. That one seems to handle also multipart channels. Not sure yet which interface should have the new method.
nsIMIMEService seems to have similar methods and nsExternalAppService implements that interface anyway.
After thinking about this some more, I think the right way to do this is to fix Bug 356086 so that the underlying channel will have a content-disposition-filename that we can pull straight out of imgICache.
Depends on: 356086
Depends on: 589292
This bug (first identified ~10 years ago(!)) is still occurring. The old test case posted earlier still shows the problem: http://forum.magicball.net/attachment.php?s=&postid=163252.

Bulk-downgrade of unassigned, 4 years untouched DOM/Storage bugs' priority.

If you have reason to believe this is wrong (especially for the severity), please write a comment and ni :jstutte.

Severity: minor → S4
Priority: -- → P5

Bug 291837 Opened 16 years ago

FFS. Yeah downgrade will really help get it fixed.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: