1768792 - Passwords imported from a CSV file aren't synced

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect, P2)

Description

[Mark Hammond [:markh] [:mhammond]]

Saw this in https://www.reddit.com/r/firefox/comments/um9d79/firefox_doesnt_sync_imported_logins_and_passwords/ and was able to reproduce it.

The problem seems to be that the imported password doesn't cause an addLogin/modifyLogin observer notification to be sent, so Sync doesn't know about the new password. I didn't try and dig into why it does not get sent though.

I know this is behind a pref, so disabled by default, but there seems a risk that other imports might have the same behaviour.

Comment 1

[Mark Hammond [:markh] [:mhammond]]

https://github.com/mozilla-services/syncserver/issues/296 implies importing from edge is impacted.

Comment 2

[Tim Giles [:tgiles]]

So if the logins are imported successfully (i.e. they show up in about:logins), then there should be an addLogin/modifyLogin message that is sent from storage-json.js to the respective observers of "passwordmgr-storage-changed" so I'm not sure why this message isn't being picked up by Sync. Maybe it's malformed in someway in the import case and is failing silently, not entirely sure.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

I was 1/2 assuming that maybe they were disabled for performance when doing a bulk import - I didn't find where that might happen, but I didn't look too hard.

Comment 4

[glen226]

(import via CSV

Comment 5

[glen226]

(In reply to glen226 from comment #4)
Previous comment on error; just wanted to add affects Firefox on MacOS (import passwords via CSV not syncing). Some frustration and Googling lead me here.

Comment 7

[EnderCoder99[:nickname]]

Exporting passwords, deleting them and importing them again fails.

Uncaught TypeError: this._errorMessages[errorType] is undefined.
chrome://browser/content/aboutlogins/components/import-error-dialog.mjs:52

Comment 8

[Neil Deakin]

Attachment: WIP: Bug 1768792, send a notification when an logins import has completed and update the sync score when this happens

Comment 9

[Mike Conley (:mconley) (:⚙️)]

If this is an issue with maybeImportLogins (which from looking at the patch, this is where the problem lies), I'll note that our browser migrators currently use this same code path:

https://searchfox.org/mozilla-central/rev/31f5847a4494b3646edabbdd7ea39cb88509afe2/browser/components/migration/MigrationUtils.sys.mjs#848

Would they also be affected by this issue?

Comment 10

[Mike Conley (:mconley) (:⚙️)]

I just tested this locally, and it appears that passwords imported via the migration wizards are also affected.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Marking tracking because we're trying to improve migration in Firefox 114, and getting this fixed in time for that seems like it'd be a good idea.

Comment 12

[Neil Deakin]

Note that this bug is dependent on bug 1752839 which is where the primary part of the fix is.

Comment 13

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

The bug is marked as tracked for firefox114 (nightly). However, the bug still has low severity.

:serg, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 14

[Neil Deakin]

Not sure why this was marked for 114.

Comment 15

[Neil Deakin]

Attachment: Bug 1768792, send a notification when an logins import has completed and update the sync score when this happens, r?#credential-management-reviewers

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/29806c49a699
send a notification when an logins import has completed and update the sync score when this happens, r=credential-management-reviewers,sync-reviewers,sgalich,skhamis

Comment 17

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/29806c49a699

Comment 18

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

Since nightly and release are affected, beta will likely be affected too.
For more information, please visit BugBot documentation.

Comment 19

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

The patch landed in nightly and beta is affected.
:enndeakin, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox115 to wontfix.

For more information, please visit BugBot documentation.