From 82696f5ff13a27d86ad2254003b04d18e7faba4e Mon Sep 17 00:00:00 2001 From: "dolske@mozilla.com" Date: Mon, 19 Nov 2007 20:51:40 -0800 Subject: [PATCH] Bug 380931 - Post-landing cleanup from Bug 374723 (rewrite password manager in JS). r=gavin, a19=schrep --- .../passwordmgr/src/nsLoginManager.js | 25 +++---------------- .../passwordmgr/src/nsLoginManagerPrompter.js | 12 ++++----- .../passwordmgr/src/storage-Legacy.js | 2 +- 3 files changed, 11 insertions(+), 28 deletions(-) diff --git a/toolkit/components/passwordmgr/src/nsLoginManager.js b/toolkit/components/passwordmgr/src/nsLoginManager.js index 5b1b816eb8b..4f4b95fc060 100644 --- a/toolkit/components/passwordmgr/src/nsLoginManager.js +++ b/toolkit/components/passwordmgr/src/nsLoginManager.js @@ -524,9 +524,6 @@ LoginManager.prototype = { } } } else { - // XXX The C++ code took care to avoid reentrancy if a - // master-password dialog was triggered here, but since - // we're decrypting at load time that can't happen right now. this.log("Creating new autocomplete search result."); var doc = aElement.ownerDocument; @@ -698,8 +695,6 @@ LoginManager.prototype = { * [Note that this happens before any DOM onsubmit handlers are invoked.] * Looks for a password change in the submitted form, so we can update * our stored password. - * - * XXX update actionURL of existing login, even if pw not being changed? */ _onFormSubmit : function (form) { @@ -770,17 +765,9 @@ LoginManager.prototype = { var logins = this.findLogins({}, hostname, formSubmitURL, null); - // XXX we could be smarter here: look for a login matching the - // old password value. If there's only one, update it. If there's - // more than one we could filter the list (but, edge case: the - // login for the pwchange is in pwmgr, but with an outdated - // password. and the user has another login, with the same - // password as the form login's old password.) ugh. - // XXX if you're changing a password, and there's no username - // in the form, then you can't add the login. Will need to change - // prompting to allow this. - if (logins.length == 0) { + // Could prompt to save this as a new password-only login. + // This seems uncommon, and might be wrong, so ignore. this.log("(no logins for this host -- pwchange ignored)"); return; } @@ -985,7 +972,6 @@ LoginManager.prototype = { // Attach autocomplete stuff to the username field, if we have // one. This is normally used to select from multiple accounts, // but even with one account we should refill if the user edits. - // XXX should be able to pass in |logins| to init attachment if (usernameField) this._attachToInput(usernameField); @@ -1074,13 +1060,10 @@ LoginManager.prototype = { return; } - // XXX: we could do better on forms with 2 or 3 password fields. + // If there are multiple passwords fields, we can't really figure + // out what each field is for, so just fill out the last field. var passwordField = pwFields[0].element; - // XXX this would really be cleaner if we could get at the - // AutoCompleteResult, which has the actual nsILoginInfo for the - // username selected. - // Temporary LoginInfo with the info we know. var currentLogin = new this._nsLoginInfo(); currentLogin.init(hostname, formSubmitURL, null, diff --git a/toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js b/toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js index 1160e86bcc7..442001850ff 100644 --- a/toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js +++ b/toolkit/components/passwordmgr/src/nsLoginManagerPrompter.js @@ -91,7 +91,7 @@ LoginManagerPrompter.prototype = { [Ci.nsIAuthPrompt2, Ci.nsILoginManagerPrompter]), _window : null, - _debug : false, + _debug : false, // mirrors signon.debug __pwmgr : null, // Password Manager service get _pwmgr() { @@ -192,9 +192,6 @@ LoginManagerPrompter.prototype = { // If the user submits a login but it fails, we need to remove the // notification bar that was displayed. Conveniently, the user will // be prompted for authentication again, which brings us here. - // XXX this isn't right if there are multiple logins on a page (eg, - // 2 images from different http realms). That seems like an edge - // case that we're probably not handling right anyway. var notifyBox = this._getNotifyBox(); if (notifyBox) this._removeSaveLoginNotification(notifyBox); @@ -207,8 +204,7 @@ LoginManagerPrompter.prototype = { var foundLogins = this._pwmgr.findLogins({}, hostname, null, httpRealm); - // XXX Like the original code, we can't deal with multiple - // account selection. (bug 227632) + // XXX Can't select from multiple accounts yet. (bug 227632) if (foundLogins.length > 0) { selectedLogin = foundLogins[0]; this._SetAuthInfo(aAuthInfo, selectedLogin.username, @@ -299,6 +295,10 @@ LoginManagerPrompter.prototype = { */ init : function (aWindow) { this._window = aWindow; + + var prefBranch = Cc["@mozilla.org/preferences-service;1"]. + getService(Ci.nsIPrefService).getBranch("signon."); + this._debug = prefBranch.getBoolPref("debug"); this.log("===== initialized ====="); }, diff --git a/toolkit/components/passwordmgr/src/storage-Legacy.js b/toolkit/components/passwordmgr/src/storage-Legacy.js index f8c64384a0e..9918473e2e6 100644 --- a/toolkit/components/passwordmgr/src/storage-Legacy.js +++ b/toolkit/components/passwordmgr/src/storage-Legacy.js @@ -546,7 +546,7 @@ LoginManagerStorage_legacy.prototype = { // Format is "http://site.com", with "(some realm)" // appended if it's a HTTP-Auth login. - const realmFormat = /^(.+?)( \(.*\))?$/; // XXX .* or .+? + const realmFormat = /^(.+?)( \(.*\))?$/; var matches = realmFormat.exec(hostrealm); var hostname, httpRealm;