Closed Bug 299372 Opened 19 years ago Closed 16 years ago

Content-Disposition headers no longer looked at for Save Link As filename, so it uses e.g. "attachment.cgi" in bugzilla instead of the name of the attachment; Save Page As works fine

Categories

(Firefox :: File Handling, defect, P2)

Product:
Firefox
Component:
File Handling
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 3

People

(Reporter: steffen.wilberg, Assigned: Dolske)

References

(Depends on 2 open bugs, Blocks 1 open bug)

Details

(Keywords: regression)

Attachments

(6 files, 9 obsolete files)

Content Disposition does not working...
112.79 KB, image/gif
Details
Inelegant attempt at using XMLHttpRequest and HEAD
2.51 KB, patch
Details | Diff | Splinter Review
Example for how it's done (by GetRight)
12.99 KB, image/png
Details
An idea for a good solution
16.01 KB, image/png
Details
partial, non-working patch including some nsITransfer changes
26.60 KB, patch
Details | Diff | Splinter Review
[checked in] Patch v.9
37.43 KB, patch
Details | Diff | Splinter Review 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050701
Firefox/1.0+

When saving a file by right-click->save as, the HTTP
Content-disposition/Content-Type headers are no longer respected.

Step to reproduce:
Right click and choose Save Link As: attachment 187911 [details] [diff] [review]

Actual result:
Dialogs asks where to save "attachment.cgi"

Expected result:
Dialogs asks where to save "images-remove.diff"

This still works in Firefox 1.0.4, so it's not a regression from the bugzilla
server move.
Works fine in a 20050627 build.
Looks like the culprit is bug 294759, which changed contentAreaUtils.js quite a
bit on 2005-06-28 07:41.
Save Page As works fine. Only Save Link As is affected.
This is intended, see bug 160454.
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → INVALID
It's not. See bug 160454 comment 108.
It was fixed by bug 263697.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Summary: HTTP header no longer used for Save Link As filename, so it uses e.g. "attachment.cgi" in bugzilla instead of the name of the attachment → Content-Disposition headers no longer looked at for Save Link As filename, so it uses e.g. "attachment.cgi" in bugzilla instead of the name of the attachment
but you're not reffering to save page us, (In reply to comment #4)
> It's not. See bug 160454 comment 108.
> It was fixed by bug 263697.

Well, using both latest seamonkey and latest firefox, i get "attachment.cgi".

Boris, what's the right behavior?
Assignee: nobody → bugs.mano
Status: REOPENED → NEW
Depends on: 294759
igone "but...page as" from comment 5.
Anyway, this is an annoying regression and should be fixed.
Flags: blocking-aviary1.1?
Summary: Content-Disposition headers no longer looked at for Save Link As filename, so it uses e.g. "attachment.cgi" in bugzilla instead of the name of the attachment → Content-Disposition headers no longer looked at for Save Link As filename, so it uses e.g. "attachment.cgi" in bugzilla instead of the name of the attachment; Save Page As works fine
the entire point was not to wait for the server reply anymore...
actually, that's not quite true, there were two points:
- do not send HEAD, since that's poorly supported
- show the dialog immediately, instead of waiting for server reply (esp. when at
connection limit)

I believe that showing the dialog immediately is better behaviour from a user's
point of view.
However, issue #1 could also be fixed differently, e.g. by simply replacing HEAD
with GET or by sending GET, showing the dialog, buffering the data in the meantime.
I'd prefer to wait one or to seconds for the HEAD (or whatever) command to
succeed and to have the Save dialog default to a useful filename instead of
attachment.cgi. On the other hand, it was pretty bad to not show the Save dialog
while two active downloads were running from the same server. In that case the
Save dialog should pop up immediately because finishing the other downloads
could take a while and it's quite confusing if the dialog shows up half an hour
after you wanted to save a link.

I'm having a hard time in finding sites affected by this bug other than
bugzilla. The other sites either have real filenames in the URI, or don't use
the content-disposition header.
Maybe bugzilla could use better filenames as well. Instead of
attachment.cgi?id=... it could probably use /attachment/id/realname.
/attachment/id/ should be supported as well.
Flags: blocking-aviary1.1?
Assignee: bugs.mano → nobody
(In reply to comment #10)
> I'd prefer to wait one or to seconds for the HEAD (or whatever) command to
> succeed and to have the Save dialog default to a useful filename instead of
> attachment.cgi.

Well, we do no HEAD anymore -  that's the primary cause of all that stuff...

> I'm having a hard time in finding sites affected by this bug other than
> bugzilla.

I operate such sites, but the biggest problem I had there (images shown on a
pages not even getting the correct filename on "save image as...") has been
fixed already.

Anyways, issuing the HTTP request, waiting for the HTTP response headers (with a
short timeout, e.g. 1 or 2 seconds) and using those headers for the dialog
sounds very nice, from a user perspective. If possible, we could even show a
disabled dialog and enable it controls only after we got the headers or the
timeout has passed.
I have no clue if that's even remotely possible code-wise, I only look at this
from a user's perspective atm.
(In reply to comment #9)
> However, issue #1 could also be fixed differently, e.g. by simply replacing HEAD
> with GET or by sending GET, showing the dialog, buffering the data in the
meantime.

Don't we start the download while the file picker is already shown (so once you
actually pick a location to save, you already have some part of the file).

If HEAD is replaced by GET, then I suppose the only issue left is the lack of
proper UI notifications while you're waiting for the response.

But new types of windows may confuse users, and you won't have the ability to
queue up downloads, like you can now.

So you either stay with the current method, and get instant download options and
no content-type/content-disposition, or use the "old" method where a max
connections limit or slow servers will delay the appearance of the UI (which is
BAD).

IE pops the download window right away in the "Getting file information" state,
and once that completes the file picker shows up with the "proper" filename.
(In reply to comment #12)
> Don't we start the download while the file picker is already shown (so once you
> actually pick a location to save, you already have some part of the file).

not for save link target as.
Whiteboard: IE-parity
If someone can figure out a sane way to do this without having the "must wait
for other downloads before the filepicker shows" issue, go for it.  I don't see
one, given the info we have (the JS code has no way to tell whether a necko call
will effectively block or not, and nsIWebBrowserPersist can't really be inited
usefully with a partial transfer).
*** Bug 308110 has been marked as a duplicate of this bug. ***
*** Bug 271091 has been marked as a duplicate of this bug. ***
*** Bug 308329 has been marked as a duplicate of this bug. ***
*** Bug 308197 has been marked as a duplicate of this bug. ***
*** Bug 308353 has been marked as a duplicate of this bug. ***
dupe of bug 246837 ??
this should block 1.5 if the release is to target the "mum"-type of user. He/She
shouldn't have to rename a downloaded file - he/she probably doesn't even know
what kind of extension the file should have.
Flags: blocking1.8b5?
*** Bug 309038 has been marked as a duplicate of this bug. ***
(In reply to comment #21)
> this should block 1.5 if the release is to target the "mum"-type of user. He/She
> shouldn't have to rename a downloaded file - he/she probably doesn't even know
> what kind of extension the file should have.

Read the whole bug and especially comment 14 before replying any further.
(In reply to comment #14)
> If someone can figure out a sane way to do this without having the "must wait
> for other downloads before the filepicker shows" issue, go for it.  I don't see
> one, given the info we have (the JS code has no way to tell whether a necko call
> will effectively block or not, and nsIWebBrowserPersist can't really be inited
> usefully with a partial transfer).

perhaps switching the order of 

  if (aDefaultFileName)
    // 4) Use the caller-provided name, if any
    return validateFileName(aDefaultFileName);

and

    if (url.fileName != "") {
      // 2) Use the actual file name, if present
      var textToSubURI = Components.classes["@mozilla.org/intl/texttosuburi;1"]
                                  
.getService(Components.interfaces.nsITextToSubURI);
      return validateFileName(textToSubURI.unEscapeURIForUI(url.originCharset ||
"UTF-8", url.fileName));

alternately, couldn't a regexp be used to extract the name which should at least
minimize the occurences. Even if the filename still is inaccurate I would
imagine it to be closer than a generic 'download.php', 'main.asp', 'bug.cgi' 
filename. 
perhaps a contentAreaUtils.js modification similar to this (lines 822-826):


  try {
    var url = aURI.QueryInterface(Components.interfaces.nsIURL);
-    if (url.fileName != "") {
+    if (url.fileName != ""  && aURI.path.indexOf('?') == -1) {
      // 2) Use the actual file name, if present
      var textToSubURI = Components.classes["@mozilla.org/intl/texttosuburi;1"]
                            .getService(Components.interfaces.nsITextToSubURI); 


at least the filename would be fairly closely related. The files's extension
would is an issue but seems like it could be helped by regexp (of which I know
close to nothing how to use properly)
ok here's my amateurish attempt at filename discovery

contentAreaUtils.js modification similar to this (lines 822-826)
  try {
    var url = aURI.QueryInterface(Components.interfaces.nsIURL);
-    if (url.fileName != "") {
+    if (url.fileName != ""  && aURI.path.indexOf('?') == -1) {
      // 2) Use the actual file name, if present
      var textToSubURI = Components.classes["@mozilla.org/intl/texttosuburi;1"]
                            .getService(Components.interfaces.nsITextToSubURI);


-----------------------
contentAreaUtils.js modification similar to this (lines 841-845)

  if (aDefaultFileName) {
    // 4) Use the caller-provided name, if any
+    if(aURI.path.indexOf("?") > -1) {
+      var fnPattern = /.*[.].*$/; 
+      var pathParts = aURI.path.split("=");
+	for (var i =1; i < pathParts.length; i++) { 
+       	  var possName = pathParts[i].match(fnPattern);
+          if(possName)  aDefaultFileName = pathParts[i];
+    	}
+    }
    return validateFileName(aDefaultFileName);
+ }


  // 5) If this is a directory, use the last directory name



This will attempt to extract a realistic file name. If it doesn't find one it
shouldn't mess any thing up worse than it is currently in FireFox. Feel free to
laugh, fixup, add, delete or critique it. Incidentally with proper modification
I think this probably could be a way to get a best guess name for more complex
truly redirected links instead of just simple links that point to  script,
however that is way over my head.
Flags: blocking1.8b5? → blocking1.8b5+
Flags: blocking1.8b5+ → blocking1.8b5-
duped/dependant in/on bug 285976 , bug 236541 ??
Still broken in 1.5RC2  

Due do various internal tools that rely on php to make files available for download this bug will not allow me to use to 1.5x

This should be considered a regression and a pref should be made  available to enable the old behavior.




(In reply to comment #28)
> Still broken in 1.5RC2  

And it's likely to remain so. Saying so will not get it fixed any faster.

If patches were made available for testing, I (amongst others) could help get a workable solution in place for the drivers to possibly allow a checkin into the 1.8 branch after Fx1.5 ships.

In reply to comment #29
> And it's likely to remain so. Saying so will not get it fixed any faster.

I'm not a coder.  Just an affected user.  

Is it improper to bring this up in bugzilla?
*** Bug 314948 has been marked as a duplicate of this bug. ***
Flags: blocking1.9a1?
*** Bug 315331 has been marked as a duplicate of this bug. ***
*** Bug 317313 has been marked as a duplicate of this bug. ***
*** Bug 319100 has been marked as a duplicate of this bug. ***
(In reply to comment #10)
> I'm having a hard time in finding sites affected by this bug other than
> bugzilla.

Webmail applications are likely to face this problem when downloading mail attachment, at least Kerio WebMail does. From user's point of view it's no problem to wait a while for server reply, usually the user doesn't mention it at all, but it's very annoying to lose file name.

The only workaround seems to develop /attachment/realname mechanism, which is unfortunate - quite a change on webserver level, instead of sending one HTTP header.

Sorry if it's inappropriate to discuss usability issues here. I hope it is not, and I hope Firefox is meant to exist for ... the users :-)
Is it possible to create a pref in prefs.js to enable the old "save link as" behavior?

Thanks,
*** Bug 320144 has been marked as a duplicate of this bug. ***
Another case:
http://europa.eu.int/eur-lex/lex/JOIndex.do?

"If you go to the URL I mentioned in my previous post you will see a page with the Latest issues and Recent issues of the Official Journal. These issues are in L or C version. If you click on any of the numbers let's say L327 then you see a new webpage with all the articles in this issue. To the right of each article is the pagenumber where this article is printed in the paper version of this issue. If you rightclick on this pagenumber and choose "save as..." you can actually download this article in PDF format.
And that's where it goes wrong. There is no blank in the filename. In fact the filename is always "LexUriServ.do" without the quotes off course."

(In reply to comment #38)
> Another case:
> http://europa.eu.int/eur-lex/lex/JOIndex.do?
> ...

I don't know if it helps, but Microsoft had the same problem: http://support.microsoft.com/kb/281119/EN-US/
*** Bug 320810 has been marked as a duplicate of this bug. ***
There is going to be a fix for it because it is a very annoing bug.

Regards.
(In reply to comment #39)
> I don't know if it helps, but Microsoft had the same problem:
> http://support.microsoft.com/kb/281119/EN-US/

that's not at all the same problem.

*** Bug 246837 has been marked as a duplicate of this bug. ***
(In reply to comment #41)
> There is going to be a fix for it because it is a very annoing bug.

You sound very convinced on a somewhat difficult issue - I guess you have a patch ready that already has private reviews by component owners?
(In reply to comment #44)

"somewhat difficult issue" ?

I don't understand what is so difficult about reverting to the previous
behavior?  Or providing a pref for those of us that are impacted to
revert to the previous behavior.

It's not like there is any mystery to the cause of this behavior is
there?
(In reply to comment #45)
> "somewhat difficult issue" ?
> 
> I don't understand what is so difficult about reverting to the previous
> behavior?  Or providing a pref for those of us that are impacted to
> revert to the previous behavior.

Please read this bug report, and even the bug reports linked here, esp. the one that caused this. The old behavior is not an option and a new one is hard to implement, see comment #4, comment #9 and comment #14 - if you don't understand them, please don't add further comments here but leave this to people who do understand the cause.

(In reply to comment #46)

> Please read this bug report, and even the bug reports linked here, esp. the one
> that caused this. The old behavior is not an option and a new one is hard to
> implement, see comment #4, comment #9 and comment #14 - if you don't understand
> them, please don't add further comments here but leave this to people who do
> understand the cause.



I've read the bug reports, and it comes down to a design decision to
move away from long established behavior.

From https://bugzilla.mozilla.org/show_bug.cgi?id=299372#c9
<snip>
 - do not send HEAD, since that's poorly supported
 - show the dialog immediately, instead of waiting for server reply (esp.      \
when at connection limit)

I believe that showing the dialog immediately is better behaviour from a
user's point of view.

</snip>

What I'm trying to impress on you and other Firefox coders is that this
decision is not better behavior from this user's point of view.

I disagree with changing the behavior and not allowing USERS that rely
on this functionality a means to continue using that functionality.

Thank you for your willingness to discuss this problem with a concerned
Firefox user.


(In reply to comment #47)

> Please read this bug report, and even the bug reports linked here [snip]

We've read them repeatedly.  Fully understand that there are those who think this is a low priority bug.  Got it.

> The old behavior is not an option and a new one is hard to implement.

Get real, my friend.  Of course it's an option.  Every other major browser on the market today (yes, I intentionally qualified that with "major") does it correctly.

If the indended consequence of this "enhancement" is to drive away people who frequently right-click to download files, then the change is serving its purpose perfectly. 

I'm amazed that this is even a question. I hate to invoke the "M" word with this crowd, but with something this fundamental, when both IE (including the AOL explorer) and Opera work one way and FFox works another, you've got a tough sell to both new AND experienced users. 

As for me, I never noticed the "long wait" before. I've never noticed it in other browsers, either.  Forcing my users to retype the filename (presuming they even know what it should be) is just plain oppressive, IMHO. 

...and yes, my sites are hosed as a result of this. The organizations (large choral groups) for which I'm creating sites use the right-click save extensively to download (instead of play in their browsers) audio files for rehearsing music. Because I want to do some things when they download a file (for tracking royalty payments) I pull the download through a link to a PHP script. At this point, we have to tell them not to use FFox unless they're feeling up to retyping twenty or thirty filenames each time. 

Apologies in advance if my post lacks the sophistication that seems to be demanded by some of the crusty folks here.  Fix the bug, guys, and we'll stop harping about it.  Geesh. I'm not willing to admit that the IE and Opera development teams are any smarter than the FFox team... It's just *code*, guys.  Figure it out.

Is it possible to make an extension that will revert the browser's behavior to its previous? 

These days I'm using a workaround: View Image followed by the Save As... button from Toolbar Enhancements to save images. But it's a nasty kludge, and having "hires.pl" come up instead of the image name looks awfully amateurish. Given the lengths BenGoodger was willing to go in other areas to avoid Firefox looking amateurish, I'm surprised this has gone unfixed as long as it has.

Sorry for adding to the bugspam; think of it as a Quaker farmer's four-by-four.
How can the QA contact for this bug be contacted?

It states "file.handling@firefox.bugs" which is not a valid email.

This bug is still Assigned to 

"Nobody's working on this, feel free to take it <nobody@mozilla.org>"

The Status of this bug is still New despite being opened on 2005-07-01.

Thirteen other bugs have been duped to this bug despite nobody being assigned to this bug.

If this bug is not gonna be fixed or worked on, then please update the status and close the bug.

Thanks,
(In reply to comment #10)

> I'm having a hard time in finding sites affected by this bug other than
> bugzilla.

The "Download" link for attachments in Gmail is a good example.
Here is the fix....


1. Close FireFox ... 

2. At open firefox\chrome\toolkit.jar in http://7-zip.org/ 

3. edit /content/global/contentAreaUtils.js

=== to add ===


function getContentDisp(url){
  var httpReq = new XMLHttpRequest();
  httpReq.open("HEAD", url, false);
  httpReq.send("");
  return httpReq.getResponseHeader( "Content-disposition");
}

=== add ends ===




=== now modify following ===

function internalSave(aURL, aDocument, aDefaultFileName, aContentDisposition,
                      aContentType, aShouldBypassCache, aFilePickerTitleKey,
                      aChosenData, aReferrer, aSkipPrompt)
{


  if (aSkipPrompt == undefined)
    aSkipPrompt = false;



=== to ===




function internalSave(aURL, aDocument, aDefaultFileName, aContentDisposition,
                      aContentType, aShouldBypassCache, aFilePickerTitleKey,
                      aChosenData, aReferrer, aSkipPrompt)
{

  var prefSvc = Components.classes["@mozilla.org/preferences-service;1"]
                          .getService(Components.interfaces.nsIPrefService);
  prefSvc = prefSvc.getBranch(null);

  if (!aContentDisposition){
     try {
       if(prefSvc.getBoolPref("browser.download.filename_from_httpheader"))
           aContentDisposition = getContentDisp(aURL);
     } catch(ex) { }
  }


  if (aSkipPrompt == undefined)
    aSkipPrompt = false;


=== end of code modification ===



start browser...


on browser goto "about:config"
create a boolean key
>> browser.download.filename_from_httpheader = true


you are all ready to go...



Issue:-

I use Async httpReq = false 
Other wise we need a complete rewrite of contentAreaUtils.js
So it freeze for few sec if the server connection is slow...

I wish there was a least a sleep() function in JavaScript.


I am not finding enough time to maintain my own extension 
http://macroeditor.mozdev.org/
So please as somebody to make it an as extension, if needed.

PS: I took only 2 hour to do this hack, so somebody can fix this fast.


Question:--

Just like "imageCache" in
>> var imageCache = Components
>>                   .classes["@mozilla.org/image/cache;1"]
>>                       .getService(imgICache);

Do we have cache Service for any URL ?

why would a sleep function help? you'd still hang the UI for that time.
Just a sleep wont give us everything we want.
But...

 then I can change .open() to
  httpReq.open("HEAD", url, true);


function getContentDisp(url){
  var httpReq = new XMLHttpRequest();
  httpReq.open("HEAD", url, false);
  httpReq.send("");

  while(httpReq_not_over){
     sleep_1_sec;
  }

  return httpReq.getResponseHeader( "Content-disposition");
}

This will give OS a chance to paint Firefox GUI 
(I assume, even though the JavaScript wont run,
as I read Mozilla JavaScript is not multithreaded)


If we can get JavaScript Engine as Reentrant and with 
Sleep function, we can do multitasking, that is almost 
all what one want to do with Multithreading. 
That will be the best we can get if not multitasking.
(In reply to comment #54)
> This will give OS a chance to paint Firefox GUI 
> (I assume, even though the JavaScript wont run,
> as I read Mozilla JavaScript is not multithreaded)

No, it won't. UI happens on the same thread as JS.

Furthermore, it's not guaranteed that this will even terminate.

I tried the fix in #52. It fails on this website:

http://jccc.afis.osd.mil/LBOX/full/1278073.jpg

Or I screwed up.
(In reply to comment #55)
I may be wrong (as I don't know the architecture of JS Engine), I still thinks there will be a way to add "sleep" in Javascript.
Give me a day I will write demo which made me think that way.


(In reply to comment #56)

issue 1: You need to vist http://jccc.afis.osd.mil/ first.
(may be to get Cookie UserName=Not%20registered )
Other wise you get file not found!!! (that realy su**s)
IMHO: they should have coded it in a better way, than a highschool kid.

issue 2: Image already have file name 1278073.jpg

issue 3: no "Content-disposition" found

NB: I am also not much happy with my fix as it sometime give a few sec freeze, which is unacceptable. 

Headers:- (using http://livehttpheaders.mozdev.org/ cool extension !!!)

-----
http://jccc.afis.osd.mil/LBOX/full/1278073.jpg

GET /LBOX/full/1278073.jpg HTTP/1.1
Host: jccc.afis.osd.mil
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051220 Firefox/1.6a1
Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5
Accept-Language: en-us,en;q=0.5
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Connection: keep-alive
Cookie: XBeenTold=true; XacceptsGSTcookies=true; UserName=Not%20registered

HTTP/1.x 200 OK
Server: Netscape-FastTrack/2.01
Date: Fri, 23 Dec 2005 15:39:42 GMT
Accept-Ranges: bytes
Last-Modified: Fri, 23 Dec 2005 15:39:41 GMT
Content-Length: 248864
Content-Type: image/jpeg
Connection: keep-alive
----------------------------------------------------------

To get to an image similar to the one I referenced in comment #56, go to this page:

http://jccc.afis.osd.mil/images/sres.pl?Lbox_cap=1278358&dir=Photo&vn=&ttl=051223-A-5745M-022&ref=

and click on the download HiRes file link. The image has a name (1278358.jpg) but when I right-click on the image and select Save Image As . . ., I get hires.pl for the title. Switching via IE Tab to IE, I get a title of 1278358, with a jpg suffix. Returning to Firefox, I can get the title right by clicking on my Save Page... button from the tbx extension.


I created an extension from the fix in Comment #52
It can be downloaded here:
http://www.extensionsmirror.nl/index.php?showtopic=4812
(In reply to Comment #59 )
tnx..

(In reply to Comment #58)
> http://jccc.afis.osd.mil/images/sres.pl?Lbox_cap=1278358&dir=Photo&vn=&ttl=051223-A-5745M-022&ref=
> and click on the download HiRes file link. The image has a name (1278358.jpg)

Issue:
http://jccc.afis.osd.mil/images/hres.pl?Lbox_cap=1278358&dir=Photo
redirects to 
Location: http://jccc.afis.osd.mil/LBOX/full/1278358.jpg

But XMLHttpRequest is not giving me to a chance to trap redirection, 
or the there is no property like
XMLHttpRequest.url
XMLHttpRequest.location
XMLHttpRequest.src


(In reply to Comment #55)
I have demo for showing multitask in Firefox
http://w3l.sourceforge.net/js_multitask/multitask_test.html

ie, Firefox can process another event in the middle of one event.


Also it will be cool if Javascript Web designer can get a way
to automaticaly click the Alert OK button after a delay.
also for confirm() and prompt()

like 
var msg = "this waits for 5 seconds";
var msg_title = "wait msg";
var default_value = 5;
var wait_for_sec = 5;
var wait_actionCancel = true;
var default_actionCancel = true;

alert(msg, msg_title, wait_for_sec);
confirm(msg, msg_title, wait_for_sec, wait_actionCancel, default_actionCancel );
prompt(msg, default_value, msg_title, wait_for_sec, wait_actionCancel, default_actionCancel );

Where wait_for_sec = 0, null, undefine it waits for user action.
wait_actionCancel - if true click Cancel
default_actionCancel - if true make default action of prompt as Cancel

also good will be confirmEx, confirmCheck in 
http://xulplanet.com/tutorials/xulqa/q_msgbox.html

(In reply to comment #59)
> http://www.extensionsmirror.nl/index.php?showtopic=4812
pl. change max version to 1.6 
(I wish max version specification should be outside *.xpi)

please stop discussing random extensions on this bug. this database is for discussing fixes in mozilla code, not workarounds that require extensions. thank you.
(In reply to comment #62)
> please stop discussing random extensions on this bug. this database is for
> discussing fixes in mozilla code, not workarounds that require extensions.
> thank you.
> 

When can we users of Firefox expect this to be fixed in mozilla code?  All of the feedback so far seems to indicate that it will never be fixed.

Can this bug be moved out of NEW status?  It's been open since July.

Is there a reachable QA contact?

Thanks,
(In reply to comment #63)
> (In reply to comment #62)
> When can we users of Firefox expect this to be fixed in mozilla code?  All of
> the feedback so far seems to indicate that it will never be fixed.
> 
> Can this bug be moved out of NEW status?  It's been open since July.
> 
> Is there a reachable QA contact?
> 
> Thanks,
> 

Please read https://bugzilla.mozilla.org/page.cgi?id=etiquette.html

NEW is the default state of bugs after they have been confirmed. It will not change status unless it is ASSIGNED, or RESOLVED as FIXED, INVALID, WONTFIX or WORKSFORME. Now it is none of those hence the NEW status. 

Another proposal for attacking this via UI:
Add an "Use the file name recommended by server" checkbox to the Save Dialog, which'll cause the File Name textbox to be disabled. This will download to a temporary file name and eventually rename to whatever name was given in Content-Disposition.

One concern with this would be the case when the Content-Disposition name overwrites an existing file. If an overwrite is due, we'll need to show a confirmation dialog upon the finish of a download.
Problem #1: we use native filepickers, so we can't add an arbitrary checkbox, but I mostly like the idea.  We could handle renaming the same way we do with the automatic download option, and skip the confirmation after the download.

Too bad its not really feasible...
Mike, most native file pickers can be extended. For a sample of Gtk's file picker being extended, see GEdit with its "Character Coding" selector. Examples of the Win32 file picker being extended are also innumerable.
Frankly, I see this method I've proposed as a sad compromise, UI-wise.

With a naive user approach, ignoring for now whatever lackings in Necko, there are two UI cases:
1. Users with "Save all files to this folder" option: I see no reason not to use Content-Disposition in this case.
2. Users with "Ask me where to save every file" option: Why not add an incomplete entry to the download manager and only ask for file name when Content-Disposition is available? (Note that before the download begins, our download entries are already incomplete, because we don't have Content-Length.)

There's certain UI charm to an immediate (not waiting for any network response) Save Link As dialog. It gives people who download a bunch of links a feeling that something is happening. However, doesn't this crowd mostly use "Save all files to this folder" already?

--

Of course, no amount of blabbering in this bug would solve the infrastructure deficiencies described by Boris Zbarsky. We'll need to attack them as soon as we decide on the UI.
*** Bug 316337 has been marked as a duplicate of this bug. ***
*** Bug 323888 has been marked as a duplicate of this bug. ***
Ok, here's what I'm seeing:

previously firefox waited to make sure the file existed on the server before displaying the "save as" dialog

NOW if you right-click, Firefox will immediately prompt you to choose a filename, sometimes before firefox knows for sure that the file exists.

why is that behaviour better?

why can't a 'waiting for download...'  dialog pop up.   that way the user knows immediately the download has been queued, but firefox also gets to wait for confirmation from the server on the existence of the file and it's actual filename.

my 2c
This Firefox extension provides a workaround for this bug (#299372):

http://www.extensionsmirror.nl/index.php?showtopic=4812

Contrary to what the extension description states, this is a workaround, not a fix.  (Sorry for the pedantry.)
(In reply to comment #71)
> This Firefox extension provides a workaround for this bug (#299372):
> 
> http://www.extensionsmirror.nl/index.php?showtopic=4812
> 
> Contrary to what the extension description states, this is a workaround, not a
> fix.  (Sorry for the pedantry.)
> 
The workaround itself if buggy.  If attempt the stemps mentioned in Comment #58 you will notice that it works right only for IE 6 and fails with Firefox + the extension.


Content Disposition does not working.
See ScreenSot in attach.
*** Bug 326781 has been marked as a duplicate of this bug. ***
Hello, i just wanted to show the people unable to find an example this link:  http://www.nvnews.net/vbulletin/showpost.php?s=da1cbe1afe7bc7fcd80b728489024763&p=776697&postcount=2

There is a file there, drag it to the download manager and you will see the problem.

The old implementation, regardless if it was better or not, worked correctly.  The new one does not.  I am not a coder but i am a user and I am a QA Eng for a day job -- this is Clearly NOT good.  IMO this is a broken feature and should warrant a high level of visibilty. 

thanks.
(In reply to comment #75)

Opera does this correctly, it is unfortunatley the workaround that I have had to use for months, while waiting for this behavior to be corrected.


> Hello, i just wanted to show the people unable to find an example this link: 
> http://www.nvnews.net/vbulletin/showpost.php?s=da1cbe1afe7bc7fcd80b728489024763&p=776697&postcount=2
> 
> There is a file there, drag it to the download manager and you will see the
> problem.
> 
> The old implementation, regardless if it was better or not, worked correctly. 
> The new one does not.  I am not a coder but i am a user and I am a QA Eng for a
> day job -- this is Clearly NOT good.  IMO this is a broken feature and should
> warrant a high level of visibilty. 
> 
> thanks.
> 


This may be a reason to raise the severity/priority of this bug, it's not just when you right click on a link and say Save Link As.   If you open a redirecting script link that takes you to an image, then right click on that image, the filename will be the script name even though the url and image are fully loaded.



example:
visit one of the Fark.com photoshop contests
http://forums.fark.com/cgi/fark/comments.pl?IDLink=2053584
click on the (Some Guy) link next to the photoshop icon and description.

a new window with just a jpg should pop up.  the url bar shows that the filename is "20060508wap_rbergerPJ01_450.jpg" but if you right click on the image and save as, it pre-populates the image name with "go.pl"  the script that fark uses to capture click counts and redirect you to the image.
aran, that sounds like a different bug. This bug talks about Save Link As.
*** Bug 338695 has been marked as a duplicate of this bug. ***
Everyone, isn't this bug (bug 299372) a duplicate of bug 224209?
(In reply to comment #80)
> Everyone, isn't this bug (bug 299372) a duplicate of bug 224209?

No, that bug is about the displayed title, this bug is about the "Save as..." dialog (a regression of the behavior described in bug 224209 comment 2).
*** Bug 340912 has been marked as a duplicate of this bug. ***
Anyone know why it works with a DNN based site?
Content Disposition Extension does not work with MAF Extension :-(
For the love of god, can the FireFox team please fix this!? The Content Disposition extension doesn't seem to work with the latest FireFox 1.5.0.4 update. The time that the new behaviour saves by not requesting for the filename from the server is completely LOST to the time it takes for me to type in the proper name! 
could it be that this bug was "silently" fixed in 1.5.0.5 or 1.5.0.6 ?

i just noticed that this seems to work again?!?
(In reply to comment #86)
> could it be that this bug was "silently" fixed in 1.5.0.5 or 1.5.0.6 ?

No, I'm running 1.5.0.6 and it's still broken.
I do confirm that this *really* annoying issue is still there in 1.5.0.6 .
It's even braking me in my day-to-day job...
Flags: blocking-firefox2?
Not going to block the release on this as the fix would likely require new strings or hard changes to Necko. Definitely want a fix, though, if possible. One suggestion myk had was to wait for 3-5s for a response. If no response, do what we do now. If response, yay!
Flags: blocking-firefox2? → blocking-firefox2-
Whiteboard: IE-parity → IE-parity [would take patch]
Why does this require complex changes or strings? Here's a mock-up with an XMLHttpRequest that does a HEAD request and a simple timeout.

http://www.cusser.net/misc/firefox/head/head_test.html
http://www.cusser.net/misc/firefox/head/head_test.js

You'll have to download these to your desktop due to requiring cross-domain privileges. Check out the code first if you're worried that I'm evil or something.

If the best-case scenario for Firefox 2.0 is something like this, please do it. It might not be a real solution, but it keeps everyone happy until there is one.
Ben Basson, thanks for moving this issue forward, but did you actually read the bug?
"we do no HEAD anymore -  that's the primary cause of all that stuff..."
Yup, and also see comment 14.
Well, my test was in response to comment 89, I can't think of any other realistic way of mitigating this bug in the time-frame. 

AFAIK, HEAD checking was removed because:
1. It doesn't always work (due to a lack of widespread support, although I've personally never seen it unsupported).
2. The dialog can take a long time to display in some circumstances.

A happy middle-ground solution would be re-implementing HEAD (in the way that my example shows) with a short timeout (perhaps 1 second). That way at least in a lot of cases the real filename would be given and the dialog would display either immediately or after a maximum wait of 1 second, regardless of whether the request was successful or not (although it clearly should be in a lot of cases, otherwise we wouldn't have so many comments and votes on this bug).

If this isn't an acceptable short-term solution then I don't really see this being fixed until someone gets down and dirty with Necko for 3.0. 
fwiw I don't think that any necko work is required here
Here's a basic working patch. It's not elegant or clever.
Improvements needed:

1. Lower time-out if possible. 
2. Needs to prevent two "Save" dialogs appearing with short time-outs.
3. Reduce code redundancy where possible.

It's someone else's turn to push this forward if they want to. I hope this helps.
Oh yeah, the double dialog problem can probably be solved by having the timeout function call req.abort(). My bad.
One user-friendly and efficient solution would be something like this: display the Save As dialog immediately, as Firefox currently does, but modify it so that, instead of showing the best-guess for the filename, something like "acquiring filename..." would be displayed over a grey background. This message would be replaced with the real filename once it's acquired, unless the user wished not to wait. In that case, clicking/double-clicking within the text area would convert this message into Firefox's best guess for the name, which the user can leave as is or modify. This way, the browser doesn't stall (as it currently often does with Content Disposition extension), the user need not wait to choose a directory while the filename is being acquired, and the user can easily override this need to wait. An arbitrary timeout would also not be required.
First off: this entire thread consists of two things -- some Google engineers jerking off and doing burn-outs in the parking lot, some Mozilla fanbois arguing **** rebuttals to try and prove that "their way is the Right Way" (even though they implemented the Wrong Way and don't want to go back to the Right Way), and some generic coders posting workarounds and extensions as if to say "The solution is available.  IMPLEMENT IT NOW".  All I see in response is reluctance, with no justified reason ("see comment 14" -- better yet, how about See Figure 1?).

Secondly, and more important -- because I'm sick of seeing people post the same thing over and over -- let's get down to the facts:

Mozilla "Foundation", you have cash, you have the resources.  FIX IT.  PEOPLE DONATED MONEY TO HAVE YOU *FIX THIS KIND-OF *****.  Stop making excuses and DO YOUR JOB.  Don't type a response to this, don't delete it, don't spend ANY EFFORT on replying -- simply COMMIT A FIX IN CVS (pick one of the many solutions and codesnippets provided!, push it out to a SYN build, do THOROUGH testing (more than enough sites provided here can be used for verification) and *CLOSE THE BUG*.  Is it that hard?  I'll repeat my question: IS IT REALLY THAT HARD?

This is the *EXACT* sort-of situation that shows why open-source *FAILS*.
(In reply to comment #93)
> Well, my test was in response to comment 89, I can't think of any other
> realistic way of mitigating this bug in the time-frame. 
> 
> AFAIK, HEAD checking was removed because:
> 1. It doesn't always work (due to a lack of widespread support, although I've
> personally never seen it unsupported).
> 2. The dialog can take a long time to display in some circumstances.

I work as a sales support engineer for an "output management" software company. Virtually all of our user interface is user requested delivery of files in various formats (standard and custom) delivered by cgigetf.exe or cgionline.exe binaries (yes, the Unix version also uses these names.)  The output is then supposed to "magically" open in the viewer of choice (Word, Excel, Acroread, GhostView, custom) and it does in IE (all versions) Netscape 7.x-, Safari, Opera and I believe in Mozilla 1.0.  This bug or a related bug in Firefox has made it completely incompatible with our application.  Every file now has to be saved on the desktop AS CGIGETF.EXE and then manually renamed to .XLS, .PDF, .DOC, .PS, .RTF, .MHT, the list goes on and on.

I used to use Netscape exclusively due to problems with IE security and have recently switched to FireFox, believeing that FireFox was the future.  Unfortunately, I can't use it now.  After reading the posts thus far, I have come to the conclusion that the old way was not only the *right* way but the *only* way.  I wouldn't try to preserve the new method at all but flush it as a horrible mistake.  Granted, it probably solved some inconvenience that you have experienced (though I've never experienced it) but this is like using decapitation to cure a headache.

I hope no one is taking offense at my comments, but up to now the Mozilla project has brought great benefit to mankind at a minimum by keeping Microsoft worried enough about IE's quality by comparison to fix a great number of IE's problems.  You have kept all the drones in the offices who have "standardised" on IE from suffer overmuch for it.  You've saved an entire generation of Javascript (OK, ECMA-script) programmers from inconsistent scripting ala Microsoft. You've steered the world toward your standard for HTTPRequest, by itself a boon for all future web developers.

Please don't underestimate the cost of this bug.  Over 50% of the web is delivering documents which are NOT html and much of that is provided by .exe, .asp, .jsp, .cgi, and what-not.  If Mozilla dies it will very likely be due to this problem.  There is no "happy middle-ground."

> A happy middle-ground solution would be re-implementing HEAD (in the way that
> my example shows) with a short timeout (perhaps 1 second). That way at least in
> a lot of cases the real filename would be given and the dialog would display
> either immediately or after a maximum wait of 1 second, regardless of whether
> the request was successful or not (although it clearly should be in a lot of
> cases, otherwise we wouldn't have so many comments and votes on this bug).
> 
> If this isn't an acceptable short-term solution then I don't really see this
> being fixed until someone gets down and dirty with Necko for 3.0. 
> 


Jeremy, did you actually get down and dirty with the cause of this "bug"?
Did you read bug 16054 which intententionally introduced this "bug"?
Did you read the dreaded comment 14, which explains loud and clear why we need a bright idea here, rather than a crude fix?

If you're sick of people posting the same thing over and over, then instead of posting "this is bullshit!!1" kind of comments, do actually read the existing comments, and then post a comment if you have a technical solution to the technical issue at hand.
(In reply to comment #100)
> If you're sick of people posting the same thing over and over, then instead of
> posting "this is ****!!1" kind of comments, do actually read the existing
> comments, and then post a comment if you have a technical solution to the
> technical issue at hand.


I agree about the bitching, but honestly I don't think there is a "technical" solution to this problem since it is not a technical issue.  It's more of a UI implementation issue.

Either the browser waits to confirm the filename or it doesn't.   Saving based on the link url is useless to the user and useless to the "service provider".

I don't understand why firefox can't just bring up a dialogue that says "waiting for server" just like every other browser.

I beg someone to provide a "technical reason" against that.
(In reply to comment #101)
> it is not a technical issue.  It's more of a UI implementation issue.

Amen, aran.

Having read the entire thread (a couple of times over the last year), the trade-off appears to be: 

"fast but often fatally wrong" 

versus

"imperceptibly delayed and almost always right"

For heavens sake, sacrifice a few milliseconds, stop the jihad against HEAD, and provide a useful product.

I can't tell you how much time I've wasted fielding hundreds of calls from users of my sites asking how they can get Firefox to recognize filenames for mp3 and pdf content they want to download.  It's sad that after more than a year now the only answer I can give them is "use another browser, because Firefox wants so badly to be *fast* that they don't care about being *right*."

As aran says, the problem here is not technical at all.  The problem here is a fanatical devotion to hatred of HEAD.  Give it up.  Let the anger out.  Relaaaaax.  Throw in an error check to handle the 0.000002% chance that you won't find a Content Disposition, and get on with it.

Until then, Firefox will remain an unusable (and damn irritating) browser for anyone downloading anything through redirected links.

It's just code, guys.  Letting go of your hate is always the hardest part.  :-)
Mozilla user since before the M releases.

This bug and the arrogant response to my feedback chased me away.




There is a need for a better solution here, that's why the bug is still open.  But the previous implementation would randomly not work, only to work much later without context or reason.  If the the tradeoff was "work reliably with a 200ms delay" vs. "work reliably without file names but with no delay" then it would be ridiculous.  But the tradeoff was "mostly work with random hangs or unexplainable delays" vs. "always work, sometimes without filenames" and that's a coin toss for most people.  I'd rather be a little wrong and predictable/reliable than more right and somethings spectacularly wrong.

As a note, the doom and gloom responses should recall that left-clicking on the link generally works fine (to note a convenient example, go to Bug 350690 and click on the first attachment after noting the URL).  The vast majority of sites I encounter are sane enough to have "Download file" links which work just fine.  I do miss this, but the bugs that came along with it make it an OK tradeoff for me in the short term.
(In reply to comment #104)
> But the tradeoff was "mostly work with random hangs or
> unexplainable delays" vs. "always work, sometimes without filenames" and that's
> a coin toss for most people.  I'd rather be a little wrong and
> predictable/reliable than more right and somethings spectacularly wrong.

Why not let the user decide, for their environment which behavior causes less pain and then let the user select the behavior with a pref?

In my environment the new behavior is always wrong.  Php scripts that call files for download always fail with the new behavior.

The code for both approaches exists, this bug is no longer about making something work.  It has degnerated into an argument that those in control of the mozilla code insist on winning.



so 1.0.4 was the last release with the old behavior?

i'm using FF or Firebird and Phoenix(?) since the beginning, and i can't remember that downloading of files that way was ever broken. maybe a small delay.

i'll grab an old build later and try it out.

"mostly work with random hangs or unexplainable delays" vs. "always work, sometimes without filenames"

so i see this totally different.

i would request this bug to get the "blocking FF2" flag.
this bug is just ludicrously, embarassing for mozilla.
but please think twice before someone denies that blocking request. thanks!

"works with small delays" vs. "works fast but always an unexpected result" (namely missing filename).
Flags: blocking-firefox2- → blocking-firefox2?
Even if we wanted to block on this, we couldn't solve it in the time that we have left. This has already been minused once (see comment 89) for Firefox2, please leave it that way.

Hopefully it'll get fixed in Gecko1.9
Flags: blocking-firefox2? → blocking-firefox2-
@mike

of course. please forgive me. setting this as blocking was meant more to bring this bug to attention.

this issue arose somwhere in the 1.0 branch and by the sheer amount of people added to cc and the votes makes people think this gets ignored.

normally when such basic functionality gets broken one would think it'll be fixed ASAP but now we're living with this "workaround" for more than a year. with one major release (1.5) and a soon another major coming with 2.0. without getting it fixed again.

is it really necessary to wait again for the next major? why not fix it with the next _possible_ minor release?

alot of bad blood came up with this issue. and in corporations this bug alone is reason enough to stay away from firefox. i heard such comments from customers myself at my workplace.

cheers,
ralf

I'm frankly a little confused by one thing.  If people want the old behavior, all they need to do is to click on the link instead of doing "save link as" (unless the link points to an HTML page or such).   So it's not like there's no way to get the old behavior...

It also seems that people misunderstood the HEAD thing.  It's not that "HEAD is not supported, so we don't get a filename out of it", it's that "HEAD is not supported, so server responds with an HTTP 404 for HEAD requests and nothing on that server is saveable".  Similarly, it's not that "there is a bit of a delay", but "there are plenty of reported cases of users waiting for _minutes_ before the dialog comes up."  Trust me, the tradeoff to stop hitting the server before putting up the dialog was not made lightly...

This is not to say that we don't need a better fix, of course.  We do.  What we have now was checked in _very_ early in the Gecko 1.8 cycle and was not really supposed to make it to any final releases.  Fundamentally, the problem is that the "save as" code has been unowned for years (going on 5 years now).  A contributor appeared who was willing to fix the HEAD issues, but he's disappeared again...

To help this bug get fixed, the best thing would be finding someone to work on it.  The next best thing would be not adding more "me too" comments -- we know this is a problem; it's just that there is no "easy fix".
A few other things (which were all stated before in a scattered manner, but people seem to be missing them).

Any solution to this bug needs to balance the real user issue of wanting good filenames with the real user issues of needing to terminate in finite time and to allow saving data that actually exists.  The latter issues are why we had to stop using HEAD; they were _very_ prevalent.

The "allow saving real data" business can be handled by ignoring all parts of the HEAD request except the Content-Disposition, or better yet only looking at the Content-Disposition if the HEAD comes back 200/OK and falling back to other means of determining the filename otherwise.  This is probably the best way  to go.

The "terminate in finite time" problem more or less requires giving up on the HEAD request after a timeout, which means unpredictable behavior depending on how slow the network is being.  This is probably OK if the very common case is predictable (so the timeout is large compared to most HEAD response times).  If we don't show any UI to the user indicating that we're waiting on the filename, I'd say that about 200ms is the absolute most we can allow before showing the filepicker.  After that, users realize that they clicked and nothing happened and start clicking again.  If we give the user some feedback that indicates that we saw the click and are working on it, that number can probably get bumped to 1000ms or so -- at this point the only real limit is user frustration with waiting, not lack of user feedback.
just some quick thought...

the current way it works is 

1. clicking "save link as"
2. filebrowser dialogs opens
3. choosing dir to save in and probably changing filename
4. click save button
5. download manager starts

what about:

1. clicking "save link as"
2. download manager starts (should be the perfect visual feedback to tell the user the browser is working on it)
3. filebrowser dialogs opens up as soon the filename is available
4. choosing dir to save in and probably changing filename
5. click save button informs the download-manager where to move the finished file
We are now at comment #112. Can't you just do whatever it is that IE6 is doing? It generally gives the correct title when you Save As . . .
> I'd say that about 200ms is the absolute most we can allow before showing the
> filepicker.  After that, users realize that they clicked and nothing happened
> and start clicking again.  If we give the user some feedback that indicates
> that we saw the click and are working on it, that number can probably get
> bumped to 1000ms or so

If, in addition to the feedback, we give the user a way to cancel the HEAD request (f.e. a button to "Cancel Looking Up the Filename"), we should be able to wait longer.

Rather than debate the pros/cons of doing it one way or the other, since this is going nowhere, how about a middle-ground?  Specifically:

Default is old behavior using the HEAD *method*; however, a timer is set within which time a response must be received from the server.  If the server does not respond in time, Firefox happily writes the filename according to current behavior and all the villagers rejoice!

Can't we all just get along? :-)
p.s. If you do implement this way, and I do believe it is the *best* approach, given the intense standoff, please add a pref for "power users" to tweak the timer.

Wouldn't a "download starting" dialogue with a spinning icon be more than sufficient.   When the server finally responds to the the request.  Bam! User is cooking and the save as dialogue opens.  Or if the server responds 404, the "download starting" changes to "file not found"   additionally, as with any dialogue box, the user can "Cancel" if they feel the download is taking too long and it is not worth their time. 

Perhaps I'm off base, but I feel like this is a simple, elegant catch-all solution.  Is there any reason why it shouldn't be done this way?  I understand if there is no one coding it yet, but can we at least agree on a proper solution?

aran 
Those of you who know the GetRight download manager (which I use with FlashGot), you'll recognize this screenshot. When you choose to download a file, this window will pop - and if you click on the "Get Size & Date" button it will download the file header, including the real filename.

If it detects something like "attachment.cgi?i=234" or the like, it just downloads the header automatically, because ultimately no one who downloads a url that ends with "attachment.cgi?i=234" wants the filename to be "attachment.cgi".

So maybe just make a piece of code to detect if downloading the header is necessary or not? It may seem like a workaround but I'm not sure I would call it that.
Newest Getright is even a bit more clever (as it seems), Download Dialog appears instantly with file named attachment.cgi while HEAD request runs in Background.
If it succeeds, filename is renamed from attachment.cgi to correct filename.
So the User does not have to wait for any dialog, can save immediately as attachment.cgi (or with already correct filename if server reply was fast enough) or choose to wait some more time to see if response still comes.
All that without any new Button, Dialog etc.  
> we should be able to wait longer.

Myk, the problem is that if I want to save 9 large files from a site, I would like to start the download of the 9th and go to lunch without having to wait for the 1st to load.  If you wait for the HEAD request, you have to wait until one of the first 8 files finishes downloading, basically.

> Can't you just do whatever it is that IE6 is doing?

If someone can clearly explain exactly what they're doing, it should be seriously considered, yes.  I have yet to see such a clear description.  I've been asking for one since 1999 (with IE 5 and IE6).
> If someone can clearly explain exactly what they're doing, it should be
> seriously considered, yes.  I have yet to see such a clear description.  I've
> been asking for one since 1999 (with IE 5 and IE6).

As determined by visual inspection, packet sniffing, and tailing the server logs, it seems that IE6 limits you to two concurrent downloads per host. 

the first and second Right Click And Save (RCAS) pop up the "Getting File Information" dialogue.  

When it obtains the HEAD info, it pops up a "Save File" dialogue with the proper filename.

Any RCAS after that pops up the "Getting File Information" dialogue which sits there waiting for the first downloads to finish.

you can try it at 
http://www.unclecrispy.com/media.php?cat=Crispy&sub=kingcrispy
(note that the first 23 downloads are from one server, 24-37 are from a different server)


Seems like an arbitrary limit but a workable solution ... the "Getting File Information" dialogue just sits there until it's turn in the queue, once it gets the HEAD information, firefox can start the background downloading.

aran
> Myk, the problem is that if I want to save 9 large files from a site, I would
> like to start the download of the 9th and go to lunch without having to wait
> for the 1st to load.  If you wait for the HEAD request, you have to wait until
> one of the first 8 files finishes downloading, basically.

Right, so my suggestion is to give you the option of canceling the HEAD request, so in your case you could cancel those HEAD requests and initiate all downloads before heading out to lunch.

In other words, when a user initiates a link save, and we start the HEAD request, we would display UI indicating that we're "in the process of getting information" about the file.  Once the HEAD request finishes, we'd replace it with the standard UI for saving the resource to disk.

But the initial UI would contain a button for "continuing without this information", so users who didn't want to wait for the HEAD request to complete could still save the file immediately, while those willing to wait could do so.

There are two solutions of merit here.

One is enabling/disabling this option via preferences.

The other is having Firefox display a "getting file name" message with the option to save immediately. This however, may require a separate dialog box to indicate this and then later popup the save dialog box as it's a pain to check whether the file name has been retrieved if it's all in one dialog. For me, the moment I see the word "save" i just click.

The first is obviously a better solution. At most, the second solution can also be incorporated into the first solution.
i.e. The options available via preferences (terms to be renamed for laymen):
-Wait for Content-Disposition Headers
-Guess best file name
-Let me choose during the download
Has anyone noticed that the Send Link command (File > Send Link, context menu > Send Link) always seems to pick up the page's title? 
Send Link, when not on an actual link, is a link to the current page (seems weird, I don't remember that string changing) but if you do it on a link it doesn't.
The point is that Send Link not only generates an email with a link to the current page, it does so With The Correct Title of The Page.

I have never had to wait while Send Link generated a page title. The only time I do not get the correct title is when I have navigated to a print page, and that occurs only on "minor league" websites.
(In reply to comment #124)
> I have never had to wait while Send Link generated a page title.

You're completely misunderstanding this bug. When you choose "Send link" on a page, *you've already loaded the page*, so obviously getting its title is fast. When you click "Save link as", getting the filename of the file pointed to by the link requires an HTTP request, over the network, which can take a long time.
*** Bug 360811 has been marked as a duplicate of this bug. ***
Blocks: 360811
Perhaps this is a closer-to-home example, although it certainly sounds like the overall intention is to fix the problem, and the question is just how to do it.  I'm trying to download the Lightning calendar extension for Thunderbird, from http://www.mozilla.org/projects/calendar/

I'm running Firefox 1.5.0.8 (I'm happy to try 2.0, but from the comments posted so far, it sounds like nothing would be different) on Mac OS X 10.4.  If I click on the link to download the .xpi file, Firefox tries to install it itself and fails.  If I right-click or control+click and choose "Save Link As...", then I get a 3.4MB .htm file that doesn't install successfully even if I rename it to .xpi and try to install it (perhaps because it gets saved as a text file?).  I can't find anything in the Firefox preferences that disables automatic handling of .xpi files, so the only possible way for me to download a Mozilla-distributed extension to a Mozilla product is to use a non-Mozilla browser.
I also get the wrong behaviour when I open the attachment link in a new tab, then drag the URL icon to a file manager.
Same thing for SeaMonkey. Is there any reason why this is declared only as FF bug?
I did not find any Suite or Core bug for this problem. IMHO this bug should be
changed to Core/File Handling.
Hardware: PC → All
please fix this, i have people now asking me why my website doesn't work correctly and i hate to have to give them the advice "well your gunna have to use internet explorer"...  a good deal of my site is now unusable to firefox users because of this bug... when did this start happening again?  was it with 2.0 the bug came back?
Here is an idea of how it can be done for those who set it to ask where to save.
When the user clicks on a download link, the download manager pops up like in the image. In place of the file type icon, there is a throbber. (Obviously it's animated.) "(Tentative)" is appended to the filename, and under it, the message "Getting filename..." is visible. (The "Open" link is there because I forgot to photoshop it out. Ignore it.) When the user clicks the "Save" link, the filepicker is shown with the tentative filename (e.g. attachment.cgi). Once the filename is retrieved, the filepicker is popped up, but this time with the correct/final filename. As a result, the user can choose to either start the download immediately (but risk getting the wrong filename), or wait until the filename is retrieved.
This requires us to either:
1. Re-enable HEAD and use it to get the filename, or
2. Use GET instead, and also use it to start prefetching the file while the user is choosing the filename. (This is the preferred option. IE also uses this method for determining the filename, and then it starts downloading in the background, until the user chooses the filename.)

Also, this requires a UI change, but I think it's more than justifiable.
>while the user is choosing the filename

A correction to my previous post: "choose the filename" was intended to be "choose the save path" in both places where I wrote it.
Flags: blocking-firefox3?
Also, I don't know whether this should remain in its current component or moved to the Download Manager component. Ideas? 
Related to earlier Bug 255382?
no, save image is different.
Comment on attachment 253490 [details]
An idea for a good solution

Something is wrong with Bugzilla. PNGs are supposed to be image/png, not image/x-png.
Attachment #253490 - Attachment mime type: image/x-png → image/png
Strange.  I'm no longer on the CC list, as I removed myself sometime ago, but I'm still getting email from this bug.

How do I get myself off this list for good?  Is it maybe because I voted for this bug?  Do I have to take my vote away to be removed?

Click the My Votes link, and uncheck this bug.
(In reply to comment #140)
> Click the My Votes link, and uncheck this bug.
> 

Thanks
Clearing 1.9 nomination (this is in toolkit)
Blocking for Firefox 3
Assigning to dmose
Assignee: nobody → dmose
Flags: blocking1.9a1?
Flags: blocking-firefox3?
Flags: blocking-firefox3+
(In reply to comment #142)
> Clearing 1.9 nomination (this is in toolkit)

Sorry, I meant "Clearing 1.9a1 nomination (this is deprecated)"
Those keys are right next to each other.
I can't believe I have to recommend IE....

What is the time frame for Firefox 3?
Target Milestone: --- → Firefox 3 beta1
Target Milestone: Firefox 3 M7 → Firefox 3 M8
Target Milestone: Firefox 3 M8 → Firefox 3 M9
No longer blocks: 372972
removed myself
add myself to cc list
might make M9, per dmose, but does not block beta 1
Target Milestone: Firefox 3 M9 → Firefox 3 M10
Does this bug also require fixes to SeaMonkey, Camino, and other Gecko-engine browsers?  If so, please update the Product to Core.   
It requires no fix anywhere else AFAIK, the affected code is core code that applies to all apps. This bug may be better off in Core > File Handling though because of that.
Priority: -- → P1
Whiteboard: IE-parity [would take patch] → IE-parity
Priority: P1 → P2
Attached patch "Save Link As..." patch, v1 (obsolete) — — Splinter Review 
This patch changes "Save Link As..." to always use the external helper app service, which means that the filename always comes from the same GET used to download the file.

The also means that, for HTTP links, once network.http.max-connections (currently defaulted to 2) for the site in question is hit, any subsequent downloads will queue invisibly until a connection frees up, after which the file picker may open (depending on pref; the pref is now attended to, unlike the old code path).

There are a number of related bugs to be spun off, 

* bump network.http.max-connections to 8.  This is likely to also help Tp, and will make the silent queuing much more of an edge case.  Should do this for Firefox 3.

* allow dl manager to accept incomplete nsITransfers and pass one in.  This will give instant UI feedback for queued transfers in the download manager, making them no longer silent.

* give the user some UI frob to abort waiting for the filename, if we think it's possible to design meaningful UI for this.  IE has a dialog for this case, and it solves the "I want to start 9 downloads with 'Save Link As' and go to lunch." case.

* change "Save As..." to "Save" in the case where that pref is set (already filed).

I'll file and/or link to the remaining bugs here.

mconnor, can you r the toolkit/browser changes?

biesi, can you r/sr the very minimal uriloader changes?
Attachment #293766 - Flags: ui-review?(mconnor)
Attachment #293766 - Flags: superreview?(cbiesinger)
Attachment #293766 - Flags: review?(mconnor)
(In reply to comment #153)
> The also means that, for HTTP links, once network.http.max-connections
> (currently defaulted to 2) for the site in question is hit, any subsequent
> downloads will queue invisibly until a connection frees up, after which the
> file picker may open (depending on pref; the pref is now attended to, unlike
> the old code path).

Isn't the default for network.http.max-connections 24 at the moment?

http://mxr.mozilla.org/firefox/source/modules/libpref/src/init/all.js#567
(In reply to comment #154)
>
> Isn't the default for network.http.max-connections 24 at the moment?

Yes; I was confused, and it appears that the conversation on IRC the other day  that led me to suggest this was confused as well.  I suspect that network.http.max-persistent-connections-per-host is what we're really bumping up against here, and that it'll probably be worth raising that default from 2 to something higher as well (at least a couple of other browsers seem to have that number set at 4 these days, though IE has it set at 2).
er, -per-server, rather.
Comment on attachment 293766 [details] [diff] [review]
"Save Link As..." patch, v1

+          extHelperAppSvc.doContent(aRequest.contentType, aRequest, window,

you want doc.defaultView here, I'm pretty sure

I'm also not sure that this will work correctly, normally the windowcontext is a docshell. in particular, I _think_ that a window doesn't give out nsIPrompt objects so error reporting will not work

not really a fan of your style of defining the objects here, I think it'd be nicer to use the more normal way of defining a function and setting the .prototype and using new Foo()

+      if (aInterface.equals(Ci.nsIAuthPrompt))

please also include nsIAuthPrompt2, e.g. http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/contentAreaUtils.js#297

also, don't use else-after-if

and here too use doc.defaultView instead of window

+    channel.loadFlags |= Ci.nsIRequest.LOAD_BYPASS_CACHE |

I always thought that was somewhat odd but whatever

+    if (channel in Ci.nsIHttpChannel)

you mean instanceof?


-      var autodownload = prefs.getBoolPref("browser.download.useDownloadDir");
+      try {
+        var autodownload = prefs.getBoolPref("browser.download.useDownloadDir");
+      } catch (e) {}

that change seems unrelated

r+sr=biesi on the uriloader changes, sr=biesi on everything
Attachment #293766 - Flags: superreview?(cbiesinger) → superreview+
Comment on attachment 293766 [details] [diff] [review]
"Save Link As..." patch, v1

>+    if (channel in Ci.nsIHttpChannel)
instanceof?
(In reply to comment #157)
> (From update of attachment 293766 [details] [diff] [review])
> +          extHelperAppSvc.doContent(aRequest.contentType, aRequest, window,
> 
> you want doc.defaultView here, I'm pretty sure
> 

That would be the default view of the XUL document associated with the toplevel window, right?

> I'm also not sure that this will work correctly, normally the windowcontext is
> a docshell. 

So is the right thing to pass really the docshell associated with the top-level XUL window?  If so, how do we get that?

> in particular, I _think_ that a window doesn't give out nsIPrompt
> objects so error reporting will not work

Given that this is in Firefox-only code, are there errors in Firefox that still trigger nsIPrompt (as opposed to just using error pages)?  I'm trying to figure out how I would test this.
(In reply to comment #157)

> +      if (aInterface.equals(Ci.nsIAuthPrompt))
> 
> please also include nsIAuthPrompt2, e.g.
> http://lxr.mozilla.org/seamonkey/source/xpfe/communicator/resources/content/contentAreaUtils.js#297

Fixed, and I removed the nsIPrompt support, since I was only assuming it was needed, and the thing you link to doesn't implement it.

> and here too use doc.defaultView instead of window

The link you pasted above uses a "window" object.  Furthermore, with a window object, authenticated save-link-as works, so I'm inclined to leave this one alone.

> +    channel.loadFlags |= Ci.nsIRequest.LOAD_BYPASS_CACHE |
> 
> I always thought that was somewhat odd but whatever

I'm just preserving existing behavior here; I agree that this seems kinda odd.

> -      var autodownload = prefs.getBoolPref("browser.download.useDownloadDir");
> +      try {
> +        var autodownload =
> prefs.getBoolPref("browser.download.useDownloadDir");
> +      } catch (e) {}
> 
> that change seems unrelated

Unfortunately, it's not.  Without this fix, the helper app dialog falls over when called from this context.  Not sure if/why this problem hasn't been hit from existing callers.

(In reply to comment #160)
> > -      var autodownload = prefs.getBoolPref("browser.download.useDownloadDir");
> > +      try {
> > +        var autodownload =
> > prefs.getBoolPref("browser.download.useDownloadDir");
> > +      } catch (e) {}
> > 
> > that change seems unrelated
> 
> Unfortunately, it's not.  Without this fix, the helper app dialog falls over
> when called from this context.  Not sure if/why this problem hasn't been hit
> from existing callers.
That change needs to be made anyway :)
Attached patch "Save Link As..." patch, v2 (obsolete) — — Splinter Review 
Patch v2; all comments addressed _except_ that I'm still using window rather than document.defaultView, since it seems to work and feels cleaner.  I'll change it upon request, though.

After playing with this on the other side of a slow link, it feels like taking this will more or less require that we also make and take the download-manager / nsITransfer changes as well, as the lack of any sort of feedback while waiting for the headers feels pretty bad.
Attachment #293766 - Attachment is obsolete: true
Attachment #294545 - Flags: ui-review?(mconnor)
Attachment #294545 - Flags: superreview+
Attachment #294545 - Flags: review?(mconnor)
Attachment #293766 - Flags: ui-review?(mconnor)
Attachment #293766 - Flags: review?(mconnor)
Whiteboard: IE-parity → [has patch][needs review mconnor][IE-parity]
Target Milestone: Firefox 3 M10 → Firefox 3 M11
Re-adding bz to the bug, as I'm interested in his thoughts on the latest...
So this is reintroducing the "no feedback after doing 'save link'" bug that was one of the major reasons we stopped doing HEAD requests, right?  This was actually a very serious problem; bumping from 2 to 4 for the number of connections, even if not violating a SHOULD-level HTTP requirement doesn't really solve it; the people who hit the limit of 2 tend to hit the limit of 4 as well.  That makes me suspect that this approach in general is somewhat wrong...  It might be a good idea to really sit down and implement something akin to what IE does.

On the pure-code-technical side, ss there a reason you're not passing the correct docshell for the window context, other than "seems to work and feels cleaner"?  There's no guarantee that it either works in all cases or will continue to work.  Even if you do want to use a window (which I don't recommend), using the chrome window is just wrong in this case; it'll break correct tab-switch-on-alert behavior, etc, etc.  You can get the docshell from the window by doing a getInterface for nsIWebNavigation, iirc.

Similar for your getInterface implemenation: why is it handing out a prompt for the chrome window instead of the relevant content window?

Other than those issues, this looks fine.

(In reply to comment #164)
> So this is reintroducing the "no feedback after doing 'save link'" bug that was
> one of the major reasons we stopped doing HEAD requests, right?  This was
> actually a very serious problem; bumping from 2 to 4 for the number of
> connections, even if not violating a SHOULD-level HTTP requirement doesn't
> really solve it; the people who hit the limit of 2 tend to hit the limit of 4
> as well.

Yes, the failure behavior is the same, and sucks.  However, the hope is that it will be a lot less frequent because most of the time, GET works.

> That makes me suspect that this approach in general is somewhat
> wrong...  It might be a good idea to really sit down and implement something
> akin to what IE does.

I believe this approach is compatible with what IE does.  Two of the spinoff bugs I suggested in comment 153 would make our behavior/functionality equivalent to that of IE:

* allow dl manager to accept incomplete nsITransfers and pass one in.  This
will give instant UI feedback for queued transfers in the download manager,
making them no longer silent.

* give the user some UI frob to abort waiting for the filename, if we think
it's possible to design meaningful UI for this.  IE has a dialog for this case,
and it solves the "I want to start 9 downloads with 'Save Link As' and go to
lunch" case.

In my mind, the main question is whether we could live without those two spinoff fixes for Firefox 3.
I hope to meet with Beltzner tomorrow to sort that out.  Note that the spinoff fixes may be a non-trivial amount of work.

I'll fix the review comments you brought up.

(In reply to comment #165)
> * allow dl manager to accept incomplete nsITransfers and pass one in.  This
> will give instant UI feedback for queued transfers in the download manager,
> making them no longer silent.
That would in fact be non-trivial.
sdwilsh: yeah, I actually started a bunch of that work earlier, but decided it would be useful to get the basic "Save Link As..." patch working alone and consider accepting it by itself.  Here I've attached the non-working combined patch which has both the "Save Link As..." chances as well as a bunch, but not all, of the dl mgr changes necessary.  How long it would take to get those dl mgr changes finished and shaken out is not clear to me.  Might just be a day or two.  Might be more.  Any thoughts on that that you have would be appreciated....
It looks right so far.  Obviously, we'd need to come up with a way to update the UI (probably with nsIDownloadProgressListener?), and this would need to have tests for both the front end and the back end to land with it.  The download manager work by itself could probably be done within a day or less.  I'm not sure how you have exthandler (didn't look) handling all this fun though, so I don't know how much work is involved there.
> Yes, the failure behavior is the same, and sucks. 

The problem is that the _success_ case sucks.  Once I have two downloads going, doing "save link as" does absolutely nothing... so I do it again... and it does absolutely nothing again.  Then later I get a bunch of filepickers.  It's a terrible user experience.

> However, the hope is that it will be a lot less frequent because most of the
> time, GET works.

The GET is irrelevant.  The badness is inherent in waiting on the server before putting up the filepicker, because the server isn't going to respond if other network activity is in progress.

> I believe this approach is compatible with what IE does.

I've never seen IE wait 30 minutes to pose a filepicker on a save as operation.  If there is no server response in a reasonable amount of time it just derives a filename without using the content-disposition information, as I recall.  Worth double-checking.

> IE has a dialog for this case, and it solves the "I want to start 9 downloads
> with 'Save Link As' and go to lunch" case.

Exactly.  I think we need to solve this case as well, and I think we need to solve it for Firefox 3.  Based on the volumes of user feedback I've seen in various bugs, this is a bigger problem than the non-use of content-disposition filenames.  Of course if we have usability data to the contrary I'd love to hear about it.

I'm strongly opposed to taking this patch if it will reintroduce the "nothing happens" behavior on "save link as", basically.
Depends on: 301186
(In reply to comment #169)
> I've never seen IE wait 30 minutes to pose a filepicker on a save as operation.

STR:
 - go to http://www.bootiesf.com/ or some other website with large files
 - right click, "Save Target As..." on 4 items, don't hit OK on any file picker
 - wait half an hour for the 4th filepicker to come up :)

Or hit OK on the file pickers, but still notice that the 4th file picker won't come up until one of the first 3 downloads complete.

Where IE wins, as dmose has pointed out, is that they show *some* form of feedback. Specifically, they show the "File Download" window:

  File Download

  ( )  []                       [/

  Getting File Information:
  [fullFileNamehere] from [tldHere]

  Estimated time left:
  Download to:
  Transfer rate:
  [ ] Close this dialog box when download completes

                     { Open } { Open Folder } (( Cancel ))


that just sits there, paper flying from planet to waiting folder, until a new connection comes up.

Regardless: they're definitely waiting to get the file information from the server before showing the filepicker, and doing so with fewer maximum connections per server than this patch proposes.

bz is totally right, though, that from a user experience perspective, the most important thing is to get immediate feedback on the action, even if that feedback is saying "please wait a second while we do the thing ...". Since we're late in the Fx3 game here, and since I'm looking at a patch in hand that sort of makes things better, I'm gonna lowbar/highbar this:

Low Bar:
 - try to get the real filename
 - if we have maxed out on connections or if we can't get in in 1s, then fall back to the "best guess" name as we do now

High Bar:
 - make the proposed changes to add items to the DM immediately even without the filename
 - when we can't get the filename right away, have it appear as "Getting file information from %eTLD+1..." in the download manager
 - once we can get the filename, show the filepicker, but start downloading the file and just move it when we're done (so that we don't waste time if the user's gone AFK in between clicking on the link and us showing the filepicker)
(In reply to comment #170)
>  - once we can get the filename, show the filepicker, but start downloading the
> file and just move it when we're done (so that we don't waste time if the
> user's gone AFK in between clicking on the link and us showing the filepicker)

An important point: don't wait until completion before moving the file, do it right after the user chooses a filename/location, at least if the chosen location is on a different partition than the temporary folder.Imagine this situation:
-An user has Windows installed on Drive C:, and the temporary folder is in its default location, C:\Users\AppData\Local\Temp (Vista, XP's temp path is much longer.) Mozilla's profile is in the AppData folder.
-Drive C: is an 5GB partition on a 200GB drive, that holds nothing but the OS itself. It has 1GB of free space. Programs are installed on drive D: (50GB), while data and downloads are stored on drive E: (the rest). This way, Windows can be easily reinstalled without any data loss when it dies.
-The user decides to download a large file (DVD image of his/her favorite Linux distro, 4,2 billion digits of PI from the latest calculation record, or maybe even an illegal HD-DVD movie, it doesn't matter), that is too large to fit in the Temp folder (remember, it's on the tiny C:!), but fits nicely in the target folder (on E:). IE would immediately throw an error message, saying "Not enough free space!" However, if the user tries to download the file with any current or previous version of Firefox/Mozilla, it works, the temporary file is created on E:, and not on C:, as in Internet Explorer.
-If we download to C:, then move to E:, we lose this advantage over IE! That is, the size of downloadable files will be limited by the space on drive C:, regardless of what drive the user picks as the download target. So, here is my proposal:
1. First, cache the download in the temp/profile folder (on C:), until the user picks the file. If C: fills up (or maybe if it has only a certain free space left, like 100 MB), pause downloading.
2. When the user picks the target file/folder, immediately move the temporary file to that drive. Once the move is done, resume downloading (if a pause was needed due to no free space on C:).

So, the key here, is that we shouldn't limit the max download size to the free space of C:.
(In reply to comment #171)
> -The user decides to download a large file (DVD image of his/her 
> favorite Linux distro, ......

I dont see any advantage over IE now.
One day I accidentally start downloading a 4GB+ Linux distro to a 1.5GB free space drive. Estimated time told 5 hours or more, so when to sleep.
Next day morning only I found it was not download due to lack of space.

* Why Firefox did not warn me when it already knew the file was 4GB+ and free space is only around 1.5GB?

In this case pausing download is just dont enough.

That's not what I'm talking about. In my comment, the user doesn't try to download the 4GB ISO to a partition with only 1.5GB free space. Instead, the user tries to download to a partition with 80GB free (call it "Drive D:"), but drive C: only has 1.5GB of free space. Just because Drive C is full, Firefox shouldn't deny downloading to other drives (that have plenty of free space). However, Beltzner's suggestion would regress this, making downloads larger than the free space on drive C: fail even if the target drive isn't C:. That's what my comment covers.
I strongly agree with Stefanik.  Just the other day a member of my team had to download a DVD ISO on a remote system that had a small, non-replaceable C: drive, and a large D: drive.  It was extremely maddening to learn after several hours of download with IE that the download had failed due to insufficient space on the C: drive, when it was the D: drive we'd asked the file to be saved to.  Our solution, sure enough, was to install Firefox and re-do the download with it.  Please do not remove this significant advantage!

(And even in the absence of out-of-space issues, having to wait for large files to be copied to where you told them to be downloaded to from where they (for some reason incomprehensible to the average user) actually were downloaded to is quite annoying, especially with slower drives.)
Comment on attachment 294545 [details] [diff] [review]
"Save Link As..." patch, v2

clearing uir since I gave some design notes; renominate me if that's wrong of me
Attachment #294545 - Flags: ui-review?(mconnor)
No longer blocks: 393488
DMose - B4 comin up - we doing ok?
Attached patch "Save Link As..." patch, v3 (obsolete) — — Splinter Review 

        Attachment #294545 -
        Attachment is obsolete: true

        Attachment #294545 -
        Flags: review?(mconnor)

    
    

    
  
Comment on attachment 304357 [details] [diff] [review]
"Save Link As..." patch, v3

This patch fixes the doc.defaultView problem that biesi and bz mentioned earlier, and implements the low-bar solution suggested by beltzner, with the maximum time to wait set to 150ms.  This isn't great, because lots of sites just don't respond that quickly.  Right now, testing from my home DSL in San Francisco to bugzilla.mozilla.org, I'm only able to regularly get the filename if I bump the delay to about 250ms.

beltzner, I noticed that your original low bar solution suggests timing out at 1s rather than 150ms (as we discussed on IRC today).  Should we consider trading more frequent filename correctness for the slight irritation that the long wait will cause?

        Attachment #304357 -
        Flags: ui-review?(beltzner)

        Attachment #304357 -
        Flags: superreview?(cbiesinger)

        Attachment #304357 -
        Flags: review?(mconnor)
Whiteboard: [has patch][needs review mconnor][IE-parity] → [has patch][needs ui-r beltzner, r mconnor, sr biesi][IE-parity]

    
    

    
  
We could also conceivably make the wait time a pref.

    
    

    
  
"Should we consider trading more frequent filename correctness for the slight irritation that the long wait will cause?"

Speaking as a longtime user who despises having to cut and paste titles - yes.

"We could also conceivably make the wait time a pref." - also yes, please.

    
    

    
  
Could we rather add a tabbrowser notification,

[ Retrieving file name..    (Cancel Download) ] after a shorter timeout?

I'm quite sure the download wouldn't proceed much if we couldn't retrieve the actual file name anyway.

    
    

    
  
(In reply to comment #181)
> Could we rather add a tabbrowser notification,
> 
> [ Retrieving file name..    (Cancel Download) ] after a shorter timeout?
> 
> I'm quite sure the download wouldn't proceed much if we couldn't retrieve the
> actual file name anyway.
this code lies in exthandler - wouldn't that be a bad dependency?

    
    

    
  
Would it be the first time for us to use odd things like observers and callback? :p

Given that we're so close to the last beta, that's probably a Fx4-thing anyway.

    
    

    
  
Comment on attachment 304357 [details] [diff] [review]
"Save Link As..." patch, v3

Yeah, that default seems too low. Let's make it a tweakable pref and set it back to 1000ms and see if that ends up feeling really bad on slower sites.

        Attachment #304357 -
        Flags: ui-review?(beltzner) → ui-review-

    
    

    
  

      
      
      
      

        Attached patch
          
          
        "Save Link As..." patch, v4 (obsolete)
        — 
          — Splinter Review
      

    


  
Updated to be pref-based, defaulting to a 1-second delay.

        Attachment #304357 -
        Attachment is obsolete: true

        Attachment #305240 -
        Flags: ui-review?(beltzner)

        Attachment #305240 -
        Flags: superreview?(cbiesinger)

        Attachment #305240 -
        Flags: review?(mconnor)

        Attachment #304357 -
        Flags: superreview?(cbiesinger)

        Attachment #304357 -
        Flags: review?(mconnor)

    
    

    
  
Comment on attachment 305240 [details] [diff] [review]
"Save Link As..." patch, v4

+    // nsIExternalHelperAppService.doContent,which will wait for the

missing space after the comma

+      onStartRequest: function saveLinkAs_onStartRequest(aRequest, aContext) {
+
+        // just in case the timer hasn't yet fired

is there a specific reason you're starting functions with an empty line?

+        if(!Components.isSuccessCode(aRequest.status))

missing space after the "if"

but is it really a good idea to not give the user any indication of the error?

+        // XXX theoretically should allocate a private nsresult to do this 
+        // signaling so that collision with any other callers of onStopRequest
+        // is impossible

this specific code seems like a poor choice though. how about NS_ERROR_BINDING_ABORTED? that seems less likely to collide with unrelated errors. (it's also the one that pressing stop uses)

     if (NS_FAILED(rv) || !fileToUse) {
       Cancel(NS_BINDING_ABORTED);
+      mTempFile->Remove(PR_FALSE);
       return NS_ERROR_FAILURE;

why?

        Attachment #305240 -
        Flags: superreview?(cbiesinger) → superreview+

    
    

    
  
(In reply to comment #186)
> +      onStartRequest: function saveLinkAs_onStartRequest(aRequest, aContext) {
> +
> +        // just in case the timer hasn't yet fired
> 
> is there a specific reason you're starting functions with an empty line?

Generally I do that when it feels like it makes the code more readable.  I can remove it if you like, however.

> +        if(!Components.isSuccessCode(aRequest.status))
> 
> but is it really a good idea to not give the user any indication of the error?

Hmm, good point.  I'll try and figure out the right thing to do here...

> +        // XXX theoretically should allocate a private nsresult to do this 
> +        // signaling so that collision with any other callers of onStopRequest
> +        // is impossible
> 
> this specific code seems like a poor choice though. how about
> NS_ERROR_BINDING_ABORTED? that seems less likely to collide with unrelated
> errors. (it's also the one that pressing stop uses)

I initially did that, but it collides with the case when someone intentionally aborts the download through the UI.  I'm not sure why NS_ERROR_NOT_AVAILABLE isn't a good one: the filename wasn't available, which is why we're aborting.

>      if (NS_FAILED(rv) || !fileToUse) {
>        Cancel(NS_BINDING_ABORTED);
> +      mTempFile->Remove(PR_FALSE);
>        return NS_ERROR_FAILURE;
> 
> why?

Hmph; trying to remember why I did this; it was in the first iteration of the patch. I'll experiment and report back.



    
    

    
  
(In reply to comment #187)
> I initially did that, but it collides with the case when someone intentionally
> aborts the download through the UI.  I'm not sure why NS_ERROR_NOT_AVAILABLE
> isn't a good one: the filename wasn't available, which is why we're aborting.

I'm mainly worried that unrelated code might cause this error to happen. it's a pretty generic one.

> >      if (NS_FAILED(rv) || !fileToUse) {
> >        Cancel(NS_BINDING_ABORTED);
> > +      mTempFile->Remove(PR_FALSE);
> >        return NS_ERROR_FAILURE;
> > 
> > why?
> 
> Hmph; trying to remember why I did this; it was in the first iteration of the
> patch. I'll experiment and report back.

OK. FWIW, if it is needed I think this is the wrong place, if necessary the code in nsExternalAppHandler::Cancel should be adapted.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        "Save Link As..." patch, v5 (obsolete)
        — 
          — Splinter Review
      

    


  
Addresses the issues biesi found; carrying forward sr.

        Attachment #305240 -
        Attachment is obsolete: true

        Attachment #305659 -
        Flags: ui-review?(beltzner)

        Attachment #305659 -
        Flags: superreview+

        Attachment #305659 -
        Flags: review?(mconnor)

        Attachment #305240 -
        Flags: ui-review?(beltzner)

        Attachment #305240 -
        Flags: review?(mconnor)

    
    

    
  
(In reply to comment #187)

> > but is it really a good idea to not give the user any indication of the error?

Good catch.  This iteration of the patch now uses a prompt identical to that used in the download manager code. 

(In reply to comment #188)
> (In reply to comment #187)
> > I initially did that, but it collides with the case when someone intentionally
> > aborts the download through the UI.  I'm not sure why NS_ERROR_NOT_AVAILABLE
> > isn't a good one: the filename wasn't available, which is why we're aborting.
> 
> I'm mainly worried that unrelated code might cause this error to happen. it's a
> pretty generic one.


OK, I chose an error code in the uriloader space, and documented it both here and in nsURILoader.h in the hopes of avoiding collision.

> > >      if (NS_FAILED(rv) || !fileToUse) {
> > >        Cancel(NS_BINDING_ABORTED);
> > > +      mTempFile->Remove(PR_FALSE);
> > >        return NS_ERROR_FAILURE;
> > > 
> > > why?
> > 
> > Hmph; trying to remember why I did this; it was in the first iteration of the
> > patch. I'll experiment and report back.
> 
> OK. FWIW, if it is needed I think this is the wrong place, if necessary the
> code in nsExternalAppHandler::Cancel should be adapted.

It looks like this isn't actually necessary any more; I've removed it.


        Attachment #305659 -
        Flags: ui-review?(beltzner) → ui-review+

    
    

    
  
Comment on attachment 305659 [details] [diff] [review]
"Save Link As..." patch, v5

ui-r=beltzner, as per comment 184
Whiteboard: [has patch][needs ui-r beltzner, r mconnor, sr biesi][IE-parity] → [has patch][needs r mconnor][IE-parity]

    
    

    
  
Comment on attachment 305659 [details] [diff] [review]
"Save Link As..." patch, v5

r=me

one nit is that the ioService getService call is wrapped improperly, but otherwise looks good to go

        Attachment #305659 -
        Flags: review?(mconnor) → review+
Whiteboard: [has patch][needs r mconnor][IE-parity] → [has patch][has reviews][IE-parity]

        Attachment #305659 -
        Flags: approval1.9b4?

    
    

    
  
Comment on attachment 305659 [details] [diff] [review]
"Save Link As..." patch, v5

a1.9b4=beltzner

        Attachment #305659 -
        Flags: approval1.9b4?

        Attachment #305659 -
        Flags: approval1.9b4+

        Attachment #305659 -
        Flags: approval1.9+

    
    

    
  
this should have a test
Flags: in-testsuite?
Flags: in-litmus?

    
    

    
  
Robert: you're right; it should.  I suspect this would take me at least 1-2 days full-time (and I'm notoriously bad at predicting this stuff); it might be faster for someone who has spent more time than I with their heads inside the guts of the test frameworks.

Unfortunately, I've got a ton of stuff on my plate, and I don't foresee having that amount of time in one stretch anytime soon (I'm extremely interrupt-driven at the moment).

So this is a crummy situation.  I suspect the right trade-off here is to land now and accept the extra risk, because it gives more time to cope with any fallout.

Alternately, if anyone reading this is interested in volunteering to take this on, that'd be great.  For what it's worth, I think that in an ideal world, the following things about save-link-as would get tested:

* traversing a link to an as-yet-authenticated server that requires auth
* traversing a link that's busted provides error notification
* traversing a link that doesn't return info quickly falls-back to the old behavior
* traversing a link that does return it quickly uses it


    
    

    
  
Comment on attachment 305659 [details] [diff] [review]
"Save Link As..." patch, v5

Re-requesting 1.9b4 approval, given the new points made about testing in the bug.

        Attachment #305659 -
        Flags: approval1.9b4+ → approval1.9b4?

    
    

    
  
Comment on attachment 305659 [details] [diff] [review]
"Save Link As..." patch, v5

Please file a follow on bug for adding tests and nominate.

        Attachment #305659 -
        Flags: approval1.9b4? → approval1.9b4+

    
    

    
  


  
Patch as landed.  The only differences are the indentation issue mconnor pointed out in his review and a typo-fix inside a comment.

Checking in uriloader/base/nsURILoader.cpp;
/cvsroot/mozilla/uriloader/base/nsURILoader.cpp,v  <--  nsURILoader.cpp
new revision: 1.147; previous revision: 1.146
done
Checking in uriloader/base/nsURILoader.h;
/cvsroot/mozilla/uriloader/base/nsURILoader.h,v  <--  nsURILoader.h
new revision: 1.29; previous revision: 1.28
done
Checking in uriloader/exthandler/nsExternalHelperAppService.cpp;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp,v  <--  nsExternalHelperAppService.cpp
new revision: 1.367; previous revision: 1.366
done
Checking in uriloader/exthandler/nsExternalHelperAppService.h;
/cvsroot/mozilla/uriloader/exthandler/nsExternalHelperAppService.h,v  <--  nsExternalHelperAppService.h
new revision: 1.91; previous revision: 1.90
done
Checking in uriloader/exthandler/nsIExternalHelperAppService.idl;
/cvsroot/mozilla/uriloader/exthandler/nsIExternalHelperAppService.idl,v  <--  nsIExternalHelperAppService.idl
new revision: 1.26; previous revision: 1.25
done
Checking in browser/app/profile/firefox.js;
/cvsroot/mozilla/browser/app/profile/firefox.js,v  <--  firefox.js
new revision: 1.294; previous revision: 1.293
done
Checking in browser/base/content/nsContextMenu.js;
/cvsroot/mozilla/browser/base/content/nsContextMenu.js,v  <--  nsContextMenu.js
new revision: 1.42; previous revision: 1.41
done

        Attachment #305659 -
        Attachment is obsolete: true

        Attachment #306624 -
        Attachment is patch: true

        Attachment #306624 -
        Attachment mime type: application/octet-stream → text/plain

    
    

    
  
OK, I've spun off and nominated bug 420401 about unit tests.

I've also spun off bug 420410 about the high bar UI solution as well.

Status: NEW → RESOLVED
Closed: 19 years ago → 16 years ago
Resolution: --- → FIXED
Depends on: 420481

    
    

    
  
Dan, there was a message in m.d.a.firefox that implied that this patch removed the option of picking the filename to save with.  Is that actually the case, or did this patch just change the default presented in the filepicker to include the content-disposition info?

    
    

    
  
In some cases, yes, and it leaves things in sort of an odd state:

As the code currently sits in the tree, if we get headers back soon enough, we'll use the external helper app service for the download.  Unlike the code that was in use until yesterday, the external helper app service properly respects the Preferences/Main/Downloads/"Save Files To" vs. "Always Ask" setting.  This means that it doesn't have the problem described in bug 257143 and in more detail in bug 244961.

If we don't get the headers back soon enough, we use the old code path, which does still have that problem.

So this fixes bug 257143 some of the time, at the cost of regressing the "Save Link As..." brings up a dialog semantic.

Bug 244961 comment 11 option 3 (split the context-menu item into two) would be one way of dealing with this.  Option 2 (change the context-menu depending on the pref) would be arguably better, but we don't know ahead of time which code path is going to be chosen.

    
    

    
  
Although splitting the context-menu would then make it so that this bug would only be fixed for "Save Page" and not "Save Page As...".  Hmph.  I'll poke around some more and see what I can figure out.

    
    

    
  
I think we should certainly ask for the location/filename by default, with reasonable default values (the default download dir and our best shot at the filename).  Is there a bug to track this regression fix?

    
    

    
  
maybe this should add a new reason value, and maybe we should also pass the reason to promptForSaveToFile? that way the UI can show the dialog for this case when it normally wouldn't.

    
    

    
  
This seems to have caused bug 420481 -- backed out the patch for b4 to avoid the regression this late (sorry!)... marking blocking final again.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Firefox 3 beta3 → Firefox 3

    
  
Assignee: dmose → dolske
Status: REOPENED → NEW

    
  
Whiteboard: [has patch][has reviews][IE-parity] → [IE-parity][needs fixed patch]
Target Milestone: Firefox 3 → Firefox 3 beta5

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch, v.7 (obsolete)
        — 
          — Splinter Review
      

    


  
This seems to fix the problem that caused v.6 to be backed out... Without the explicit QI, aRequest.contentType is undefined. This causes doContent() to fail to find a nsIMIMEInfo [or, more specifically, the call there to mimeSvc->GetFromTypeAndExtension() fails to find one], and it thinks it ran out of memory.

I'm a little unclear on what to do about the issues raised in comments 200-204... Land this as-is, or is more work needed?

        Attachment #306624 -
        Attachment is obsolete: true

    
  

        Attachment #308371 -
        Flags: ui-review?(mconnor)

        Attachment #308371 -
        Flags: review?(mconnor)

    
  

        Attachment #308371 -
        Flags: review?(mconnor)

    
    

    
  
(In reply to comment #206)
> Created an attachment (id=308371) [details]
> Patch, v.7
> 
> This seems to fix the problem that caused v.6 to be backed out... Without the
> explicit QI, aRequest.contentType is undefined. This causes doContent() to fail
> to find a nsIMIMEInfo [or, more specifically, the call there to
> mimeSvc->GetFromTypeAndExtension() fails to find one], and it thinks it ran out
> of memory.
> 
> I'm a little unclear on what to do about the issues raised in comments
> 200-204... Land this as-is, or is more work needed?

I suspect the right thing to do is address those comments before landing, but but it may depend on how much work that will be.  biesi's suggestion in comment 204 sounds worthy of more investigation.



    
    

    
  
Comment on attachment 308371 [details] [diff] [review]
Patch, v.7

need to get the behaviour issues sorted, as discussed

        Attachment #308371 -
        Flags: ui-review?(mconnor)

        Attachment #308371 -
        Flags: review?(mconnor)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Patch v.8 (obsolete)
        — 
          — Splinter Review
      

    


  
Checkpointing... I had a couple false starts trying to pass the extra "no, really, prompt for a filename" flag around, but this works. There are a surprising number of other promptForSaveToFile implementations (minimo, camino, embedding) that needed updated for the interface change, although most looked like they were prompting for a filename anyway and can just ignore the new flag.

Todo: a couple IDL comment cleanups, and figure out if/how to do extra testing.

        Attachment #308371 -
        Attachment is obsolete: true

    
    

    
  
Comment on attachment 309307 [details] [diff] [review]
Patch v.8

Bah, I took at shot at trying to make some automated tests, but that just ended in frustration.

Much easier Litmus testcase:

Load up a Bugzilla bug with attachments. Like, say, this one.

Case 1:

1) Right-click the attachment name (left column), select Save As.
2) You should get a "Save As" dialog with the attachment filename filled in. Eg, the first attachment here has a filename of "ScreenShot.gif". It should NOT be "attachment.cgi".
3) Click Save.
4) File should download. (possible variations to test: change the filename in the dialog, or click cancel)

Case 2:
1) Go to about:config, find browser.download.saveLinkAsFilenameTimeout, and set it to "1"
2) Right click attachment, Save As
3) You should get a Save As dialog immediately, with the filename set to "attachment.cgi" (not the actual file name).
4) Click save
5) File should download

        Attachment #309307 -
        Flags: review?(mconnor)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        (updated IDL comments from v.8) (obsolete)
        — 
          — Splinter Review
      

    


  
Blocks: 393488

    
    

    
  
Comment on attachment 309352 [details] [diff] [review]
(updated IDL comments from v.8)

why not introduce a new reason and pass that on, as suggested in comment 204?

    
    

    
  
(In reply to comment #212)

> why not introduce a new reason and pass that on, as suggested in comment 204?

Mostly because I found the existing usage the nsIContentDispatchChooser::REASON_* confusing, and wasn't sure what would break (and need fixed) when code expecting one reason now started getting a new REASON_FORCESAVE.

But also because the core change was to make nsHelperAppDlg.js's promptForSaveToFile() be able to make a binary decision (force a prompt, or not)... Which smells like a boolean. And avoids having to ask questions like "but what if the reason is REASON_TYPESNIFFED, should it force a prompt or not?".

    
  
Whiteboard: [IE-parity][needs fixed patch] → [IE-parity][needs review mconnor]
Target Milestone: Firefox 3 beta5 → Firefox 3

    
    

    
  
Why not just download all the headers for any links on a page and cache them in the background.

    
    

    
  
(In reply to comment #214)
> Why not just download all the headers for any links on a page and cache them in
> the background.
> 

Wouldn't that (which would be done for all links on all pages in all tabs) be prohibitively expensive in terms of bandwidth?

    
    

    
  
(In reply to comment #215)
> (In reply to comment #214)
> > Why not just download all the headers for any links on a page and cache them in
> > the background.
> > 
> 
> Wouldn't that (which would be done for all links on all pages in all tabs) be
> prohibitively expensive in terms of bandwidth?
> 

Well, there's already prefetch which potentially can do something similar.  However, as a web developer I can tell you this would be not at all unlikely to break sites (since a lot of them perform actions on links.)  Too few people actually check for the prefetch header.

Also, a post form could easily generate a download just as much as a link.  It would be ridiculous to follow those in the background because it would be assured to break sites.

-[Unknown]


    
    

    
  
That makes since, then ie and others must be waiting for the header then before they enable the save as dialog box.

    
    

    
  
> Also, a post form could easily generate a download

That's not relevant here.  This bug is specifically about the "save link as" menuitem.  There is no problem if we've got a server response before we think we need to save, which is what would happen with a form post.

    
    

    
  
(In reply to comment #218)
> > Also, a post form could easily generate a download
> 
> That's not relevant here.  This bug is specifically about the "save link as"
> menuitem.  There is no problem if we've got a server response before we think
> we need to save, which is what would happen with a form post.
> 

Good point, sorry about my mistake.

Perhaps right clicking could prefetch a URL (since then it wouldn't be so bad as every link on the page.)  That said, currently prefetching only happens on links with rel set properly...

Still, I don't think prefetching all the links on a page for download headers is a good idea.

-[Unknown]


    
    

    
  
Comment on attachment 309307 [details] [diff] [review]
Patch v.8

mconnor wants a review from biesi or bz first...

        Attachment #309307 -
        Flags: review?(mconnor)

        Attachment #309307 -
        Flags: review?(cbiesinger)

        Attachment #309307 -
        Flags: review?(bzbarsky)

    
    

    
  
I'm not going to be able to get to this in time.
Whiteboard: [IE-parity][needs review mconnor] → [IE-parity][needs review mconnor and biesi]

    
    

    
  
Comment on attachment 309352 [details] [diff] [review]
(updated IDL comments from v.8)

this needs a new IID

    
    

    
  
Comment on attachment 309307 [details] [diff] [review]
Patch v.8

uriloader/exthandler/nsIHelperAppLauncherDialog.idl
needs a new IID

        Attachment #309307 -
        Flags: review?(cbiesinger) → review+
Whiteboard: [IE-parity][needs review mconnor and biesi] → [has patch][needs review mconnor]

    
  

        Attachment #309307 -
        Flags: review?(bzbarsky)

    
    

    
  
Comment on attachment 309307 [details] [diff] [review]
Patch v.8

Crap, I cleared the mconnor review flag last week when requesting review from bz/biesi.

Readding, since I presume mconnor still want to make a review pass on this.

        Attachment #309307 -
        Flags: review?(mconnor)

    
    

    
  
Comment on attachment 309307 [details] [diff] [review]
Patch v.8

r=mconnor, sorry for the delay here.  also double-checked the uriloader bits, so this can probably be counted as having SR from either biesi or myself

        Attachment #309307 -
        Flags: review?(mconnor) → review+
Whiteboard: [has patch][needs review mconnor] → [has patch][has reviews]

    
    

    
  


  
Checking in browser/app/profile/firefox.js;
  new revision: 1.321; previous revision: 1.320
Checking in browser/base/content/nsContextMenu.js;
  new revision: 1.53; previous revision: 1.52
Checking in browser/components/sessionstore/src/nsSessionStore.js;
  new revision: 1.99; previous revision: 1.98
Checking in embedding/browser/activex/src/control/HelperAppDlg.cpp;
  new revision: 1.15; previous revision: 1.14
Checking in embedding/browser/gtk/src/EmbedDownloadMgr.cpp;
  new revision: 1.8; previous revision: 1.7
Checking in embedding/browser/photon/src/nsUnknownContentTypeHandler.cpp;
  new revision: 1.32; previous revision: 1.31
Checking in embedding/components/ui/helperAppDlg/nsHelperAppDlg.js;
  new revision: 1.75; previous revision: 1.74
Checking in toolkit/mozapps/downloads/src/nsHelperAppDlg.js.in;
  new revision: 1.66; previous revision: 1.65
Checking in uriloader/base/nsURILoader.cpp;
  new revision: 1.149; previous revision: 1.148
Checking in uriloader/base/nsURILoader.h;
  new revision: 1.31; previous revision: 1.30
Checking in uriloader/exthandler/nsExternalHelperAppService.cpp;
  new revision: 1.369; previous revision: 1.368
Checking in uriloader/exthandler/nsExternalHelperAppService.h;
  new revision: 1.93; previous revision: 1.92
Checking in uriloader/exthandler/nsIExternalHelperAppService.idl;
  new revision: 1.28; previous revision: 1.27
Checking in uriloader/exthandler/nsIHelperAppLauncherDialog.idl;
  new revision: 1.11; previous revision: 1.10
Checking in camino/src/embedding/CHBrowserService.mm;
  new revision: 1.38; previous revision: 1.37
Checking in minimo/customization/HelperAppDlg.js;
  new revision: 1.5; previous revision: 1.4

        Attachment #309307 -
        Attachment is obsolete: true

        Attachment #309352 -
        Attachment is obsolete: true

    
  
Status: NEW → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews]

    
  
Blocks: 426742

    
    

    
  
I'm not sure this is really fixed, if I'm understanding the purpose correctly.  If you go to the website linked in Comment #119, right-click any of those song links and choose "Save Link As...", you end up with filename "kcdl".  On the other hand, if you first left-click the link and cancel the download dialog then you right-click the exact same link and choose "Save Link As...", it shows the proper filename.

It appears something is still broken.


    
    

    
  
(In reply to comment #227)

> If you go to the website linked in Comment #119, right-click any of those song
> links and choose "Save Link As...", you end up with filename "kcdl". 

The server is just slow. Go to about:config, and increase the value of browser.download.saveLinkAsFilenameTimeout. With the default of 1000ms (1s), I usually get "kcdl.php", but with 10s I get the proper filename.

    
    

    
  
dolske: was that change to nsSessionStore.js intended to go in as part of this bug?

    
    

    
  
Thank you, Justin.  But wouldn't 3000 ms be a better default for browser.download.saveLinkAsFilenameTimeout?  To me the 1000 ms limit is too easy to surpass.  3000 ms, on the other hand, is just long enough not to be annoying but allows for typical delays that can easily approach or surpass the 1000 ms.


    
    

    
  
(In reply to comment #229)
> dolske: was that change to nsSessionStore.js intended to go in as part of this
> bug?

Oops, no. I just backed that bit out, and looked through the patch again to make sure nothing else unintended snuck in.

(In reply to comment #230)
> Thank you, Justin.  But wouldn't 3000 ms be a better default for
> browser.download.saveLinkAsFilenameTimeout?

It's certainly tunable, although (1) there will always be a slower site and (2) large delays make the browser feel sluggish on slow sites. This is fodder for a new bug, though, since this one is already long-winded and the patch is working as intended.

    
    

    
  
> 3000 ms, on the other hand, is just long enough not to be annoying

The real question is whether 3000ms of no feedback would cause the user to assume they somehow failed to click the right menu item and try the operation again.  If it would, it's too long.  The way to answer this question is through a usability study.

    
    

    
  
If you right click attachments above and click save as with another browser you may notice that the first box that pop up is actually the File Download box, not the save as box, then you may notice that for a brief second the file name in that box is not the actual file name but may be the name of the link, that name changes when the actual filename is received, it is only at that time that the actual save as dialog box appears.  Further, I've noticed sometimes that the File Download box never seems to timeout until the filename is received, however the cancel button will be enabled.

    
    

    
  
Folks - these issues should be filed as new bugs.  Posting a comment here isn't going to help at all.

    
    

    
  
For what it's worth, comment 233 is exactly how I think this bug should have been fixed.  I've said that before.

    
    

    
  
Other than current Firefox nightlies, current SeaMonkey nightlies still show 'attachment.cgi' in the file picker after following the steps in comment #0. This clearly contradicts the statement in comment #151. Does SeaMonkey require significant changes in addition to the core changes applied through the patched of this bug?

    
    

    
  
Uploading grades to Blackboard 8.0 takes a process of downloading a comma-delimited (.CSV) or tab-delimited (.XLS) files. Once such file is downloaded it has to be opened, saved, and changed to upload new grades. 

Firefox 3 changes the parameters of both the file to be saved without prompting and eliminates the automatically generated file name of that file. As a result Blackboard will not recognize the file and instructor is stuck with ... downgrading to Firefox 2, where it works normally.

Students also have the same problem when they submit work using Microsoft Word or Excel file. Blackboard interface accessed with Firefox 3 indicates that the file format is incompatible and action fails.

    
    

    
  
petersb: You should open a new bug for whatever your issue is. The bug fix here just allows Firefox to save a file with a name explicitly requested by the server. It doesn't change the format or contents of the file.

    
    

    
  
(In reply to comment #239)
> petersb: You should open a new bug for whatever your issue is. The bug fix here
> just allows Firefox to save a file with a name explicitly requested by the
> server. It doesn't change the format or contents of the file.


It actually does change something. The server does not recognize the file when I upload it back without even peaking into it. Default is for Unicode text (.txt), I select an .xls workbook as usual and it is not accepted back. To makes things worse the other browser is locked up the same way unless I switch to the earlier version of FFox (2.14). File is changed before I get any options and is should not be. :(

New ticket created anyway, I thought you may find it useful/interesting. Thanks!

    
    

    
  
Bug is back in 3.1 beta3.
Don't know about previous betas.
Flags: blocking-firefox3.6?

    
    

    
  
This bug is fixed, as you can see from its status.  Whatever you're seeing should be filed as a separate bug, with clear steps to reproduce.  Please feel free to cc me on that separate bug.
Flags: blocking-firefox3.6?

    
  
Blocks: 682478

    
  
Depends on: 732406

    
  
Depends on: 608568

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: