329117 - Password Manager removes incorrect entries when multiple are entered for the same domain/user

Status: RESOLVED WORKSFORME

(Toolkit :: Password Manager, defect)

Description

[Glen Murphy]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.1) Gecko/20060111 Firefox/1.5.0.1

nsiPasswordManager::removeUser only takes domain and user as arguments, but it's possible to have multiple password manager entries with the same domain and user, but different passwords (and userFieldNames and passFieldNames) - so when deleting a user, the one that gets deleted is arbitrary.

Reproducible: Sometimes

Steps to Reproduce:
1. Go to http://glenmurphy.com/tmp/login1.html
2. Enter 'one' as username, and 'one' as password
3. Go to http://glenmurphy.com/tmp/login2.html
4. Enter 'one' as username, and 'two' as password

(note that this two forms have different username and password field names, so things should be remembered correctly)

5. Go to password manager, view passwords, show passwords
6. Select the entry with 'one' as the password and press 'remove'
7. Close the view passwords dialog, reopen it, show passwords.
Actual Results:  
The entry with 'one' as the password is still there, and 'two' has been removed instead.

Expected Results:  
Should have deleted the entry with 'one' as the password

Comment 1

[Marco Bonardo [:mak]]

This is still valid in 2.0.0.4
Mozilla/5.0 (Windows; U; Windows NT 6.0; it; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4

Instead on Trunk only the first login is saved. the second one is ignored
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a7pre) Gecko/2007070614 Minefield/3.0a7pre
This feels like a slightly different bug, that maybe should be created

Comment 2

[Justin Dolske [:Dolske]]

WFM on current trunk. Also tried deleting in the reverse order.

The old password manager was bad about mapping strings to logins, and could get confused in these kinds of cases. The new password manager keeps the actual logins around and modifies them based on the selected index, so it's more robust about not confusing similar logins.