Differential D5227

Add a configuration option to hide the show password button
AbandonedPublic
Actions

Authored by fvogt on Mar 28 2017, 5:36 PM.

Details

Reviewers
graesslin
Group Reviewers
Plasma
Summary

The show password button only makes sense with a virtual keyboard
on touchscreens. On desktop PCs it's not only not useful, but it
can also be used to show (partially) typed in passwords.
It also makes it possible to display clipboard entries in plaintext,
which is why the clipboard is cleared when the screen locks.
By adding a configuration option, the user can decide whether the
show password functionality is useful to him.
The default is set to show the button to now change the default
behaviour.

Once the clipboard issue is fixed properly in all cases, the configuration
option can be reevaluated, but currently it is in my opinion the best
way to fix both issues at once.

Test Plan

Ran kscreenlocker_greet --testing with and without the option
changed in the kcm. Worked as expected.

Diff Detail

Repository
R133 KScreenLocker
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
fvogt created this revision.Mar 28 2017, 5:36 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMar 28 2017, 5:36 PM
fvogt updated this revision to Diff 12933.Mar 28 2017, 5:39 PM

Add a comment in greeterapp.cpp and also add a tooltip to the configuration option label.

graesslin requested changes to this revision.Mar 28 2017, 5:57 PM

As already outlined in your thread on distributions I think this is wrong. And as the maintainer of the lock screen I'm against adding this option to the lock screen. It's the typical KDE 3 approach of not knowing what we want to do, so we offer an option.

If your concern is that the clipboard content gets restored then let's work on this. It is very simple to restore the clipboard with the help of Klipper, but as I already outlined I won't be able to do it as I'm on Wayland.

So overall: -2 from maintainer perspective.

greeter/greeterapp.cpp
369–374

This is absolutely unrelated to the config option. Why should the clipboard safety feature be combined with show password.

Please note that we got bug reports about this years before we offered the show password.

This revision now requires changes to proceed.Mar 28 2017, 5:57 PM
fvogt planned changes to this revision.Mar 28 2017, 6:17 PM

I would fix the clipboard content issue with klipper (although I find that approach wrong as it's too complicated as a seemingly unrelated component needs to support the screenlocker), but I do not have enough experience in that area to do it properly.

Once that is fixed, I can remove the condition around the clipboard clear code. I would very much like to keep the show password button configuration option.

It's the typical KDE 3 approach of not knowing what we want to do, so we offer an option.

No, it's the approach of setting a sane default and letting the user change it.

broulik added a subscriber: broulik.Mar 28 2017, 6:48 PM

For show password button there's a kiosk restriction available: lineedit_reveal_password (see https://userbase.kde.org/KDE_System_Administration/Kiosk/Keys)

fvogt added a comment.Mar 28 2017, 6:53 PM
In D5227#98542, @broulik wrote:

For show password button there's a kiosk restriction available: lineedit_reveal_password (see https://userbase.kde.org/KDE_System_Administration/Kiosk/Keys)

Indeed, that works! Now if just there was a user-friendly way to configure it...

IMO kiosk is not really the right place for that and it's still useful for sharing shared passwords, like for WiFi.

graesslin added a comment.Mar 28 2017, 7:15 PM

I'm against adding this option as the greeter does not have any influence over the lnf package. It works with our default but as soon as users change the theme we do not know. I'm against adding new options which only work in the default setup.

fvogt added a comment.Mar 28 2017, 7:23 PM
In D5227#98555, @graesslin wrote:

I'm against adding this option as the greeter does not have any influence over the lnf package. It works with our default but as soon as users change the theme we do not know. I'm against adding new options which only work in the default setup.

Understandably. Is there a way for look-and-feels to offer own configuration options?

fvogt abandoned this revision.Jul 17 2017, 9:11 AM

Revision Contents
Changeset List

PathPackages
Mgreeter/greeterapp.cpp (12 lines)
Mkcfg/kscreenlockersettings.kcfg (5 lines)
Mkcm/kcm.ui (23 lines)

Diff 12933

View Options

greeter/greeterapp.cpp

Loading...
View Options

kcfg/kscreenlockersettings.kcfg

Loading...
View Options

kcm/kcm.ui

Loading...