This is a follow-on bug to bug 113515. The problem is that rjc added code to check for a 530 trying to fix bug 44325. The problem is that an FTP server will return a 530 for _any_ login failure.

Note also that even _if_ we remove the 530 checking, there is still a problem with the FTP protocol (or perhaps with wallet) where the connection just times out. Anyway, feel free to mark this bug as INVALID. If you decide to do that, then please remove the password manager code from nsFTPConnectionThread.cpp.

In Chris' first comment, make that bug # 44324 instead of bug # 44325, BTW. :)

I'm not sure what the correct behaviour is from a UI point of view. The current behvaiour is obviously incorrect, though. Given that we cannot differentiate between login failure types, I think the best thing to do is to display the error, and then somehow tell the password manager to reprompt if we try again. It shouldn't lose the stored password, but it shouldn't return it automatically, either. Is this possible? I will not get to this before I return at the end of Jan. Feel free to take this bug from me.

Lets try for 0.9.9

Just a note: In Composer's new publishing code, we remove the password from the database if the user unchecks the "Use password manager to save password" checkbox. Of course that's only if our nsIWebProgressListener callbacks are called. So this bug won't affect the publishing feature.

Charlie, what about if the user enters an incorrect password when publishing? Do you detect that and remove the password from the database then too? It's the responsibility of each application (http auth, ftp auth, publishing, etc) to explicitly call on password manager to remove incorrect paswords. Password manager cannot detect if the password was invalid.

OK, try this. The problem was that the string we were asking for was different to the string we were giving to delete the stored password. This patch causes us to always use the PrePath. However, the observer callback takes a url and then gets its spec, which has a trailing /, so we need to add that to the initial prePath, which is passed in as a string (Why doesn't the observer callback take a string, btw?) Our behaviour for when login is denied is now the same as ns4, see bug 124561 for changing that.

Stephen: Composer publishing totaly manages the username and password -- we store the username with other publishing data, and call to the PasswordManager for that. If we get the appropriate nsIWebProgressListener prompt callbacks, we will take care of fixing any username and password errors. Unfortunately, bugs like bug 124561 stand in the way of that working right now :(

what about using nsIPasswordManager instead of this observer notification stuff? that's what HTTP does. nsIPasswordManager.idl is part of necko.

So we now have two ways to cancel this then... Bleh.

Comment on attachment 70662 [details] [diff] [review] v2 sr=darin

Comment on attachment 70662 [details] [diff] [review] v2 looks okay.

a=asa (on behalf of drivers) for checkin to 0.9.9

Checked in.

Mozilla, 1.1 Mac OS X VERIFIED: someone tell me if I got this wrong... STEPS: 1- go to FTP URL w/ user w/o password. Enter incorrect password, check password manager. NOTE: failure to login. 2- go to same URL again. NOTE: prompt re-appears. 3- enter correct password, check password manager. 4- Quit browser, re-access URL, login is automatic. If composer is depending on this, please have the composer QA file a separate bug, mark this as depends, then verify it there. If I did this wrong, please reopen.

checked remaining plats.