Open Bug 1536728 Opened 9 months ago Updated 6 days ago

Show a dismissed login capture doorhanger when a user edits a password field (before submission)

Categories

(Toolkit :: Password Manager, enhancement, P1)

enhancement

Tracking

()

ASSIGNED
Tracking Status
firefox71 --- wontfix
firefox72 --- wontfix

People

(Reporter: MattN, Assigned: sfoster)

References

(Blocks 3 open bugs, )

Details

(Keywords: parity-chrome, uiwanted)

Attachments

(2 files)

Currently we wait until we see a form "submission"[1] before showing a doorhanger to save/update a login but it will be hard to detect all times that we should offer to save as there are many different ways that applications with JavaScript can do their log in process (e.g. some combinations of window.location.href = "…", document.cookie = "…", XHR/fetch, location.reload(), <form>, etc. [see deps of bug 1119035). Since we will never cover 100% of the cases where we should prompt, we can reduce the impact of missing a capture signal by allowing the user to save before they even "submit" by showing a dismissed doorhanger as soon as a password (or username?) field is edited.

Chrome Beta 73 already seems to have this feature for password field edits. You can test with attachment 8880688 [details]. As soon as you edit the password field, the key icon appears (the panel doesn't open automatically at that point).

Advantages over manual addition:

  • Doesn't require re-typing or copying+pasting the username and password.
  • Doesn't require the user to correctly identify the login frame's origin (e.g. if it doesn't match the top-level window's origin).
  • Allows capturing other useful data about the form such as the formActionOrigin and other currently unused metadata such as the field IDs/names.

Potential issues:

  • Don't make it too easy for others to see the plaintext password in the doorhanger. We already avoid the toggle when a Master Password is set for the security conscious. Should the icon persist for the same duration even without a capture signal (i.e. across origin navigations).
  • Having the icon appear in its usual icon on the left would cause a shift of the identity block and URL, potentially a bit distracting. Chrome doesn't have this problem since it's on the right side of the URL bar.
  • We probably should not show the doorhanger toggle for edits to autofilled passwords since that makes them too easily revealed to users. We should check how Chrome handles this. We could either not show the doorhanger when it was autofilled previously (we already have that some of that state) or we should hide the checkbox in that case.

[1] Not necessarily a <form> 'submit' event but possibly just a navigation away or some other signal.

Flags: qe-verify+

Could you explore the UX of this in case we have time for it? See the "Potential issues" section above, specifically about the icon causing a visual shift.

Flags: needinfo?(rgaddis)

Having the icon appear in its usual icon on the left would cause a shift of the identity block and URL, potentially a bit distracting. Chrome doesn't have this problem since it's on the right side of the URL bar.

I'm not worried about the shift in the URL bar, should we need to trigger an ability to access the doorhanger. I think it's more important to keep the same location for the login doorhangers.

Bonus points if we toggle it to blue if a change has been detected (that hasn't yet been saved)

Flags: needinfo?(rgaddis)
Priority: P2 → P1
Depends on: 1585952

Potential issues:

  • Don't make it too easy for others to see the plaintext password in the doorhanger. We already avoid the toggle when a Master Password is set for the security conscious. Should the icon persist for the same duration even without a capture signal (i.e. across origin navigations).

Some %age of login forms cross from e.g. login.example.com to example.com. A smaller %age of those doesn't use a form submission or other signal we can detect to know a login has been submitted. I don't know what that %age is, so its hard to know how to handle the trade-off as we balance the concerns about exposing a password via the doorhanger vs. not giving the user the opportunity to persist the password. I think there's overlap here with how we handle generated passwords. We're not going to auto-save a login for password field edits, but borrowing some of the generated password doorhanger behavior seems like a good place to start with this.

  • We probably should not show the doorhanger toggle for edits to autofilled passwords since that makes them too easily revealed to users. We should check how Chrome handles this. We could either not show the doorhanger when it was autofilled previously (we already have that some of that state) or we should hide the checkbox in that case.

Katie, do you have a preference here?

Also, should we have the same dismissed capture doorhanger behavior for private windows? Or skip this entirely.

Flags: needinfo?(kcaldwell)

Hey Sam, based on our conversation, we decided:
• remove show password toggle from doorhanger for autofill +edit (for pages where we can't detect fields)
• behaviour should be the same in Private and non-Private windows

Flags: needinfo?(kcaldwell)

I think my general approach here is to repurpose the generatedPasswordFilledOrEdited stuff (on both the child and parent side.) We need the same child-side edit detection and messaging (though we don't need the blur/focus event handling for non-generated password edits as we aren't going to toggle password masking.) In the parent, we need to do a lot of the same work figuring out what kind of dismissed doorhanger - if any - we need to show.

I'll refactor without functional changes and ensure nothing regresses in the first patch, and implement the non-generated password modification doorhanger in a 2nd patch. This becomes pretty straightforward with the changes in 1388674, so I'll base on those patches.

Assignee: nobody → sfoster
Status: NEW → ASSIGNED
Depends on: 1388674
Blocks: 1598541
  • No functional changes.
  • Rename *GeneratedPasswordFilledOrEdited to passwordFilledOrEdited and pass a 'generated' flag

Depends on D24556

From Ryan, Comment 2

Bonus points if we toggle it to blue if a change has been detected (that hasn't yet been saved)

Sam, did we get this in as part of this fix?

Flags: needinfo?(sfoster)

(In reply to katieC from comment #8)

From Ryan, Comment 2

Bonus points if we toggle it to blue if a change has been detected (that hasn't yet been saved)

Sam, did we get this in as part of this fix?

This bug hasn't been resolved yet but I also don't think we should do this (at least not in this bug) as it's changing the meaning of the blue colour… now it means that the login was automatically saved but the new meaning would be different.

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