KIO::PasswordDialog and the keep checkbox

[Marc Mutz]

Hi!

Wondering why in KMail, no "[ ] Keep password" is displayed, I stumbled 
across the fact that KIO::PasswordDialog::getNameAndPassword() misuses 
the enableKeep parameter of the PasswordDialog ctor:

PasswordDialog::PasswordDialog( ..., bool enableKeep, ... ) {
 //...
  if ( enableKeep ) {
	// create and add "keep password" checkbox
  }
}

PasswordDialog::getNameAndPassword( ... , bool * keep, ... ) {
  //...
  if ( keep )
	dlg = new PasswordDialog( prompt, user, *keep );
  else
	dlg = new PasswordDialog( prompt, user );
 //...
 dlg->exec();
 //...
 if ( keep ) *keep = dlg->keepPassword();
 //...
}

Now, as you can see the keep parameter is used in a two-way fashion:
IN: determine whether or not to show the checkbox
OUT: state of the checkbox.

Now, this alone would be OK (if it was correctly documented, which it 
isn't), but the if ( keep ) in the getNameAndPassword suggests that 
it's keep parameter should be 0 if there should be no checkbox shown 
and *keep should be the preset value of the shown checkbox otherwise.

At least, that's how I interpreted it from looking at the apidocs. And 
indeed, that's the bahaviour that should be implemented, since you 
might want to keep one password, and not the other.

I've grepped kdelibs, kdebase, kdenetwork and koffice to have a look at 
their uses. Results:
- AuthInfo explicitely mentions the two-way-ness of the flag.
- kio/{ob,ui}server.cpp uses it via AuthInfo
- kdeprint uses it with keep = 0
- kmail and knode use it with setting keep = false, which is nonsense,
  since you could save the local bool variable and just pass NULL.

As it stands, KMail allows you to store or not store the passwords of 
accounts on a per-account basis. It would be nice if the Keep password 
checkbox could be used there to let the user decide to keep the 
password when she's first required to enter it.

Currently, we'd need to set the [Passwords]->Keep key of our KConfig 
before calling

bool keep = true; // unintuitive
KConfigGroup passwds( KGlobal::config(), "Passwords" );
passwds.writeEntry( "Keep", ourKeepPasswds() );
if ( QDialog::Accepted == KIO::PasswordDialog::getNameAndPassword( ..., 
&keep, ... ) {
   //...
   setOurKeepPasswds( keep );
   //...
}

which is a bit verbose.

So, can't we add a
  void PasswordDialog::setKeepPassword( bool keep ) { d->keep = keep; }
and use that in getNameAndPassword like this:
  // ...
  dlg = new PasswordDialog( prompt, user, keep /*sic*/ );
  if ( keep )
    dlg->setKeepPassword( *keep );
  // ...

For 3.1, we could misuse slotKeep() for setKeepPassword().

What would that beak? kio/observer, kio/uiserver? They could be fixed 
easily. KNode, KMail and kdeprint would work as they do today (ie. they 
don't show the checkboxes).

Marc

-- 
Ich gegen meinen Bruder.
Ich und mein Bruder gegen unseren Cousin.
Ich, mein Bruder und unser Cousin gegen unsere Nachbarn.
Wir alle gegen den Fremden.
                                      -- Beduinen-Sprichwort
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: signature
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20021116/8e1ee21c/attachment.sig>