PATCH: Fix most of the login issues with the FTP ioslave...
Review Request #101173 - Created April 22, 2011 and submitted
|99686, 124675, 143488, 258888|
The attached patch addresses most of the FTP login related problems and is a replacement for the previous review request https://git.reviewboard.kde.org/r/100873/. Here are all the changes in this patch: - Show the "Remember password" checkbox even after the failure of the first login attempt. [Bug:258888] - Always check for cached password before trying to login anonymously unless the "TryAnonymousLoginFirst" flag was set in kio_ftprc. [Bug: 99686, 143488, 124675] - Avoid sending the "anonymous" username so it will not be used in the key used to store the password in kwallet. - When a url contains a username, but the user chooses to login with a different username in the password dialog, then use redirection to update the client of the change. - Store password information in persistent storage if and only if the user checked the "Remember password" checkbox.
- Attempt to login with incorrect username and validate the "Remember password" is actually shown again. - Corrected the username information from the password dialog to ensure the client is updated properly about the password change. - Clicked on the "Remember password" to store password in persistent storage and retry logging into the same server at a later point.
Oh, I thought we could remove the boolean altogether. I don't really understand the way the patch works. The boolean is set in ftpOpenControlConnection and read in ftpOpenConnection? What's the relatoin between these two methods? Can't they communicate without using a more long term (= more possible side effects) boolean? Even a bool& as parameter would seem better (even if it reads ugly), because the decision point and the usage point would be clear. I could be wrong because I didn't look into details, but wouldn't it work to do this all in ftpOpenControlConnection maybe? redirect, and return false?
Sorry, I still have a question. After a username change, setHost is going to be called, right? So it will disconnect kio_ftp from the host? In that case, what's the point in doing the redirection code in this method - I thought the point was to do it after connected(), but that can't be it. So it still seems to me that the redirection code could be done inside of ftpLogin, which would simplify the code quite a lot (no bool *, in particular). But I guess I'm missing something; maybe setHost isn't called, after a redirection, so the app simply reuses the already-connected slave? In that case I don't see a better solution, indeed.