1622295 - "Show Password" toggle is not functional after editing password 2 times

Status: NEW

(Toolkit :: Password Manager, defect, P2)

Description

[Timea Cernea [:tbabos][inactive]]

Attachment: Recording of the issue

Affected Builds
Windows 10

Affected version
Release 74 -> using STR2
Beta75 -> using STR2
Nightly 76 -> using STR1 due to the implementation of dismissed doorhanger

Steps to reproduce
STR1:

1. Go to reddit.com login form
2. Fill in only the password field and focus out of the field (to toggle the dismissed doorhanger)
3. Click on the grey icon (check that the "Show Password" option is functional)
4. Click back in the password field and edit the password
5. Click directly the grey key icon after editing the password (Show Password toggle is not displayed)
6. Click back in the password field and edit the password
7. Click directly the grey key icon after editing the password (Show Password toggle is displayed)
8. Click on the 'Show Password toggle"

STR2:
Password Generation option instead of manually typing a password. Steps 1-8.

Expected:
The password should be unmasked.

Actual:
The "Show Password" toggle is not functional. Password remains masked.

Notes:
Dismissing the doorhanger and toggling it again solves the issue.

Comment 1

[Timea Cernea [:tbabos][inactive]]

This mostly occurs since the user has to click out of the password field after editing for the updates to be made in the dismissed doorhanger. In this scenario, we click directly the key icon to toggle the doorhanger and the updates are somehow behind with 1 step.
If I choose to focus out of the field after each editing and then toggle the dismissed doorhanger everything works fine.

Comment 2

[Timea Cernea [:tbabos][inactive]]

Attachment: Log.txt

Comment 3

[Matthew N. [:MattN]]

I'm pretty sure we had an existing bug on this but I can't find it now. Maybe it was fixed once already.

Comment 4

[Matthew N. [:MattN]]

Timea found bug 1576199 which seems similar. I think we didn't fix the root cause there, only one case.

Comment 5

[Sam Foster [:sfoster] (he/him)]

I'll dig into this a little to see what is going on.

Comment 6

[Sam Foster [:sfoster] (he/him)]

Ok I can reproduce this. After editing in the password the second time and re-opening the doorhanger, we are running afoul of some logic in LoginManagerPrompter that only shows the toggle for dismissed doorhangers when they have not yet been opened: https://searchfox.org/mozilla-central/source/toolkit/components/passwordmgr/LoginManagerPrompter.jsm#492

let hideToggle =
    // Dismissed-by-default prompts should still show the toggle.
    (this.timeShown && this.wasDismissed) ||

So fixing this here would presumably break some other cases, and I'm not sure at this point what those are? I think the idea was that a prompt which was created as dismissed should allow toggling of the password visibility, but if a doorhanger was shown and then dismissed by the user, we don't want someone else coming along later and opening it back up to snoop the password. ISTM we already have that case covered though with the VISIBILITY_TOGGLE_MAX_PW_AGE_MS timer in which we'll hide the toggle after 2 minutes?

Note that there is some overlap here with bug 1618587, where we are implementing the OS reauth prompt for this toggle, which replaces all this logic where OS reauth is available. We do still use this logic in the fallback case though so a fix here is still relevant.

Comment 7

[Matthew N. [:MattN]]

Yeah, the idea is that if the user dismissed ever then the option should go away. If a doorhanger starts dismissed then the user dismissal only comes from them opening it and then dismissing it.