Bug 1548381 - Simplify login autocomplete footer result to avoid JSON. r=sfoster
☠☠ backed out by 2690e619a493 ☠ ☠
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Mon, 20 May 2019 19:55:34 +0000
changeset 474638 1e2300b95a59dfa70594e01e6b34716141e22105
parent 474637 e0cf735bdcf5235d4e2e7416d5a07956e7e74671
child 474639 60ff6e363acf84e6c77e0f10fa47acced7b7968e
push id85915
push usermozilla@noorenberghe.ca
push dateMon, 20 May 2019 23:09:47 +0000
treeherderautoland@0e7d8f96bf12 [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;
     }