Bug 1135451 - fillForm() cleanup part D: Use early returns to un-nest logic and make it more obvious that selectedLogin is always set. r=MattN
authorJustin Dolske <dolske@mozilla.com>
Fri, 27 Feb 2015 15:47:37 -0800
changeset 231212 7171e15ba2ffd34b9f002b0ca7a68a331832e496
parent 231211 01a4a61f9db795c078203221fc921ae4bb28b1cf
child 231213 d4f4f8cf938bb878083fee8b8d3264027db694b9
push id11542
push userjdolske@mozilla.com
push dateFri, 27 Feb 2015 23:48:29 +0000
treeherderfx-team@a80f93233b34 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersMattN
bugs1135451
milestone39.0a1
Bug 1135451 - fillForm() cleanup part D: Use early returns to un-nest logic and make it more obvious that selectedLogin is always set. r=MattN
toolkit/components/passwordmgr/LoginManagerContent.jsm
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -688,64 +688,67 @@ var LoginManagerContent = {
         (this._isAutocompleteDisabled(form) ||
          this._isAutocompleteDisabled(usernameField) ||
          this._isAutocompleteDisabled(passwordField))) {
 
       isFormDisabled = true;
       log("form not filled, has autocomplete=off");
     }
 
-    // Variable such that we reduce code duplication and can be sure we
-    // should be firing notifications if and only if we can fill the form.
-    var selectedLogin = null;
-
+    // Select a login to use for filling in the form.
+    var selectedLogin;
     if (usernameField && (usernameField.value || usernameField.disabled || usernameField.readOnly)) {
       // If username was specified in the field, it's disabled or it's readOnly, only fill in the
       // password if we find a matching login.
       var username = usernameField.value.toLowerCase();
 
       let matchingLogins = logins.filter(function(l)
                                          l.username.toLowerCase() == username);
-      if (matchingLogins.length) {
-        // If there are multiple, and one matches case, use it
-        for (let l of matchingLogins) {
-          if (l.username == usernameField.value) {
-            selectedLogin = l;
-          }
-        }
-        // Otherwise just use the first
-        if (!selectedLogin) {
-          selectedLogin = matchingLogins[0];
-        }
-      } else {
+      if (matchingLogins.length == 0) {
         log("Password not filled. None of the stored logins match the username already present.");
         recordAutofillResult(AUTOFILL_RESULT.EXISTING_USERNAME);
+        return;
+      }
+
+      // If there are multiple, and one matches case, use it
+      for (let l of matchingLogins) {
+        if (l.username == usernameField.value) {
+          selectedLogin = l;
+        }
+      }
+      // Otherwise just use the first
+      if (!selectedLogin) {
+        selectedLogin = matchingLogins[0];
       }
     } else if (logins.length == 1) {
       selectedLogin = logins[0];
     } else {
       // We have multiple logins. Handle a special case here, for sites
       // which have a normal user+pass login *and* a password-only login
       // (eg, a PIN). Prefer the login that matches the type of the form
       // (user+pass or pass-only) when there's exactly one that matches.
       let matchingLogins;
       if (usernameField)
         matchingLogins = logins.filter(function(l) l.username);
       else
         matchingLogins = logins.filter(function(l) !l.username);
-      if (matchingLogins.length == 1) {
-        selectedLogin = matchingLogins[0];
-      } else {
+
+      if (matchingLogins.length != 1) {
         log("Multiple logins for form, so not filling any.");
         recordAutofillResult(AUTOFILL_RESULT.MULTIPLE_LOGINS);
+        return;
       }
+
+      selectedLogin = matchingLogins[0];
     }
 
+    // We will always have a selectedLogin at this point.
+
     var didFillForm = false;
-    if (selectedLogin && autofillForm && !isFormDisabled) {
+    if (autofillForm && !isFormDisabled) {
       // Fill the form
 
       if (usernameField) {
         // Don't modify the username field if it's disabled or readOnly so we preserve its case.
         let disabledOrReadOnly = usernameField.disabled || usernameField.readOnly;
 
         let userNameDiffers = selectedLogin.username != usernameField.value;
         // Don't replace the username if it differs only in case, and the user triggered
@@ -757,20 +760,20 @@ var LoginManagerContent = {
         if (!disabledOrReadOnly && !userEnteredDifferentCase && userNameDiffers) {
           usernameField.setUserInput(selectedLogin.username);
         }
       }
       if (passwordField.value != selectedLogin.password) {
         passwordField.setUserInput(selectedLogin.password);
       }
       didFillForm = true;
-    } else if (selectedLogin && !autofillForm) {
+    } else if (!autofillForm) {
       log("autofillForms=false but form can be filled; notified observers");
       recordAutofillResult(AUTOFILL_RESULT.NO_AUTOFILL_FORMS);
-    } else if (selectedLogin && isFormDisabled) {
+    } else if (isFormDisabled) {
       log("autocomplete=off but form can be filled; notified observers");
       recordAutofillResult(AUTOFILL_RESULT.AUTOCOMPLETE_OFF);
     }
 
     if (didFillForm) {
       recordAutofillResult(AUTOFILL_RESULT.FILLED);
     }
   },