> On April 27, 2011, 7:58 p.m., David Faure wrote: > > kioslave/ftp/ftp.cpp, line 605 > > <http://git.reviewboard.kde.org/r/101173/diff/3/?file=15471#file15471line605> > > > > 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?
Ahhh.... m_bUserNameChanged is never set in ftpOpenControlConnection. It does get set to false in ftpCloseControlConnection. Did you mean that ? If so, that is done to reset the flag just like say m_bLoggedOn. More importantly the flag is only used between ftpLogin and ftpOpenConnection ; so it definitely can be changed into a parameter that ftpLogin sets. Come to think of it I did say I was going to remove that flag on irc yesterday didn't I ? Oh well... let me do that then... - Dawit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101173/#review2926 ----------------------------------------------------------- On April 27, 2011, 7:28 a.m., Dawit Alemayehu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101173/ > ----------------------------------------------------------- > > (Updated April 27, 2011, 7:28 a.m.) > > > Review request for kdelibs and David Faure. > > > Summary > ------- > > 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. > > > This addresses bugs 99686, 124675, 143488, and 258888. > http://bugs.kde.org/show_bug.cgi?id=99686 > http://bugs.kde.org/show_bug.cgi?id=124675 > http://bugs.kde.org/show_bug.cgi?id=143488 > http://bugs.kde.org/show_bug.cgi?id=258888 > > > Diffs > ----- > > kioslave/ftp/ftp.h 4ccdd4c > kioslave/ftp/ftp.cpp f7db42b > > Diff: http://git.reviewboard.kde.org/r/101173/diff > > > Testing > ------- > > - 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. > > > Thanks, > > Dawit > >
