Closed Bug 468916 Opened 13 years ago Closed 13 years ago
Login() smarter than just remove+add
The current implementation of modifyLogin in storage-mozStorage.js is a bit lazy, in that it's just a call to removeLogin() and addLogin() wrapped in a transaction. I'd like to change this to actually modifying an existing row. This is specifically needed for bug 467463, so that we can modify a login without changing its GUID. This is clear way of operation, although it does mean adding a bit of extra code instead of just leveraging removeLogin/addLogin.
Adds a bit of testing for the "modify a non-existent login" case, other tests cover normal usage cases. (Specifically, the prompt tests exercise changing passwords).
Attachment #352391 - Flags: review?(paul) → review+
Comment on attachment 352391 [details] [diff] [review] Patch v.2 >+ let userCanceled, encUsername, encPassword; >+ // Get the encrypted value of the username and password. >+ [encUsername, userCanceled] = this._encrypt(newLogin.username); >+ if (userCanceled) >+ throw "User canceled master password entry, login not modified."; >+ >+ [encPassword, userCanceled] = this._encrypt(newLogin.password); >+ // Probably can't hit this case, but for completeness... >+ if (userCanceled) >+ throw "User canceled master password entry, login not modified."; We now do this in a couple places (here and |_addLogin()|), just with different messages for the throw. Does it make sense to pull this into another method, or is it fine as is? Otherwise r+ by me.
Yeah, we could do a: [encUser, encPass, userCanceled] = encryptStuff(aLogin);
Comment on attachment 352391 [details] [diff] [review] Patch v.2 Not sure if I can delegate reviews of my own patches. :)
Attachment #352391 - Flags: review?(gavin.sharp)
Attachment #352391 - Flags: review?(gavin.sharp) → review+
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment on attachment 352391 [details] [diff] [review] Patch v.2 a191=beltzner
Attachment #352391 - Flags: approval1.9.1? → approval1.9.1+
You need to log in before you can comment on or make changes to this bug.