D10362: Change reveal password button logic

[Björn Feber]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:fb18347e4dea: Change reveal password button logic 
(authored by GB_2).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10362?vs=65592=65681

REVISION DETAIL
  https://phabricator.kde.org/D10362

AFFECTED FILES
  src/declarativeimports/plasmacomponents/qml/TextField.qml

To: GB_2, #plasma, #vdg, ngraham, broulik, ndavis
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Björn Feber]

GB_2 updated this revision to Diff 65592.
GB_2 added a comment.


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

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10362?vs=65585=65592

BRANCH
  arcpatch-D10362

REVISION DETAIL
  https://phabricator.kde.org/D10362

AFFECTED FILES
  src/declarativeimports/plasmacomponents/qml/TextField.qml

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Björn Feber]

GB_2 updated this revision to Diff 65585.
GB_2 added a comment.


  Fix normal text fields

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10362?vs=65551=65585

BRANCH
  arcpatch-D10362

REVISION DETAIL
  https://phabricator.kde.org/D10362

AFFECTED FILES
  src/declarativeimports/plasmacomponents/qml/TextField.qml

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Björn Feber]

GB_2 updated this revision to Diff 65551.
GB_2 added a comment.


  Use `length` property and stop revealing password if field is empty

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10362?vs=65541=65551

BRANCH
  arcpatch-D10362

REVISION DETAIL
  https://phabricator.kde.org/D10362

AFFECTED FILES
  src/declarativeimports/plasmacomponents/qml/TextField.qml

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Björn Feber]

GB_2 updated this revision to Diff 65541.
GB_2 added a comment.


  Fix diff

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10362?vs=65540=65541

BRANCH
  arcpatch-D10362

REVISION DETAIL
  https://phabricator.kde.org/D10362

AFFECTED FILES
  src/declarativeimports/plasmacomponents/qml/TextField.qml

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Björn Feber]

GB_2 updated this revision to Diff 65540.
GB_2 added a comment.
This revision is now accepted and ready to land.


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

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10362?vs=59081=65540

BRANCH
  arcpatch-D10362

REVISION DETAIL
  https://phabricator.kde.org/D10362

AFFECTED FILES
  src/declarativeimports/plasmacomponents/qml/TextField.qml

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Nathaniel Graham]

ngraham added a comment.


  In D10362#488571 , @ndavis wrote:
  
  > In D10362#488389 , @ngraham 
wrote:
  >
  > > 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.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  change-reveal-password-button-logic (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D10362

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Noah Davis]

ndavis added a comment.


  In D10362#488389 , @ngraham wrote:
  
  > 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. 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. It's not like people 
can't quickly hide the password again if they already know about the button.
  
  > 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.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  change-reveal-password-button-logic (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D10362

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Nathaniel Graham]

ngraham added a comment.


  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.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  change-reveal-password-button-logic (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D10362

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Noah Davis]

ndavis added a comment.


  In D10362#202435 , @starbuck wrote:
  
  > Would further be also likely more consistent with other toggles in the 
future, like imagined on mobile "wifi crossed out=disabled", etc.
  
  
  The difference between the WiFi toggle and the visibility toggle is that the 
WiFi toggle has a checkbox next to it while this doesn't.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  change-reveal-password-button-logic (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D10362

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Noah Davis]

ndavis added a comment.


  -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?
  
  -0.75 for the previous version of the patch
  
  We normally show the action that will happen when a button is clicked. Why 
change the behavior here? The state is already know to the user simply by 
seeing that their password is exposed.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  change-reveal-password-button-logic (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D10362

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: ndavis, kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, 
starbuck, plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, 
Pitel, michaelh, ZrenBot, bruns, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Björn Feber]

GB_2 added a comment.


  Ping

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  change-reveal-password-button-logic (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D10362

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, starbuck, 
plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
michaelh, ZrenBot, bruns, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart

D10362: Change reveal password button logic

[Björn Feber]

GB_2 added a comment.


  In D10362#202592 , 
@elvisangelaccio wrote:
  
  > 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.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  change-reveal-password-button-logic (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D10362

To: GB_2, #plasma, #vdg, ngraham, broulik
Cc: kde-frameworks-devel, rizzitello, elvisangelaccio, ngraham, starbuck, 
plasma-devel, ericadams, jraleigh, GB_2, ragreen, Pitel, michaelh, ZrenBot, 
bruns, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, 
mart