This bug is logged for tracing the rest issue/improvement for bug 245813: So far, the fix of bug 245813 is: After click "Cancel" from the Master Password Dialog. It displays 3 times for those stored password POP accounts. And it displays 1 time for those non-store password POP accounts. Actually, from user point of view, Master Password dialog shouldn't display for those non-store password POP accounts...(This is how Netscape 7.1 did) Steps: 1) Setup 4 POP mail acocunts without "checked for new msgs at startup" 2) Setup 1 POP mail acocunt "check for new msgs every 1 minute" 3) Saved Passwords for 2 POP mail acocunts 4) Checked Preferences|Passwords|Use encryption when storing sensitive data 5) After select "Get Msgs", notice Master Password dialog displays for those stored password POP accounts. 6) After select "Cancel" 7) Select "Get Msgs" for those non-store password POP accounts. Actual Results: Master Password Dialog still displays for those non-store password POP accounts Expected results: Master Password Dialog shouldn't display for those non-store password POP accounts.

I used latest Mozilla 1.7 branch build (06-17)...

*** Bug 247623 has been marked as a duplicate of this bug. ***

This bug regressed between linux trunk builds 2004092205 and 2004092305, indicating this is a regression from bug 219976.

I'd say this is a wallet bug. the xpcom frontend for wallet uses the enumerator to get all the urls, usernames and passwords. In clear text. So the master password is needed. Only after that, it goes to search for the right url. It should look for the right url first, and then decypher the username and password.

Decyption should happen only if the uri's match. Some might think of this as a hack. I think of all of wallet as a hack, so one extra won't hurt.

Comment on attachment 151220 [details] [diff] [review] delay decryption >Index: extensions/wallet/src/nsPasswordManager.cpp >- nsCOMPtr<nsISimpleEnumerator> enumerator; >- rv = GetEnumerator(getter_AddRefs(enumerator)); >- if (NS_FAILED(rv)) { >- return rv; // could not unlock the database >+ nsPasswordManagerEnumerator* enumerator = new nsPasswordManagerEnumerator(PR_FALSE); who frees this enumerator now? it used to be COMPtr, which would clean itself up after it goes out of scope. > while (hasMoreElements) { > rv = enumerator->GetNext(getter_AddRefs(passwordElem)); > if (NS_FAILED(rv)) > return rv; here's an early return that appears to leak |enumerator|. As for the core functionality change, I think you should poke dveditz for a review.

Patch updated to darins comments. It's a comptr again.

Comment on attachment 154006 [details] [diff] [review] updated carrying timeless' review forward. dveditz can you sr? darin: there isn't a real functionality changes. the patch only skips decrypting passwords for entries that are thrown away anyway later, because the url's don't match. The end result is the same.

Comment on attachment 154006 [details] [diff] [review] updated sr=dveditz

checked in

this should be checked into the aviary branch, right? If I don't hear any dissenting voices, I'll check this into the aviary branch.

*** Bug 185786 has been marked as a duplicate of this bug. ***

*** Bug 236510 has been marked as a duplicate of this bug. ***

this is what I'll check into the aviary branch.

fixed on 1.0 branch

Comment on attachment 154708 [details] [diff] [review] -up diff for aviary branch getting on radar for 1.7.12

*** Bug 306710 has been marked as a duplicate of this bug. ***

Comment on attachment 154708 [details] [diff] [review] -up diff for aviary branch out of scope for a security release