Review Request: Move the KPasswordDialog "Anonymous" checkbox below the input boxes...

Fri Mar 25 05:26:43 GMT 2011

> On March 22, 2011, 6:23 p.m., John Layt wrote:
> > My thoughts are if the anonymous tickbox is first focus it should be above the username/password fields and the username/password fields should be disabled if anonymous is selected.  But that may look a little weird with the text to the left and the title below it.
> > 
> > Or does entering a username/password automatically de-select the anonymous?  In which case this is fine so long as the username field has the cursor focus and pressing enter immediately activates the ok button.  
> > 
> > Or perhaps pre-populate username with 'anonymous' and have the word pre-selected so it can be overtyped and do away with the tickbox entirely?
> > 
> > Basically what I'm looking for is the easiest shortest most obvious path for the main use cases.
> > 
> > </bikeshedding>
> 
> Thomas Lübking wrote:
>     > the anonymous tickbox is first focus it should be above the username/password
>     +1
>     
>     > username/password fields should be disabled if anonymous is selected
>     the problem is about checking the box will disable the UI.
>     
>     can we maybe drop the checkbox entirely and just QLinedit::setPlaceholderText("anonymous")  and QLinedit::setPlaceholderText("no password required for anonymous login") or sth. like this?
>     
>     The tab order can btw be controlled explicitly and unrelated to the actual layout.
> 
> Dawit Alemayehu wrote:
>     > Or does entering a username/password automatically de-select the anonymous?  In which case this is fine so long as the username field has the cursor focus and pressing enter
>     > immediately activates the ok button.
>     
>     Nope. You have to manually deselect the checkbox to enter the username/password. Otherwise, those input widgets will be set to read only.
>     
>     > Basically what I'm looking for is the easiest shortest most obvious path for the main use cases.
>     
>     Unfortunately, that is the problem. This feature does not solve any main use case. Rather it seems to have been added to deal with a very corner case in specific ioslaves, ftp being the main or only?? one.
>     
>     > can we maybe drop the checkbox entirely and just QLinedit::setPlaceholderText("anonymous")  and QLinedit::setPlaceholderText("no password required for anonymous
>     > login") or sth. like this?
>     
>     Well, it seems that the anonymous checkbox was specifically added to distinguish between the default "anonymous" login and a user entered "anonymous" user name for the ftp ioslave. At least that is what I get reading the commit message in the ftp ioslave that seems to be related to this checkbox [1]. As such, I fail to see how the placeholder text approach would satisfy what the checkbox is attempting to achieve. However, I personally do not think that checkbox should exist at all. It is there to handle a very corner case for very specific ioslaves (mainly ftp ioslave). Even then it seems to be used incorrectly right now. Anyhow, I like the placeholder idea but I will serve the same purpose as the checkbox.
>     
>     > The tab order can btw be controlled explicitly and unrelated to the actual layout.
>     
>     Right, but the checkbox will still look out of place where it is right now.
>     
>     
>     
>     
>     [1] http://quickgit.kde.org/?p=kdelibs.git&a=commit&h=2e48eecc2fda83f08fb5f55519fe51793f536734
> 
> John Layt wrote:
>     Looking at the api and code, the Anonymous tickbox is only displayed if the ShowAnonymousLoginCheckBox flag is set, and the default value of the tickbox is set by calling setAnonymousMode(), so it is a corner case that is enabled by the client.  Most of the time the tickbox will not be displayed.  When it is displayed I think it needs to be more prominent as you are effectively choosing a mode, so if you keep the tickbox it does need to be _above_ the username field.  If the anonymous mode is enabled, then the tickbox needs first focus, if it is not enabled and no username is preset then the username needs first focus, if the username is set or if it is protected then the password needs first focus (most of this already happens).
>     
>     I like the placeholder idea, but I don't think it can be made to work with the existing api, e.g. if both setAnonymousMode(true) and setUsername("Bob") are called by the client then it won't work.
>     
>     So we just need to figure out a non-ugly way for the tickbox to be above the Username.  The ugliest part at the moment is having the tickbox in the label column which is not HIG compliant, it needs to be in the same column as the text entry boxes.  To look even neater I'd be tempted to put a text label to the left of the tickbox, either "Login:", or "Username:" and hide the label on the actual Username box.  Have a play :-)
> 
> Aurélien Gâteau wrote:
>     Maybe using radio buttons instead of a checkbox would help? I am thinking about something like this:
>     
>     ( ) Anonymous Login
>     (*) Authenticated Login
>     
>          Login: [_______________]
>       Password: [_______________]
> 
> Thomas Lübking wrote:
>     Since we cannot avoid the checkbox, what about attaching it to the "Username" label.
>     The anon-enabled login would then only differ by the preset, empty line handling and this checkbox.
>     
>     ui dummy (lacks a little smartness regarding the pw placeholder and the login content)
>     http://pastebin.com/NikPB1PJ

Hmm... I think we are now over thinking this and trying to be too smart about it when it really should not be the case. The placement of the "Login anonymously" should be equivalent to where the "Remember password" checkbox is for a number of very valid reasons:

#1. Both of them are options, not requirements, and their effects apply equally to both inputs, i.e. the username and the password. "Remember password" is not limited to just remembering the password.

#2. In the normal use case, both are final acts the user would likely perform before dismissing the dialog box. That is click on the checkboxes and press Ok or Cancel.

Thomas, I can tell you that your approach is a non-starter because the login in anonymously right now is a corner case for a single ioslave (ftp) right now and the password dialog is more generic. It is used by many applications for different purposes. Hence, optimizing for this minor use case does not make sense. 

Perhaps if I explain under what circumstance this checkbox will be shown in the immediate future, it will clarify the use case for all. The only time this checkbox will be visible to the end user is when {s}he types a username as part of the url, i.e. ftp://<user>@<host>. Otherwise, the anonymous login will have been automatically be tried by the ftp ioslave before the user is prompted for credentials. In which case of course it simply means that the anonymous login was rejected by the server. As such, there is absolutely no point in giving the user the option to retry the same thing that failed just now. This means the "login anonymously" checkbox will only be available to the user under very limited circumstances ; no to mention only one ioslave at the moment. This is why I argued it should not have been implemented in the first place.  However, whomever implemented it, saw the need/use-case for it.

Anyhow, I just want someone to explain to me why where I placed the checkbox in the first place is inappropriate based on what I just described above. Anonymous login was never intended to have a prominent role as seems to be assumed by most of you. IMO, it should by no means interfere with the normal workings of the password dialog. That would be true if there were many more ioslaves requiring this functionality. It is even more so when only one needs and uses it right now.

- Dawit

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/100920/#review2106
-----------------------------------------------------------

On March 22, 2011, 6:04 p.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100920/
> -----------------------------------------------------------
> 
> (Updated March 22, 2011, 6:04 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> The attached patch renames and moves the "Anonymous" box shown in the first screenshot below the password input widgets. This change is done to 
> 
> #1. Remove the checkbox from screwing tab order when you enter username/password.
> #2. Make the purpose of the checkbox less cryptic to the end user.
> 
> Any objections ?
> 
> 
> Diffs
> -----
> 
>   kdeui/dialogs/kpassworddialog.ui 2649870 
> 
> Diff: http://git.reviewboard.kde.org/r/100920/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> Current Password Dialog
>   http://git.reviewboard.kde.org/r/100920/s/106/
> New Password Dialog
>   http://git.reviewboard.kde.org/r/100920/s/107/
> 
> 
> Thanks,
> 
> Dawit
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110325/72d6eef6/attachment.htm>