User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10 ( .NET CLR 3.5.30729) Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de-DE; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10 ( .NET CLR 3.5.30729) When trying to download a file via a https:// from a password protected directory, Firefox asks for the user name and password. After entering the user credentials, nothing more happens - also copying the link into the browsers address bar does not work. When putting the link in the address bar after *restarting* the browser, the download is working as expected. I already checked with CharlesProxy and it seems as if Firefox does not send any credentials (or anything else) at all to the web server after entering username/password. The supplied URL demonstrates the problem. Reproducible: Always Steps to Reproduce: 1. go to the URL 2. right-click on the link -> "save file as..." 3. enter the following username and password: mozilla / test Actual Results: nothing happens Expected Results: File should be downloaded I already talked to moo / Thomas B. Ruecker (dm8tbr@afthd.tu-darmstadt.de) on the #firefox.de IRC channel and he could reproduce the problem as well.

For completeness: here's my build ID Mozilla/5.0 (X11; U; Linux i686; de; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10

using a newly created user profile does not help either

The problem is not limited to HTTPS-URLs, but also applies to non-SSL connections, Testcase URL is http://www.ellis-events.com/download.htm, Username and password is the same.

I now checked some older Firefox versions plus the current 4.0 Beta 6: Firefox 3.0 Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.0.14) Gecko/2009082707 Firefox/3.0.14 and Firefox 3.5.7 Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.1.7) Gecko/20091221 Firefox/3.5.7 behave both as expected. The current 4.0 beta: Mozilla/5.0 (Windows NT 5.1; rv:2.0b6) Gecko/20100101 Firefox/4.0b6 shows the same problem as 3.6.10

I think this is enough info for the bug to be reproducible. Tested latest trunk.

As this is obviously a regression, we could also need a regressionwindow.

Regression window: Works: http://hg.mozilla.org/mozilla-central/rev/247524e19d0c Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090720 Minefield/3.6a1pre Fails: http://hg.mozilla.org/mozilla-central/rev/f2a58ffcd00c Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090721 Minefield/3.6a1pre Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=247524e19d0c&tochange=f2a58ffcd00c

In local build, The following changeset causes the probrem. d6895cb5ac3b Honza Bambas — Bug 475053 - Implement asyncPromptAuth to fix multiple password prompt overlap

If I save the user credentials for the downloaded resource, I get the following, slightly different behaviour: - the first attempt to "save as..." shows the user/password dialog with the username and password already filled in, but the download fails. - A second attempt (*without* closing the browser) succeeds, the file gets downloaded then.

Did this work in Firefox 3.6, or 3.5? I.e. is it only a problem in Firefox 4 betas or is this an older problem...?

This problem happens on 3.6 and 4.0b7pre. 3.5 works fine. And See comment #4

And comment 7. I suspect the sync prompt thing is a good regression candidate here.... and that we do want to fix this on 1.9.2 if we can.

Am I right to assume that a fix probably will not make it into 3.6.11 as it is only 4 days to go until code freeze and only 3 blocking issues left?

Unless some sort of magic happens, that's probably true....

Not going to "block" a branch release (we've managed to live with it for 10 updates), but we'll look at the patch when it's done for trunk. It probably should block Firefox 4.

Honza, please see comment 8. Blocking.

Looking into this.

nsContextMenu (in its js file) in saveAs command assigns its own listener to the channel. Before nsContextMenu asyncOpens the channel it starts a timer that fires in one second and cancels the channel with NS_ERROR_SAVE_LINK_AS_TIMEOUT. The timer is canceled in the listener's OnStartRequest. Workaround: set browser.download.saveLinkAsFilenameTimeout to 60000. It will make the code wait for one minute instead of just a single second. Going to investigate why this worked before the patch for bug 475053.

Here's why: when the timer cancels the channel, nsHttpChannel::Cancel cancels wait for the auth prompt and gets OnAuthCancelled with userCancel=false. In case of userCancel=false it doesn't cancel+respin the transaction and therefor it hangs forever suspended. And, it keeps the cache entry and therefor the download is no more possible until firefox restart. nsHttpChannel needs to be patched. However, there is also a different bug. In bug 299372 we introduced a code that waits a second for the server response to get the filename from content-disposition headers, if this times out, we do it an older way, w/o waiting for the header to do not make user wait. This new code, however, doesn't count with a pop-up of the authentication dialog. Nor it counts with many other things like url classification, async cache lookup, all that may delay OnStartRequest. Starting the timer before AsyncOpen is wrong IMO. I have to think of some API we already have to use it to start the timer.

Patching nsHttpChannel doesn't help to fix this. We get both the auth prompt and save dialog (from the legacy save uri system) on the screen.

Fix for both problems. Not sure of the reviewer for the nsContextMenu.js changes.

Comment on attachment 478561 [details] [diff] [review] v1 the magic wheel of "not actually me" lands on.... dolske!

Comment on attachment 478561 [details] [diff] [review] v1 >+++ b/browser/base/content/nsContextMenu.js ... > saveLink: function() { > // canonical def in nsURILoader.h > const NS_ERROR_SAVE_LINK_AS_TIMEOUT = 0x805d0020; >+ // defined in nsISocketTransport.idl >+ const NS_NET_STATUS_WAITING_FOR = 0x804b000a; The previous line has the excuse that the value is in a .h (and, sadly, not in Components.results), but for the one you're adding you should just use Ci.nsISocketTransport.STATUS_WAITING_FOR. >+ onStatus: function sLA_callbacks_onStatus(aRequest, aContext, aStatus, aHost) { >+ if (aStatus != NS_NET_STATUS_WAITING_FOR) >+ return; I don't think this is right (2nd half of comment 19, too). The problem bug 299372 was fixing (well, improving what had earlier been fixed) was to not have clicking "Save As" do nothing for a long period of time before showing UI. EG, consider being on a terrible connection where it takes a minute to resolve the host and successfully connect. AIUI, we won't get NS_NET_STATUS_WAITING_FOR until after that point, so this reverts us back to the original bug. Thus, I think we really need to start the timer immediately. It's a little sucky to have the auth prompt come up while we're showing the Save As dialog, but I don't see an easy way around that. Followup fodder for finding a way to suppress the auth prompt until after the Save As has been responded to? I suppose we could try suspending the channel while the dialog is up (if the timeout fired)? I'm a bit wary of making non-authenticated downloads slower (because we wouldn't be slurping down data while waiting for the user), but maybe that's ok since the site is already slow to respond to the request... I think we need a test here too. Should be testable via a .sjs that deliberately does not respond to a request?

OK, if we do not authenticate to the site, we don't have a chance to get the proper header anyway. So, I suggest to cancel the channel and fall back to the old way when the channel asks for an auth prompt. We then can run the timer before we open the channel (what I really don't like, but understand why we need it).

Honza, what's the nsHttpChannel change (which I assume is what you want me to review) actually doing?

(In reply to comment #25) > Honza, what's the nsHttpChannel change (which I assume is what you want me to > review) actually doing? The 'userCancel' argument is true when user presses the cancel button in the auth dialog (refuse to provide credentials). It is false when the queued authentication task has been canceled by something else then user, e.g. cancellation of the channel, as in our case. However, while we wait for the auth prompt result (result here is also user or internal cancellation of the dialog) the transaction is always suspended. So, we always have to resume it after we get the result, whatever it is. We was not doing that for internal cancellation, that was making the channel hang forever, also with the cache entry. I changed then the condition to just check the transaction is non-null. We are apparently missing a test for this.

- the channel is canceled after it demands an auth prompt - other remains unchanged To preserve the file name determination even we must prompt for credentials would be to cancel the timer when the channel wants an auth prompt, provide an auth prompt, and start the timer again after a notification comes to the progress listener, whatever notification it is - the only way to figure out the credentials were provided. This is relatively complex, but if we really want it, I can have a patch and a good test for it. I'll add a test anyway and update the patch with a test ones again.

Comment on attachment 481178 [details] [diff] [review] v2 [Check in comment 30] r=me for the nsContextMenu.js change.

Comment on attachment 481178 [details] [diff] [review] v2 [Check in comment 30] sr=me

Comment on attachment 481178 [details] [diff] [review] v2 [Check in comment 30] http://hg.mozilla.org/mozilla-central/rev/d9fdddb0a30e

Now that this is fixed (thanks to everybody involved) - what are the chances that this fix will find its way into 3.6.12 and not only 4.x?

Comment on attachment 481178 [details] [diff] [review] v2 [Check in comment 30] >diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js >@@ -953,24 +953,27 @@ nsContextMenu.prototype = { > function callbacks() {} > callbacks.prototype = { > getInterface: function sLA_callbacks_getInterface(aIID) { > if (aIID.equals(Ci.nsIAuthPrompt) || aIID.equals(Ci.nsIAuthPrompt2)) { >- var ww = Cc["@mozilla.org/embedcomp/window-watcher;1"]. >- getService(Ci.nsIPromptFactory); >- return ww.getPrompt(doc.defaultView, aIID); >+ // If the channel demands authentication prompt, we must cancel it >+ // because the save-as-timer would expire and cancel the channel >+ // before we get credentials from user. Both authentication dialog >+ // and save as dialog would appear on the screen as we fall back to >+ // the old fashioned way after the timeout. >+ timer.cancel(); >+ channel.cancel(NS_ERROR_SAVE_LINK_AS_TIMEOUT); > } > throw Cr.NS_ERROR_NO_INTERFACE; > } > } Is it actually intended that this code/case now throws instead of returning (something)? If so, I'd say it lacks an explicit comment.

(In reply to comment #32) > Is it actually intended that this code/case now throws instead of returning > (something)? It's actually the fix for this bug. > If so, I'd say it lacks an explicit comment. And I'd say the comment is enough to explain what we are doing here, isn't it?

(In reply to comment #33) > It's actually the fix for this bug. Ah, unusual but if it is then fine. Thanks for the confirmation. > And I'd say the comment is enough to explain what we are doing here, isn't it? Well, I know little about this code and the comment seemed related to the 2 .cancel(), not to the s/return/throw/. I would have expected at least something like a '// fall through' to be (even more) explicit. I guess NS_ERROR_NO_INTERFACE after succeeding .equals() confuses me by default :-|