Change reveal password button logic
ClosedPublic

Authored by GB_2 on Feb 7 2018, 2:21 PM.

Details

Summary

Indicate state, not the action it will have. When the password field is empty it feels like the password will be shown once I start typing. Dolphin has been doing it this way for the "show hidden files" action forever.
Also only show the button when the field contains text or when revealing the password, like in the PolKit dialog.

Test Plan

Empty field


Dolphin
Show hidden files, files are currently hidden


Hide hidden files, files are currently shown

Diff Detail

Repository
R242 Plasma Framework (Library)
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Feb 7 2018, 2:21 PM
Restricted Application added projects: Plasma, Frameworks. · View Herald TranscriptFeb 7 2018, 2:21 PM
Restricted Application added subscribers: Frameworks, plasma-devel. · View Herald Transcript
broulik requested review of this revision.Feb 7 2018, 2:21 PM
broulik edited the summary of this revision. (Show Details)
broulik edited the test plan for this revision. (Show Details)Feb 7 2018, 2:25 PM
starbuck added a subscriber: starbuck.EditedFeb 7 2018, 2:26 PM

Would further be also likely more consistent with other toggles in the future, like imagined on mobile "wifi crossed out=disabled", etc.

ngraham accepted this revision.Feb 7 2018, 3:08 PM
ngraham added a subscriber: ngraham.

Makes sense to me, consistency is good.

This revision is now accepted and ready to land.Feb 7 2018, 3:08 PM

Hmm I don't know, that's a button and a button should tell me what happens when I click it (Dolphin is also wrong imho).

Would it be possible to implement this feature like in the Windows 10 lock screen? (i.e. don't toggle the state; reveal the password only as long as the button is pressed). I feel like it would remove the security issue in the first place (as there isn't anymore a persistent state where the password can be visible).

Would it be possible to implement this feature like in the Windows 10 lock screen? (i.e. don't toggle the state; reveal the password only as long as the button is pressed). I feel like it would remove the security issue in the first place (as there isn't anymore a persistent state where the password can be visible).

+1

Restricted Application edited subscribers, added: kde-frameworks-devel; removed: Frameworks. · View Herald TranscriptAug 1 2018, 9:02 PM
GB_2 commandeered this revision.Jun 3 2019, 5:09 PM
GB_2 added a reviewer: broulik.
GB_2 updated this revision to Diff 59081.Jun 3 2019, 5:10 PM
This comment was removed by GB_2.
GB_2 added a comment.EditedJun 3 2019, 5:11 PM

Change onClicked to onPressedChanged

GB_2 edited the summary of this revision. (Show Details)Jun 3 2019, 5:12 PM
GB_2 retitled this revision from Swap reveal password button logic to Change reveal password button logic.

Hmm I don't know, that's a button and a button should tell me what happens when I click it (Dolphin is also wrong imho).

Would it be possible to implement this feature like in the Windows 10 lock screen? (i.e. don't toggle the state; reveal the password only as long as the button is pressed). I feel like it would remove the security issue in the first place (as there isn't anymore a persistent state where the password can be visible).

Done.

ndavis added a subscriber: ndavis.EditedJun 30 2019, 3:04 PM

-1 for the current version

How am I supposed to type with 2 hands and see the password at the same time if I have to hold down the left mouse button?

+1 for the previous version of the patch

This comment was removed by ndavis.

I think the idea is that you're not supposed to be typing with the characters visible as you type because then anyone else who might be around could just see your password. If you're in an environment where security concerns are sufficiently low as to permit this, it would probably make more sense to not even set a password, or use auto-login.

ndavis added a comment.EditedJun 30 2019, 5:12 PM

I think the idea is that you're not supposed to be typing with the characters visible as you type because then anyone else who might be around could just see your password.

I understand this, but I think this is one of those cases where a marginal increase in security should take a backseat to usability. It's not like people can't quickly hide the password again if they already know about the button. If one is afraid of others looking over their shoulder, they shouldn't use the reveal password button in the first place, regardless of how it functions.

If you're in an environment where security concerns are sufficiently low as to permit this, it would probably make more sense to not even set a password, or use auto-login.

I disagree. Just because you aren't likely to have someone looking over your shoulder, doesn't mean nobody will attempt to access your computer while you're not around.

I think the idea is that you're not supposed to be typing with the characters visible as you type because then anyone else who might be around could just see your password.

I understand this, but I think this is one of those cases where a marginal increase in security should take a backseat to usability. It's not like people can't quickly hide the password again if they already know about the button. If one is afraid of others looking over their shoulder, they shouldn't use the reveal password button in the first place, regardless of how it functions.

Yeah, I guess that makes sense.

GB_2 abandoned this revision.Aug 1 2019, 1:01 PM
GB_2 updated this revision to Diff 65540.Sat, Sep 7, 6:34 AM

Change onPressedChanged back to onClicked and only show the button when there is text inside the TextField, like in the PolKit dialog

This revision is now accepted and ready to land.Sat, Sep 7, 6:34 AM
GB_2 updated this revision to Diff 65541.Sat, Sep 7, 6:40 AM

Fix diff

GB_2 requested review of this revision.Sat, Sep 7, 6:42 AM
GB_2 edited the summary of this revision. (Show Details)
GB_2 edited the test plan for this revision. (Show Details)
GB_2 updated this revision to Diff 65551.Sat, Sep 7, 9:04 AM

Use length property and stop revealing password if field is empty

GB_2 edited the test plan for this revision. (Show Details)Sat, Sep 7, 9:06 AM
GB_2 updated this revision to Diff 65585.Sat, Sep 7, 4:13 PM

Fix normal text fields

GB_2 updated this revision to Diff 65592.Sat, Sep 7, 6:25 PM

Don't hide button if revealing password, like in the PolKit dialog and simplify code

GB_2 edited the summary of this revision. (Show Details)Sat, Sep 7, 6:26 PM
ndavis accepted this revision.Sun, Sep 8, 12:50 PM
This revision is now accepted and ready to land.Sun, Sep 8, 12:50 PM
This revision was automatically updated to reflect the committed changes.