<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="http://git.reviewboard.kde.org/r/100920/">http://git.reviewboard.kde.org/r/100920/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On March 22nd, 2011, 6:23 p.m., <b>John Layt</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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></pre>
 </blockquote>




 <p>On March 22nd, 2011, 6:56 p.m., <b>Thomas Lübking</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> 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.</pre>
 </blockquote>





 <p>On March 23rd, 2011, 4:20 a.m., <b>Dawit Alemayehu</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">> 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</pre>
 </blockquote>





 <p>On March 23rd, 2011, 10:11 a.m., <b>John Layt</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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 :-)</pre>
 </blockquote>





 <p>On March 24th, 2011, 1 p.m., <b>Aurélien Gâteau</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Maybe using radio buttons instead of a checkbox would help? I am thinking about something like this:

( ) Anonymous Login
(*) Authenticated Login

     Login: [_______________]
  Password: [_______________]</pre>
 </blockquote>





 <p>On March 25th, 2011, 1:06 a.m., <b>Thomas Lübking</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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</pre>
 </blockquote>





 <p>On March 25th, 2011, 5:26 a.m., <b>Dawit Alemayehu</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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.</pre>
 </blockquote>





 <p>On March 25th, 2011, 12:53 p.m., <b>Thomas Lübking</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">From a GUI design pov the box is wrong because it adds a horizontal structure to an otherwise pure vertical one (no, the buttonbar doesn't count ;-)

However,
> Perhaps if I explain under what circumstance this checkbox will be shown
On this background the entire thing is -as you suspected- flawed.

a) Placeholders don't work since there's content (attempted login) or (iff, in doubt) anon is for sure wrong.
b) The checkbox does NOT belong next to the "store PW" box, since that is a persistent setting while the former one is (hopefully, the option is likely wrong ;-) a temporary option - also it's mutually exclusive to all other UI elements.
c) The dialog should stress that the attempted login has /failed/ and therefore one has to 
   c1) provide better login data and click ok (which would ideally read "login", problem solution #1) or
   c2) suggest "Try anon login" in a pushbutton (problem solution #2) - NOT in a checkbox. It's no longer an option but a completely other action to resolve the initial problem. (from a user POV - that it uses the same codepath with different parameters doesn't matter)

Updated dummy (notice that the buttonbar is no real buttonbar, since i don#t know how to change the text w/ designer only ;-)
http://pastebin.com/qvisH1Vv</pre>
 </blockquote>





 <p>On March 25th, 2011, 5:57 p.m., <b>John Layt</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">I think you're missing that KPasswordDialog is supposed to be a generic but configurable dialog for use by any application to obtain a users credentials for whatever purposes they need, not just for use by kio slaves logging into ftp servers.  The anonymous feature was added in 2008 long before the patch you point to tweaking its behaviour.  You can't design the dialog based solely on the single special case you know about in kio-ftp where it is displayed in response to a failure to login as anonymous.  You have to think about what happens when an application deliberately wants to ask the user to choose between anonymous and username login to ANYTHING and so chooses to enable the option.  The first thing needed in that case is for the user to choose between anonymous and username, not the last.  This should be the default behaviour if anonymous is enabled, with the app able to choose if it is selected by default or not.  If it isn't selected then focus moves to the username.

If the kio-ftp is a special use case of the anonymous feature then it needs to be catered for in a special way consistent with the generic use case of the dialog.  Perhaps look at using the showErrorMessage() api call to add the message about anonymous failing, default the anonymous tickbox to enabled but not selected, and so the username takes first focus so the user can enter the details they need, completely bypassing the anonymous selection box while leaving it there should they want to try again (as futile as that may seem).  This solves your use case cleanly while preserving the general case.

Using radio button may be a solution, but I don't think it's much better visually, you still get the mismatch between widgets with text on the left and right sides, and it's an extra thing to tab through to firstly turn anonymous off and then to get to the username.  I like the look of the Anonymous and Login buttons provided they fit the various sizes needed, it solves the text alignment issue and reduces clutter, but I'd need to do walk-through's of the scenarios to make sure it would be consistent with existing behaviour and api options.

You asked what was wrong with your design?  It does look tidy visually but functionally it is the wrong position.  Besides it really being the first decision not the last as I explained above, the position suggests it's a minor option related to passwords and saving passwords.  Think also of the tab order if it's enabled and selected by default, you have to have a funny tab order where you start at the tickbox then jump back up to the username then down to password and then the OK, it just doesn't flow following the natural visual path of top-to-bottom and left-to-right (or rtl) that people expect.</pre>
 </blockquote>





 <p>On March 25th, 2011, 6:50 p.m., <b>John Layt</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">http://i.imgur.com/THJfp.png</pre>
 </blockquote>








</blockquote>

<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Please give an example other than FTP which allows anonymous login. Anonymous login is an anachronism. It's a workaround for a protocol which does not work without login.

Also, kdelibs is the wrong group for reviewing this. Please take it to kde-usability.</pre>
<br />








<p>- Ingo</p>


<br />
<p>On March 22nd, 2011, 6:04 p.m., Dawit Alemayehu wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for kdelibs.</div>
<div>By Dawit Alemayehu.</div>


<p style="color: grey;"><i>Updated March 22, 2011, 6:04 p.m.</i></p>




<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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 ?</pre>
  </td>
 </tr>
</table>





<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>kdeui/dialogs/kpassworddialog.ui <span style="color: grey">(2649870)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/100920/diff/" style="margin-left: 3em;">View Diff</a></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>

<div>

 <a href="http://git.reviewboard.kde.org/r/100920/s/106/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/03/22/current_password_dialog_box_400x100.png" style="border: 1px black solid;" alt="Current Password Dialog" /></a>

 <a href="http://git.reviewboard.kde.org/r/100920/s/107/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2011/03/22/new_password_dialog_box_400x100.png" style="border: 1px black solid;" alt="New Password Dialog" /></a>

</div>


  </td>
 </tr>
</table>








  </div>
 </body>
</html>