Open Bug 1622295 Opened 9 months ago Updated 3 months ago

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

Categories

(Toolkit :: Password Manager, defect, P2)

Desktop
All
defect

Tracking

()

Tracking Status
firefox74 --- affected
firefox75 --- affected
firefox76 --- affected

People

(Reporter: tbabos, Unassigned)

References

Details

(Whiteboard: [passwords:capture-UI])

Attachments

(2 files)

Attached video 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.

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.

Attached file Log.txt

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

Keywords: dupeme
Whiteboard: [passwords:capture-UI]

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

Blocks: 1576199
Keywords: dupeme
Priority: -- → P2

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

Flags: needinfo?(sfoster)

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.

Flags: needinfo?(sfoster) → needinfo?(MattN+bmo)

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.

Flags: needinfo?(mozilla+bmo)
You need to log in before you can comment on or make changes to this bug.