Bug 1536728 adds the ability to show a dismissed login capture doorhanger when the user edits a password field. If there is a username field in the form, this should be observed and used in the matching we do against saved logins, and to update the username in the doorhanger.

The bug applies to both generated password and regular password field edits of a saved login. There a few variations in behavior based on what logins are saved, whether the form is autofilled or not, etc.

Editing a password STR for this bug are:
0. Have a saved login for the site like user1/pass1

1. Load a login form for the site, the saved login should be autofilled.
2. Edit the password, this should create a dismissed doorhanger. Opening the doorhanger shows user1 in the username field, and the edited password in the password field
3. Edit the username field to e.g. user2. (and moved focus to generate a change event)

Expected:
The username field in the doorhanger should match the value in the form.

Actual:
The username field in the doorhanger reflects the previously filled value (user1)

For the generated-password case, the basic STR for this bug are:

1. Fill the password field with a new generated password (a blue dismissed doorhanger is created and a "Password saved" toast shown)
2. Edit the value of the username field (and move focus to generate a change event.)

Expected:
When the doorhanger is opened, the username field should match the value from the form

Actual:
The username field in the doorhanger is empty (or whatever the original value was before step 2)

One consideration is that in the generated password case, when updating the doorhanger we don't want to clear the "attention" state of that doorhanger which indicates that a password has been auto-saved. We can pass the same arguments to promptToChangePassword to keep the "attention" state on the new doorhanger, but that also has the side effect of calling ConfirmationHint.show(..). So the user edits the username and see a new toast saying "Password saved" - which isn't really true, nothing new was saved at this point.

I didn't really to want modify LoginManagerPrompter's method signatures in this bug to tease apart these two behaviors, but this has come up before and might be a necessary step here. The choices seem to be:

1. Clear the attention state when the username is edited. We get no extra toast, but the icon loses its blue "attention" highlight from the earlier auto-saving of the generated password
2. Retain the attention state and live with the extra "Password saved" toast generated when the username field is changed
3. Retain the attention state but use a new string for the toast when the username field is changed
4. Update the LoginManagerPrompter's method signatures to have separate arguments for the doorhanger attention state and the toast/confirmation hint so we can keep the attention state when necessary but avoid new toasts.

(In reply to Sam Foster [:sfoster] (he/him) from comment #2)

One consideration is that in the generated password case, when updating the doorhanger we don't want to clear the "attention" state of that doorhanger which indicates that a password has been auto-saved. We can pass the same arguments to promptToChangePassword to keep the "attention" state on the new doorhanger, but that also has the side effect of calling ConfirmationHint.show(..). So the user edits the username and see a new toast saying "Password saved" - which isn't really true, nothing new was saved at this point.

I didn't really to want modify LoginManagerPrompter's method signatures in this bug to tease apart these two behaviors, but this has come up before and might be a necessary step here. The choices seem to be:

1. Clear the attention state when the username is edited. We get no extra toast, but the icon loses its blue "attention" highlight from the earlier auto-saving of the generated password

This wouldn't be the end of the world.

2. Retain the attention state and live with the extra "Password saved" toast generated when the username field is changed

I don't like this since it's misleading since the password didn't change.

3. Retain the attention state but use a new string for the toast when the username field is changed

This seems unnecessarily distracting

4. Update the LoginManagerPrompter's method signatures to have separate arguments for the doorhanger attention state and the toast/confirmation hint so we can keep the attention state when necessary but avoid new toasts.

This seems reasonable too.

I wonder if there is an option 5:

Only show the toast if the password actually changed. Or is there a case where we should the toast but login.password == aOldLogin.password? https://searchfox.org/mozilla-central/rev/fca0be7e2cf2f922c9b927423ce28e8a04b3fd90/toolkit/components/passwordmgr/LoginManagerPrompter.jsm#573,600

I wonder if there is an option 5:

Only show the toast if the password actually changed. Or is there a case where we should the toast but login.password == aOldLogin.password? https://searchfox.org/mozilla-central/rev/fca0be7e2cf2f922c9b927423ce28e8a04b3fd90/toolkit/components/passwordmgr/LoginManagerPrompter.jsm#573,600

If the edit was to a generated password that we just auto-saved, then we do update the auto-saved login in storage. But if the username changed so that the edit is to a different saved login, we don't persist that until the user ok's it in the doorhanger. So I don't think there is a case where we should display the toast but login.password == aOldLogin.password

So I think I'll go with option 1 for the generated password case here and we can triage the LoginManagerPrompter changes that would allow more fine-grained handling of when to notify vs. when to use the attention state.

Hrm, I'm still getting test failures on that new "Change to existing username, password" test in browser_doorhanger_form_password_edit.js but haven't been able to reproduce locally:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d598baff7b251b65f500f315d2f91d0f6b434074&selectedJob=290700572

https://hg.mozilla.org/mozilla-central/rev/244195c33411

Hey Sam,
Works as expected for the first scenario with usual username and password edits. However, for the Generate Password one there is always a new "Password Saved" toast displayed after each username edit.

If I understood correctly, it should be handled like this: Clear the attention state when the username is edited. We get no extra toast, but the icon loses its blue "attention" highlight from the earlier auto-saving of the generated password.

(In reply to Timea Cernea [:tbabos] from comment #9)

Works as expected for the first scenario with usual username and password edits. However, for the Generate Password one there is always a new "Password Saved" toast displayed after each username edit.

If I understood correctly, it should be handled like this: Clear the attention state when the username is edited. We get no extra toast, but the icon loses its blue "attention" highlight from the earlier auto-saving of the generated password.

Can you file this as a bug? We shouldn't get the additional toasts there. The doorhanger should show the changed username, but those additional toasts are misleading as the auto-saved login uses an empty username and shouldn't need to change with each edit.

Bug 1625242 submitted for that. Waiting for that to be fixed before any further QA actions on this bug.