Bug 1333256 - Bring back the insecure field warning Learn More text in bold. r=johannh draft
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Tue, 14 Feb 2017 02:19:20 +0800
changeset 483067 da03efa9aaaa70a504fb158359f7897c1d4883e3
parent 482430 4ec373fafebf79846cd5fde0561ac02fa0bb9647
child 545543 e67a0179067f74bf0b3369ed0bc9678a9483b0b3
push id45203
push usermozilla@noorenberghe.ca
push dateMon, 13 Feb 2017 18:20:37 +0000
reviewersjohannh
bugs1333256
milestone54.0a1
Bug 1333256 - Bring back the insecure field warning Learn More text in bold. r=johannh MozReview-Commit-ID: CpPgkdmbb2O
toolkit/components/passwordmgr/LoginManagerContent.jsm
toolkit/components/passwordmgr/test/mochitest/test_insecure_form_field_autocomplete.html
toolkit/components/passwordmgr/test/mochitest/test_password_field_autocomplete.html
toolkit/components/passwordmgr/test/unit/test_user_autocomplete_result.js
toolkit/content/widgets/autocomplete.xml
toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
--- a/toolkit/components/passwordmgr/LoginManagerContent.jsm
+++ b/toolkit/components/passwordmgr/LoginManagerContent.jsm
@@ -1473,27 +1473,26 @@ UserAutoCompleteResult.prototype = {
     return this._isPasswordField ? selectedLogin.password : selectedLogin.username;
   },
 
   getLabelAt(index) {
     if (index < 0 || index >= this.matchCount) {
       throw new Error("Index out of range.");
     }
 
-    if (this._showInsecureFieldWarning && index === 0) {
-      return this._stringBundle.GetStringFromName("insecureFieldWarningDescription");
-    }
-
-    let that = this;
+    let getLocalizedString = (key, formatArgs = null) => {
+      if (formatArgs) {
+        return this._stringBundle.formatStringFromName(key, formatArgs, formatArgs.length);
+      }
+      return this._stringBundle.GetStringFromName(key);
+    };
 
-    function getLocalizedString(key, formatArgs) {
-      if (formatArgs) {
-        return that._stringBundle.formatStringFromName(key, formatArgs, formatArgs.length);
-      }
-      return that._stringBundle.GetStringFromName(key);
+    if (this._showInsecureFieldWarning && index === 0) {
+      let learnMoreString = getLocalizedString("insecureFieldWarningLearnMore");
+      return getLocalizedString("insecureFieldWarningDescription3", [learnMoreString]);
     }
 
     let login = this.logins[index - this._showInsecureFieldWarning];
     let username = login.username;
     // If login is empty or duplicated we want to append a modification date to it.
     if (!username || this._duplicateUsernames.has(username)) {
       if (!username) {
         username = getLocalizedString("noUsername");
--- a/toolkit/components/passwordmgr/test/mochitest/test_insecure_form_field_autocomplete.html
+++ b/toolkit/components/passwordmgr/test/mochitest/test_insecure_form_field_autocomplete.html
@@ -228,17 +228,17 @@ add_task(function* test_form1_warning_en
   restoreForm();
   let shownPromise = promiseACShown();
   doKey("down"); // open
   let results = yield shownPromise;
 
   let popupState = yield getPopupState();
   is(popupState.selectedIndex, -1, "Check no entries are selected upon opening");
 
-  let expectedMenuItems = ["This connection is not secure. Logins entered here could be compromised.",
+  let expectedMenuItems = ["Logins entered here could be compromised. Learn More",
                            "tempuser1",
                            "testuser2",
                            "testuser3",
                            "zzzuser4"];
   checkArrayValues(results, expectedMenuItems, "Check all menuitems are displayed correctly.");
 
   doKey("down"); // select insecure warning
   checkACForm("", ""); // value shouldn't update just by selecting
@@ -760,17 +760,17 @@ add_task(function* test_form9_filtering(
   checkACForm("form9userAB", "");
   uname.focus();
   doKey("left");
   shownPromise = promiseACShown();
   sendChar("A");
   let results = yield shownPromise;
 
   checkACForm("form9userAAB", "");
-  checkArrayValues(results, ["This connection is not secure. Logins entered here could be compromised.", "form9userAAB"],
+  checkArrayValues(results, ["Logins entered here could be compromised. Learn More", "form9userAAB"],
                             "Check dropdown is updated after inserting 'A'");
   doKey("down"); // skip insecure warning
   doKey("down");
   doKey("return");
   yield promiseFormsProcessed();
   checkACForm("form9userAAB", "form9pass");
 });
 
--- a/toolkit/components/passwordmgr/test/mochitest/test_password_field_autocomplete.html
+++ b/toolkit/components/passwordmgr/test/mochitest/test_password_field_autocomplete.html
@@ -193,17 +193,17 @@ add_task(function* test_form1_enabledIns
   // Trigger autocomplete popup
   let shownPromise = promiseACShown();
   doKey("down"); // open
   let results = yield shownPromise;
 
   let popupState = yield getPopupState();
   is(popupState.selectedIndex, -1, "Check no entries are selected upon opening");
 
-  let expectedMenuItems = ["This connection is not secure. Logins entered here could be compromised.",
+  let expectedMenuItems = ["Logins entered here could be compromised. Learn More",
                            "No username (" + DATE_NOW_STRING + ")",
                            "tempuser1",
                            "testuser2",
                            "testuser3",
                            "zzzuser4"];
   checkArrayValues(results, expectedMenuItems, "Check all menuitems are displayed correctly.");
 
   doKey("down"); // select insecure warning
@@ -252,17 +252,17 @@ add_task(function* test_form1_enabledIns
   // Trigger autocomplete popup
   let shownPromise = promiseACShown();
   doKey("down"); // open
   let results = yield shownPromise;
 
   let popupState = yield getPopupState();
   is(popupState.selectedIndex, -1, "Check no entries are selected upon opening");
 
-  let expectedMenuItems = ["This connection is not secure. Logins entered here could be compromised.",
+  let expectedMenuItems = ["Logins entered here could be compromised. Learn More",
                            "No username (" + DATE_NOW_STRING + ")",
                            "tempuser1",
                            "testuser2",
                            "testuser3",
                            "zzzuser4"];
   checkArrayValues(results, expectedMenuItems, "Check all menuitems are displayed correctly.");
 
   doKey("down"); // select insecure warning
--- a/toolkit/components/passwordmgr/test/unit/test_user_autocomplete_result.js
+++ b/toolkit/components/passwordmgr/test/unit/test_user_autocomplete_result.js
@@ -61,17 +61,17 @@ let expectedResults = [
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: true,
     isSecure: false,
     isPasswordField: false,
     matchingLogins,
     items: [{
       value: "",
-      label: "This connection is not secure. Logins entered here could be compromised.",
+      label: "Logins entered here could be compromised. Learn More",
       style: "insecureWarning"
     }, {
       value: "",
       label: LABEL_NO_USERNAME,
       style: "login",
     }, {
       value: "tempuser1",
       label: "tempuser1",
@@ -121,17 +121,17 @@ let expectedResults = [
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: true,
     isSecure: false,
     isPasswordField: true,
     matchingLogins,
     items: [{
       value: "",
-      label: "This connection is not secure. Logins entered here could be compromised.",
+      label: "Logins entered here could be compromised. Learn More",
       style: "insecureWarning"
     }, {
       value: "emptypass1",
       label: LABEL_NO_USERNAME,
       style: "login",
     }, {
       value: "temppass1",
       label: "tempuser1",
@@ -293,17 +293,17 @@ let expectedResults = [
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: false,
     isSecure: false,
     isPasswordField: false,
     matchingLogins,
     items: [{
       value: "",
-      label: "This connection is not secure. Logins entered here could be compromised.",
+      label: "Logins entered here could be compromised. Learn More",
       style: "insecureWarning"
     }, {
       value: "",
       label: LABEL_NO_USERNAME,
       style: "login",
     }, {
       value: "tempuser1",
       label: "tempuser1",
@@ -353,17 +353,17 @@ let expectedResults = [
   {
     insecureFieldWarningEnabled: true,
     insecureAutoFillFormsEnabled: false,
     isSecure: false,
     isPasswordField: true,
     matchingLogins,
     items: [{
       value: "",
-      label: "This connection is not secure. Logins entered here could be compromised.",
+      label: "Logins entered here could be compromised. Learn More",
       style: "insecureWarning"
     }, {
       value: "emptypass1",
       label: LABEL_NO_USERNAME,
       style: "login",
     }, {
       value: "temppass1",
       label: "tempuser1",
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -1529,16 +1529,38 @@ extends="chrome://global/content/binding
 
     <implementation>
       <constructor><![CDATA[
         // Unlike other autocomplete items, the height of the insecure warning
         // increases by wrapping. So "forceHandleUnderflow" is for container to
         // recalculate an item's height and width.
         this.classList.add("forceHandleUnderflow");
       ]]></constructor>
+
+      <property name="_learnMoreString">
+        <getter><![CDATA[
+          if (!this.__learnMoreString) {
+            this.__learnMoreString =
+              Services.strings.createBundle("chrome://passwordmgr/locale/passwordmgr.properties").
+              GetStringFromName("insecureFieldWarningLearnMore");
+          }
+          return this.__learnMoreString;
+        ]]></getter>
+      </property>
+
+      <!-- Override _getSearchTokens to have the Learn More text emphasized -->
+      <method name="_getSearchTokens">
+        <parameter name="aSearch"/>
+        <body>
+          <![CDATA[
+            return [this._learnMoreString.toLowerCase()];
+          ]]>
+        </body>
+      </method>
+
     </implementation>
   </binding>
 
   <binding id="autocomplete-richlistitem" extends="chrome://global/content/bindings/richlistbox.xml#richlistitem">
 
     <content align="center"
              onoverflow="this._onOverflow();"
              onunderflow="this._onUnderflow();">
--- a/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
+++ b/toolkit/locales/en-US/chrome/passwordmgr/passwordmgr.properties
@@ -66,22 +66,21 @@ loginsDescriptionFiltered=The following 
 # 1st string is the username for the login, 2nd is the login's age.
 loginHostAge=%1$S (%2$S)
 # LOCALIZATION NOTE (noUsername):
 # String is used on the context menu when a login doesn't have a username.
 noUsername=No username
 duplicateLoginTitle=Login already exists
 duplicateLogin=A duplicate login already exists.
 
-insecureFieldWarningDescription = This connection is not secure. Logins entered here could be compromised.
 # LOCALIZATION NOTE (insecureFieldWarningDescription2, insecureFieldWarningDescription3):
 # %1$S will contain insecureFieldWarningLearnMore and look like a link to indicate that clicking will open a tab with support information.
 insecureFieldWarningDescription2 = This connection is not secure. Logins entered here could be compromised. %1$S
 insecureFieldWarningDescription3 = Logins entered here could be compromised. %1$S
-insecureFieldWarningLearnMore = Learn More
+insecureFieldWarningLearnMore = Learn More
 
 # LOCALIZATION NOTE (removeAll, removeAllShown):
 # When only partial sites are shown as a result of keyword search,
 # removeAllShown is displayed as button label.
 # removeAll is displayed when no keyword search and all sites are shown.
 removeAll.label=Remove All
 removeAllShown.label=Remove All Shown
 removeAll.accesskey=A