1604472 - Credentials and generated passwords saved in Private Browsing mode can't be merged

Status: VERIFIED FIXED

(Toolkit :: Password Manager, defect, P2)

Description

[Timea Cernea [:tbabos][inactive]]

Attachment: LogExport.txt

Affected versions
Nightly 73
Beta 72

Affected platforms
Windows 10 x64 (to be checked on the other OS)

Steps to reproduce

1. Launch Firefox and use Private Browsing window
2. Go to reddit.com
3. Save a set of credentials
4. Re-load login form
5. Generate a secure password
6. Re-open form
7. Fill in the username with the saved credential
8. Delete the filled-in password and replace it with the "No username" entry -> generated password
9. Click on Log in
10. Choose to "Update" the entry in the displayed doorhanger

Expected result
The entries should be merged in about:logins.
The initial username should have the generated password
The "No username" entry should be deleted given it was merged

Actual Result
Both entries are still displayed in about:logins.
The username does not have the new(generated) password updated
The "No username" entry is still displayed in about:logins because it was not merged

Note
This happens ONLY to credentials/generatedPasswords saved in Private Browsing mode.
Those credentials can't be merged in normal browsing mode either.
Saving credentials in normal browsing and merging them in Private Browsing WORKS.

Comment 1

[Timea Cernea [:tbabos][inactive]]

Attachment: Recording of the issue

Comment 2

[Matthew N. [:MattN]]

Thanks for the logs, The relevant part is:

LoginManagerPrompter: multiple logins, loginToRemove: undefined LoginManagerPrompter.jsm:1189:14
Unexpected match of multiple logins. LoginManagerPrompter.jsm:1193
persistData resource://gre/modules/LoginManagerPrompter.jsm:1193
callback resource://gre/modules/LoginManagerPrompter.jsm:1247
_onButtonEvent resource://gre/modules/PopupNotifications.jsm:1894
oncommand chrome://browser/content/browser.xhtml:1

Comment 3

[Matthew N. [:MattN]]

Hi Timea, does the problem still happen if you shut off autofilling of logins (signon.autofillForms)?

Comment 4

[Timea Cernea [:tbabos][inactive]]

Switched "signon.autofillForms" to False and I still happens as before.

Comment 5

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

Attachment: Bug 1604472 - Use documentPrincipal.origin when looking up generated passwords onFormSubmit. r?MattN

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/42a0445eebdb
Use documentPrincipal.origin when looking up generated passwords onFormSubmit. r=MattN

Comment 8

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/42a0445eebdb

Comment 9

[Timea Cernea [:tbabos][inactive]]

This is fixed now on the latest Nightly 73 - Build ID (20191220041517) - Windows 10. Verified the following scenarios and possible regression/leftovers:

Save and merge credentials in Private Browsing - works
Save and merge credentials in Normal Browsing - works
Save credential in normal and save in Private -> Doesn't work
Save credentials in private and save in Normal -> Doesn't work.

Marking this as Verified-fixed and for the remaining issue, I submitted Bug 1605322.
FYI, this patch also fixed Bug 1604452.

Waiting for this to be uplifted in Beta.
CC Adrian, I will be in PTO for the next 2 weeks and can't verify further.

Comment 10

[Matthew N. [:MattN]]

Comment on attachment 9116873 [details]
Bug 1604472 - Use documentPrincipal.origin when looking up generated passwords onFormSubmit. r?MattN

Beta/Release Uplift Approval Request

• User impact if declined: A user who generates a password in private window can't merge it with an existing login
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is simply fixing the wrong origin being used in a lookup to a Map, it was using the origin without the suffix but now it will use the suffix.
• String changes made/needed: None

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 9116873 [details]
Bug 1604472 - Use documentPrincipal.origin when looking up generated passwords onFormSubmit. r?MattN

fix for password generation in private windows, approved for 72.0b11

Comment 12

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/releases/mozilla-beta/rev/388c4975969e

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug is a regression, but please revert this change in case of error.

Comment 14

[Adrian Florinescu [:aflorinescu]]

Verified the fix with 72.0b11 2019-12-27 on Ubuntu 16.04/ Windows 10 / Mac 10.15.1 following update & merge scenario ( comment 0 ) , excluding the 'doesn't work' scenarios from comment 9 (filled in as Bug 1605322) which will not be present in 72, since they weren't nor would be uplifted.