1630191 - The "Show password" toggle is not displayed in the dismissed/save doorhanger on form submit for fields where new credentials were filled in after deleting autofilled ones

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect, P1)

Description

[Timea Cernea [:tbabos][inactive]]

Attachment: Recording of the issue

Affected Versions
Treeherder build: Nightly 77.0a1 ( 20200415040917) - with fix for Bug 1627658
Beta 76.0b4 (64-bit) - only for the dismissed doorhanger part, Beta is still affected by Bug 1627658 so it doesn't show the save doorhanger on form submit to check Expected 5.1

Tested on
Windows 10 x64
Ubuntu 18.04

Steps to reproduce

1. Have 1 set of credentials saved on facebook.com
2. Load the Log in Form (ensure credentials are autofilled)
3. Delete the autofilled credentials and type in new ones (observe the grey key icon)
4. Click on the grey icon to toggle the dismissed doorhanger
   4.1 Check if the "Show password" checkbox is displayed (Nightly and Beta)
5. Click on Log In to toggle the save doorhanger on form submit
   5.1.Check if the "Show password" checkbox is displayed (Nightly only)

Expected
4.1 The "Show password" toggle should be displayed for new credentials in the dismissed doorhanger (Nightly and Beta)
5.1 The "Show password" toggle should be displayed for new credentials in the save doorhanger that appears on form submit (Nightly only)

Actual
4.1 The "Show password" toggle is not displayed for new credentials in the dismissed doorhanger (Nightly and Beta)
5.1 The "Show password" toggle is not displayed for new credentials in the save doorhanger that appears on form submit (Nightly only)

Regression-Range
Introduced via the implementation of Dismissed Doorhanger, Bug 1536728. The show toggle appears correctly in the Save doorhanger on form submit by following the same scenario in Release 74 and Release 75.

Note
Release 75 has the pref disabled for the dismiss doorhanger.

Comment 1

[Timea Cernea [:tbabos][inactive]]

Attachment: Log.txt

Comment 2

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

In bug 1618587 we started sending along the login of the autofilled guid when opening the save/update doorhanger, and using it to hide/show the "Show password" toggle. That is correct, but it exposes an issue where we are not clearing the autofilled guid when the values of the form change.

We need closer tracking in LoginManagerChild when the username and password fields are cleared or overwritten such that the initial autofilled login no longer matches. This needs to be done with care as the point of bug 1618587 was to prevent using the doorhanger as a simple way to view a saved password by making a trivial edit to the username or password.

We do something similar for fields that have been filled with generated passwords in _maybeStopTreatingAsGeneratedPasswordField - if we get an input event which replaces the value we clear the flag on that field. Something similar would allow us to clear the flag in docState.fillsbyRootElement so that the lookup in _maybeSendFormInteractionMessage fails and the message sent to the parent has a falsy value for autoFilledLoginGuid

Comment 3

[Matthew N. [:MattN]]

We don't think this needs to block 76 but if we get it done in time we will uplift.

Comment 4

[:severin Rudie]

Attachment: Bug 1630191 - enable 'show password' toggle after deleting autofilled password. r=sfoster

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1866cc0edaf2
enable 'show password' toggle after deleting autofilled password. r=sfoster,MattN

Comment 6

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1866cc0edaf2

Comment 7

[Matthew N. [:MattN]]

We should request uplift once QA verifies.

Comment 8

[Timea Cernea [:tbabos][inactive]]

Verified-fixed on latest Nightly 78.0a1 (220-05-08) on Windows 10, MacOS 10.13 and Ubuntu 16.04.
Waiting for uplift to Beta.

Comment 9

[Pascal Chevrel:pascalc (PTO until Sept 2)]

Severin, do you want to request an uplift to beta?

Comment 10

[:severin Rudie]

Attachment: Bug 1630191 - enable 'show password' toggle after deleting autofilled password.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1618587
[User impact if declined]: Users who delete/replace an autofilled password will not see a "show password" toggle for their new password.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: No
[Why is the change risky/not risky?]: The patch changes what the password manager internally considers to have been autofilled. This narrows the potential impact to the reasonably short list of things that rely on that (autofilled fields turning yellow, some telemetry, etc). Test coverage is also pretty good in this area.
[String changes made/needed]: None

Comment 11

[Pascal Chevrel:pascalc (PTO until Sept 2)]

Comment on attachment 9147362 [details]
Bug 1630191 - enable 'show password' toggle after deleting autofilled password.

Fix for a P1 regression, patch has tests and was verified in Nightly, let's take it to beta, thanks!

Comment 12

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/383f15236ed0

Comment 13

[Timea Cernea [:tbabos][inactive]]

Verified-fixed on latest Beta 77.0b5 on Windows 10 x64 and MacOS 10.13.