Bug 1548381 - Simplify login autocomplete footer result to avoid JSON. r=sfoster
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Tue, 21 May 2019 00:24:26 +0000
changeset 474682 c5b9ba215f498e7e9b00e455df27c426dc1da4a0
parent 474681 e79711ad9ef56e0ede123fd455ed444980d173bc
child 474683 1433d315b1b74f64a560349c1a06a863b4083566
push id85933
push usermozilla@noorenberghe.ca
push dateTue, 21 May 2019 05:19:43 +0000
treeherderautoland@b74e5737da64 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssfoster
bugs1548381
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1548381 - Simplify login autocomplete footer result to avoid JSON. r=sfoster We don't need to use JSON since we now support getCommentAt for extra data. Also add unit tests that are missing. Differential Revision: https://phabricator.services.mozilla.com/D31208
toolkit/components/passwordmgr/LoginAutoCompleteResult.jsm
toolkit/components/passwordmgr/test/mochitest/pwmgr_common.js
toolkit/components/passwordmgr/test/unit/test_login_autocomplete_result.js
toolkit/content/widgets/autocomplete-richlistitem.js
--- a/toolkit/components/passwordmgr/LoginAutoCompleteResult.jsm
+++ b/toolkit/components/passwordmgr/LoginAutoCompleteResult.jsm
@@ -137,22 +137,20 @@ class LoginAutocompleteItem extends Auto
       Services.logins.removeLogin(this._login);
     }
   }
 }
 
 class LoginsFooterAutocompleteItem extends AutocompleteItem {
   constructor(hostname) {
     super("loginsFooter");
+    this.comment = hostname;
 
     XPCOMUtils.defineLazyGetter(this, "label", () => {
-      return JSON.stringify({
-        label: getLocalizedString("viewSavedLogins.label"),
-        hostname,
-      });
+      return getLocalizedString("viewSavedLogins.label");
     });
   }
 }
 
 
 // nsIAutoCompleteResult implementation
 function LoginAutoCompleteResult(aSearchString, matchingLogins, {
   isSecure,
--- a/toolkit/components/passwordmgr/test/mochitest/pwmgr_common.js
+++ b/toolkit/components/passwordmgr/test/mochitest/pwmgr_common.js
@@ -35,26 +35,25 @@ function $_(formNum, name) {
     return null;
   }
 
   return element;
 }
 
 /**
  * Check autocomplete popup results to ensure that expected
- * values are being shown correctly as items in the popup.
+ * *labels* are being shown correctly as items in the popup.
  */
 function checkAutoCompleteResults(actualValues, expectedValues, hostname, msg) {
   if (hostname !== null) {
     isnot(actualValues.length, 0, "There should be items in the autocomplete popup: " + JSON.stringify(actualValues));
 
     // Check the footer first.
     let footerResult = actualValues[actualValues.length - 1];
-    ok(footerResult.includes("View Saved Logins"), "the footer text is shown correctly");
-    ok(footerResult.includes(hostname), "the footer has the correct hostname attribute");
+    is(footerResult, "View Saved Logins", "the footer text is shown correctly");
   }
 
   if (hostname === null) {
     checkArrayValues(actualValues, expectedValues, msg);
     return;
   }
 
   if (actualValues.length == 1) {
--- a/toolkit/components/passwordmgr/test/unit/test_login_autocomplete_result.js
+++ b/toolkit/components/passwordmgr/test/unit/test_login_autocomplete_result.js
@@ -49,16 +49,36 @@ let expectedResults = [
     }, {
       value: "testuser3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzuser4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
+    }],
+  },
+  {
+    insecureFieldWarningEnabled: true,
+    insecureAutoFillFormsEnabled: true,
+    isSecure: false,
+    isPasswordField: false,
+    matchingLogins: [],
+    items: [{
+      value: "",
+      label: "This connection is not secure. Logins entered here could be compromised. Learn More",
+      style: "insecureWarning",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: true,
     isSecure: false,
     isPasswordField: false,
     matchingLogins,
@@ -81,16 +101,20 @@ let expectedResults = [
     }, {
       value: "testuser3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzuser4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: true,
     isSecure: true,
     isPasswordField: true,
     matchingLogins,
@@ -109,16 +133,20 @@ let expectedResults = [
     }, {
       value: "testpass3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzpass4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: true,
     isSecure: false,
     isPasswordField: true,
     matchingLogins,
@@ -141,16 +169,20 @@ let expectedResults = [
     }, {
       value: "testpass3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzpass4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: false,
     insecureAutoFillFormsEnabled: true,
     isSecure: true,
     isPasswordField: false,
     matchingLogins,
@@ -169,16 +201,20 @@ let expectedResults = [
     }, {
       value: "testuser3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzuser4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: false,
     insecureAutoFillFormsEnabled: true,
     isSecure: false,
     isPasswordField: false,
     matchingLogins,
@@ -197,16 +233,20 @@ let expectedResults = [
     }, {
       value: "testuser3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzuser4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: false,
     insecureAutoFillFormsEnabled: true,
     isSecure: true,
     isPasswordField: true,
     matchingLogins,
@@ -225,16 +265,20 @@ let expectedResults = [
     }, {
       value: "testpass3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzpass4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: false,
     insecureAutoFillFormsEnabled: true,
     isSecure: false,
     isPasswordField: true,
     matchingLogins,
@@ -253,16 +297,20 @@ let expectedResults = [
     }, {
       value: "testpass3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzpass4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: false,
     isSecure: true,
     isPasswordField: false,
     matchingLogins,
@@ -281,16 +329,20 @@ let expectedResults = [
     }, {
       value: "testuser3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzuser4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: false,
     isSecure: false,
     isPasswordField: false,
     matchingLogins,
@@ -313,16 +365,20 @@ let expectedResults = [
     }, {
       value: "testuser3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzuser4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: false,
     isSecure: true,
     isPasswordField: true,
     matchingLogins,
@@ -341,16 +397,20 @@ let expectedResults = [
     }, {
       value: "testpass3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzpass4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: false,
     isSecure: false,
     isPasswordField: true,
     matchingLogins,
@@ -373,16 +433,20 @@ let expectedResults = [
     }, {
       value: "testpass3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzpass4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: false,
     insecureAutoFillFormsEnabled: false,
     isSecure: true,
     isPasswordField: false,
     matchingLogins,
@@ -401,25 +465,65 @@ let expectedResults = [
     }, {
       value: "testuser3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzuser4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
+    }],
+  },
+  {
+    insecureFieldWarningEnabled: false,
+    insecureAutoFillFormsEnabled: false,
+    isSecure: false,
+    isPasswordField: false,
+    matchingLogins: [],
+    items: [{
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: false,
     insecureAutoFillFormsEnabled: false,
     isSecure: false,
     isPasswordField: false,
     matchingLogins,
-    items: [],
+    items: [{
+      value: "",
+      label: LABEL_NO_USERNAME,
+      style: "loginWithOrigin",
+    }, {
+      value: "tempuser1",
+      label: "tempuser1",
+      style: "loginWithOrigin",
+    }, {
+      value: "testuser2",
+      label: "testuser2",
+      style: "loginWithOrigin",
+    }, {
+      value: "testuser3",
+      label: "testuser3",
+      style: "loginWithOrigin",
+    }, {
+      value: "zzzuser4",
+      label: "zzzuser4",
+      style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
+    }],
   },
   {
     insecureFieldWarningEnabled: false,
     insecureAutoFillFormsEnabled: false,
     isSecure: true,
     isPasswordField: true,
     matchingLogins,
     items: [{
@@ -437,45 +541,76 @@ let expectedResults = [
     }, {
       value: "testpass3",
       label: "testuser3",
       style: "loginWithOrigin",
     }, {
       value: "zzzpass4",
       label: "zzzuser4",
       style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
     }],
   },
   {
     insecureFieldWarningEnabled: false,
     insecureAutoFillFormsEnabled: false,
     isSecure: false,
     isPasswordField: true,
     matchingLogins,
-    items: [],
+    items: [{
+      value: "emptypass1",
+      label: LABEL_NO_USERNAME,
+      style: "loginWithOrigin",
+    }, {
+      value: "temppass1",
+      label: "tempuser1",
+      style: "loginWithOrigin",
+    }, {
+      value: "testpass2",
+      label: "testuser2",
+      style: "loginWithOrigin",
+    }, {
+      value: "testpass3",
+      label: "testuser3",
+      style: "loginWithOrigin",
+    }, {
+      value: "zzzpass4",
+      label: "zzzuser4",
+      style: "loginWithOrigin",
+    }, {
+      value: "",
+      label: "View Saved Logins",
+      style: "loginsFooter",
+    }],
   },
 ];
 
 add_task(async function test_all_patterns() {
   LoginHelper.createLogger("LoginAutoCompleteResult");
+  Services.prefs.setBoolPref("signon.showAutoCompleteFooter", true);
   Services.prefs.setBoolPref("signon.showAutoCompleteOrigins", true);
 
   expectedResults.forEach(pattern => {
+    info(JSON.stringify(pattern, null, 2));
     Services.prefs.setBoolPref(PREF_INSECURE_FIELD_WARNING_ENABLED,
                                pattern.insecureFieldWarningEnabled);
     Services.prefs.setBoolPref(PREF_INSECURE_AUTOFILLFORMS_ENABLED,
                                pattern.insecureAutoFillFormsEnabled);
     let actual = new LoginAutoCompleteResult("", pattern.matchingLogins, {
       isSecure: pattern.isSecure,
       isPasswordField: pattern.isPasswordField,
     });
+    equal(actual.matchCount, pattern.items.length, "Check matching row count");
     pattern.items.forEach((item, index) => {
-      equal(actual.getValueAt(index), item.value);
-      equal(actual.getLabelAt(index), item.label);
-      equal(actual.getStyleAt(index), item.style);
+      equal(actual.getValueAt(index), item.value, `Value ${index}`);
+      equal(actual.getLabelAt(index), item.label, `Label ${index}`);
+      equal(actual.getStyleAt(index), item.style, `Style ${index}`);
     });
 
     if (pattern.items.length != 0) {
       Assert.throws(() => actual.getValueAt(pattern.items.length),
                     /Index out of range\./);
 
       Assert.throws(() => actual.getLabelAt(pattern.items.length),
                     /Index out of range\./);
--- a/toolkit/content/widgets/autocomplete-richlistitem.js
+++ b/toolkit/content/widgets/autocomplete-richlistitem.js
@@ -983,32 +983,27 @@ class MozAutocompleteRichlistitemLoginsF
   constructor() {
     super();
 
     function handleEvent(event) {
       if (event.button != 0) {
         return;
       }
 
+      // ac-label gets populated from getCommentAt despite the attribute name.
+      // The "comment" is used to populate additional visible text.
+      let formHostname = this.getAttribute("ac-label");
       LoginHelper.openPasswordManager(this.ownerGlobal, {
-        filterString: this._data.hostname,
+        filterString: formHostname,
         entryPoint: "autocomplete",
       });
     }
 
     this.addEventListener("click", handleEvent);
   }
-
-  get _data() {
-    return JSON.parse(this.getAttribute("ac-value"));
-  }
-
-  _adjustAcItem() {
-    this._titleText.textContent = this._data.label;
-  }
 }
 
 class MozAutocompleteRichlistitemLoginWithOrigin extends MozElements.MozRichlistitem {
   connectedCallback() {
     if (this.delayConnectedCallback()) {
       return;
     }