Last Comment Bug 315227 - Failed download when using "Save Link As..." on a link to an page / file that uses HTTP/FTP authentication.
: Failed download when using "Save Link As..." on a link to an page / file that...
Status: VERIFIED FIXED
: regression
Product: Toolkit
Classification: Components
Component: Download Manager (show other bugs)
: Trunk
: All All
: -- normal with 3 votes (vote)
: mozilla1.9alpha7
Assigned To: Christian :Biesinger (don't email me, ping me on IRC)
:
:
Mentors:
http://www.andy-online.de/bug315227/i...
: 313730 339733 352810 355861 388308 (view as bug list)
Depends on:
Blocks: 146574 313730
  Show dependency treegraph
 
Reported: 2005-11-05 12:45 PST by N/A
Modified: 2008-07-31 04:30 PDT (History)
16 users (show)
mbeltzner: blocking1.9+
mbeltzner: blocking‑firefox2-
stephen.donner: in‑testsuite?
stephen.donner: in‑litmus+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (firefox) (3.20 KB, patch)
2007-05-27 05:01 PDT, Christian :Biesinger (don't email me, ping me on IRC)
no flags Details | Diff | Splinter Review
patch (seamonkey) (6.34 KB, patch)
2007-05-27 05:09 PDT, Christian :Biesinger (don't email me, ping me on IRC)
neil: review-
Details | Diff | Splinter Review
patch v1.1 (9.63 KB, patch)
2007-06-18 15:09 PDT, Christian :Biesinger (don't email me, ping me on IRC)
neil: review+
gavin.sharp: review+
Details | Diff | Splinter Review
final patch (10.00 KB, patch)
2007-07-15 17:00 PDT, Christian :Biesinger (don't email me, ping me on IRC)
no flags Details | Diff | Splinter Review

Description N/A 2005-11-05 12:45:39 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051025 Firefox/1.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8) Gecko/20051025 Firefox/1.5

When using "Save Link As..." on a link to a page or file (PDF, etc.) that uses HTTP authentication (not yet authenticated) simply brings up the "Save As" dialog instead of the "Enter User Name and Password" dialog. This results in failed download (saves 401 error page).

Works as expected in 1.0x (prompts user for Authentication, then "Save As").

Reproducible: Always

Steps to Reproduce:
1.Find a link to a HTTP authenticated page.
2.Right Click on link -> "Save Link As..."


Actual Results:  
"Save As" dialog is displayed and an 401 error page is saved no matter the file type.

Expected Results:  
"Enter User Name and Password" dialog is displayed for authentication before the "Save As" dialog.

Possibly related to: 308715 (404 page is saved).

Works as expected in 1.0x (prompts user for Authentication, then "Save As").
Comment 1 Mike Connor [:mconnor] 2006-02-25 12:23:03 PST
Jesse, can you provide a testcase and confirmation before asking for blocking? Thanks!
Comment 2 mails4andy 2006-02-25 13:32:55 PST
I can confirm this with
Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060224 Firefox/1.6a1

A testcase cannot be applied as an attachment, so I have created an online testcase under http://www.andy-online.de/bug315227/index.html

I hope that helps.
Comment 3 maenneph 2006-05-21 12:33:54 PDT
Confirming this with nightly build.

(Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20060521 Minefield/3.0a1)
Comment 4 Jackie Liu 2006-09-25 23:16:37 PDT
I can also confirm with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20060925 BonEcho/2.0.  Showstopper for Firefox 2?
Comment 5 Mike Beltzner [:beltzner, not reading bugmail] 2006-09-26 10:18:10 PDT
This isn't a 1.5->2.0 regression, and while we're sorry it didn't get fixed, it's too late to deal with it now and not serious enough to block.

Putting it on the radar for Firefox 3, though.
Comment 6 Jesse Ruderman 2006-10-07 17:10:41 PDT
*** Bug 352810 has been marked as a duplicate of this bug. ***
Comment 7 Jesse Ruderman 2006-10-07 17:11:00 PDT
*** Bug 355861 has been marked as a duplicate of this bug. ***
Comment 8 OstGote! 2007-01-13 09:35:13 PST
Same thing for SeaMonkey. Is there any reason why this is declared as FF only? I did not find any Suite or Core bug for this problem. IMHO this bug should be Core/File Handling (or Network...). 
Comment 9 Christian :Biesinger (don't email me, ping me on IRC) 2007-05-26 12:36:50 PDT
note that ff and seamonkey have different code here, though I'm going to fix it for both
Comment 10 Christian :Biesinger (don't email me, ping me on IRC) 2007-05-27 05:01:28 PDT
Created attachment 266249 [details] [diff] [review]
patch (firefox)

I don't think there's a good way to unit test this, unfortunately...
Comment 11 Christian :Biesinger (don't email me, ping me on IRC) 2007-05-27 05:09:02 PDT
Created attachment 266250 [details] [diff] [review]
patch (seamonkey)
Comment 12 neil@parkwaycc.co.uk 2007-05-27 09:38:57 PDT
Comment on attachment 266250 [details] [diff] [review]
patch (seamonkey)

r- because cancelling the prompt doesn't cancel the download. Instead it gets stuck in a limbo state, and you have to cancel it manually.

Note that the transfer tries to open the download manager / progress window while the persist is trying to prompt, which is suboptimal.

>+  this.window = win;
You don't use this at all, because you always pass the global window.

>+  this.transfer = transfer;
You don't use this either, except via makeClosure.

>+  // Now... we need to forward all calls to our transfer
If only __noSuchMethod__ worked... but it doesn't :-(

>+  QueryInterface: function dl_qi(iid)
Nit: aIID
Comment 13 neil@parkwaycc.co.uk 2007-05-27 13:48:53 PDT
(In reply to comment #12)
>r- because cancelling the prompt doesn't cancel the download.
Looks like an issue with the tree I tested it with :-\ Two other trees are ok.
Comment 14 Christian :Biesinger (don't email me, ping me on IRC) 2007-06-18 15:09:37 PDT
Created attachment 268856 [details] [diff] [review]
patch v1.1

nits fixed, except for the this.window one. I prefer passing this in from the caller.
Comment 15 neil@parkwaycc.co.uk 2007-06-19 06:43:01 PDT
Comment on attachment 268856 [details] [diff] [review]
patch v1.1

I guess this is OK although I had a look into the source and it looks as if it would be much neater to add this to nsDownloadProxy instead.
Comment 16 Christian :Biesinger (don't email me, ping me on IRC) 2007-06-19 11:57:41 PDT
nsDownloadProxy doesn't have access to the nsIDOMWindow
Comment 17 neil@parkwaycc.co.uk 2007-06-20 03:43:48 PDT
(In reply to comment #16)
>nsDownloadProxy doesn't have access to the nsIDOMWindow
I was expecting you to pass it in from the caller :-P
Comment 18 Christian :Biesinger (don't email me, ping me on IRC) 2007-07-05 06:55:26 PDT
*** Bug 313730 has been marked as a duplicate of this bug. ***
Comment 19 :Gavin Sharp [email: gavin@gavinsharp.com] 2007-07-06 17:07:17 PDT
Comment on attachment 268856 [details] [diff] [review]
patch v1.1

>+  getInterface: function dl_gi(aIID)
>+  {
>+    if (aIID.equals(Components.interfaces.nsIAuthPrompt)) {
>+      return ww.getNewAuthPrompter(this.window);

>+    if (aIID.equals(Components.interfaces.nsIAuthPrompt2)) {
>+      return ww.getPrompt(this.window, aIID);

Is there a reason you didn't just use getPrompt for both of these, and factor out the common code? getPrompt(window, nsIAuthPrompt) seems to be equivalent to getNewAuthPrompter(window).

Shouldn't we fail the download on a 403, rather than save the error (e.g. if you cancel the password prompt)?
Comment 20 Christian :Biesinger (don't email me, ping me on IRC) 2007-07-06 17:16:28 PDT
sure, I can use getPrompt for both

Yes, we probably should fail. But we should also fail 404 pages, etc. Do you think I should do that as part of this patch?
Comment 21 :Gavin Sharp [email: gavin@gavinsharp.com] 2007-07-06 17:29:59 PDT
Comment on attachment 268856 [details] [diff] [review]
patch v1.1

(In reply to comment #20)
> Yes, we probably should fail. But we should also fail 404 pages, etc. Do you
> think I should do that as part of this patch?

No, I didn't realize we didn't fail for other HTTP errors.
Comment 22 Christian :Biesinger (don't email me, ping me on IRC) 2007-07-15 17:00:16 PDT
Created attachment 272431 [details] [diff] [review]
final patch
Comment 23 Christian :Biesinger (don't email me, ping me on IRC) 2007-07-15 17:02:26 PDT
Checking in suite/common/contentAreaUtils.js;
/cvsroot/mozilla/suite/common/contentAreaUtils.js,v  <--  contentAreaUtils.js
new revision: 1.144; previous revision: 1.143
done
Checking in toolkit/content/contentAreaUtils.js;
/cvsroot/mozilla/toolkit/content/contentAreaUtils.js,v  <--  contentAreaUtils.js
new revision: 1.90; previous revision: 1.89
done
Checking in xpfe/communicator/resources/content/contentAreaUtils.js;
/cvsroot/mozilla/xpfe/communicator/resources/content/contentAreaUtils.js,v  <--  contentAreaUtils.js
new revision: 1.141; previous revision: 1.140
done
Comment 24 Dave Townsend [:mossop] 2007-07-16 08:33:57 PDT
*** Bug 388308 has been marked as a duplicate of this bug. ***
Comment 25 Stephen Donner [:stephend] 2007-08-19 00:21:05 PDT
http://litmus.mozilla.org/manage_testcases.cgi

in-litmus+
Comment 26 Christian :Biesinger (don't email me, ping me on IRC) 2007-08-19 18:06:21 PDT
*** Bug 339733 has been marked as a duplicate of this bug. ***
Comment 27 Stephen Donner [:stephend] 2007-10-28 01:01:28 PDT
Verified FIXED using:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007102704 Minefield/3.0a9pre

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007102705 Minefield/3.0a9pre

and

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a9pre) Gecko/2007102704 Minefield/3.0a9pre 

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