Bug 1390433 - (From 1380910)Enhance form autofill note by showing the categories intersection of result fields and form fields instead of their union. draft
authorRay Lin <ralin@mozilla.com>
Fri, 11 Aug 2017 10:23:49 +0800
changeset 649807 ef55362dc916b05aa3ed83418577668a3d091e23
parent 649806 8180c096add88b50e4973e2fe1b66008b6e2f132
child 649808 3664cc4368b6d6f0127225b56a1f99bdca26f6ed
push id75162
push userschung@mozilla.com
push dateMon, 21 Aug 2017 10:29:17 +0000
bugs1390433, 1380910
milestone56.0
Bug 1390433 - (From 1380910)Enhance form autofill note by showing the categories intersection of result fields and form fields instead of their union. MozReview-Commit-ID: AeCszHgfSS5
browser/extensions/formautofill/ProfileAutoCompleteResult.jsm
browser/extensions/formautofill/content/formautofill.xml
browser/extensions/formautofill/test/browser/browser_autocomplete_footer.js
browser/extensions/formautofill/test/browser/head.js
--- a/browser/extensions/formautofill/ProfileAutoCompleteResult.jsm
+++ b/browser/extensions/formautofill/ProfileAutoCompleteResult.jsm
@@ -29,26 +29,32 @@ class ProfileAutoCompleteResult {
 
     // nsISupports
     this.QueryInterface = XPCOMUtils.generateQI([Ci.nsIAutoCompleteResult]);
 
     // The user's query string
     this.searchString = searchString;
     // The field name of the focused input.
     this._focusedFieldName = focusedFieldName;
-    // All field names in the form which contains the focused input.
-    this._allFieldNames = allFieldNames;
     // The matching profiles contains the information for filling forms.
     this._matchingProfiles = matchingProfiles;
     // The default item that should be entered if none is selected
     this.defaultIndex = 0;
     // The reason the search failed
     this.errorDescription = "";
     // The value used to determine whether the form is secure or not.
     this._isSecure = isSecure;
+    // All fillable field names in the form including the field name of the currently-focused input.
+    this._allFieldNames = [...this._matchingProfiles.reduce((fieldSet, curProfile) => {
+      for (let field of Object.keys(curProfile)) {
+        fieldSet.add(field);
+      }
+
+      return fieldSet;
+    }, new Set())].filter(field => allFieldNames.includes(field));
 
     // The result code of this result object.
     if (resultCode) {
       this.searchResult = resultCode;
     } else if (matchingProfiles.length > 0) {
       this.searchResult = Ci.nsIAutoCompleteResult.RESULT_SUCCESS;
     } else {
       this.searchResult = Ci.nsIAutoCompleteResult.RESULT_NOMATCH;
--- a/browser/extensions/formautofill/content/formautofill.xml
+++ b/browser/extensions/formautofill/content/formautofill.xml
@@ -169,78 +169,62 @@
           this._optionButton = document.getAnonymousElementByAttribute(
             this, "anonid", "autofill-option-button"
           );
           this._warningTextBox = document.getAnonymousElementByAttribute(
             this, "anonid", "autofill-warning"
           );
 
           /**
-           * Update the text on the footer.
+           * A handler for updating warning message once selectedIndex has been changed.
+           *
+           * There're three different states of warning message:
+           * 1. None of addresses were selected: We show all the categories intersection of fields in the
+           *    form and fields in the results.
+           * 2. An address was selested: Show the additional categories that will also be filled.
+           * 3. An address was selected, but the focused category is the same as the only one category: Only show
+           * the exact category that we're going to fill in.
            *
            * @private
-           * @param {string|string[]} categories
-           *        A list of categories that used to generate the message.
-           * @param {boolean} hasExtraCategories
-           *        Used to determine if it has the extra categories other than the focued category. If
-           *        the value is true, we show "Also fill ...", otherwise, show "Fill ..." only.
+           * @param {string[]} data.categories
+           *        The categories of all the fields contained in the selected address.
            */
           const namespace = "category.";
-          this._updateText = (categories = this._allFieldCategories, hasExtraCategories = true) => {
-            let warningTextTmplKey = hasExtraCategories ? "phishingWarningMessage" : "phishingWarningMessage2";
-            let sep = this._stringBundle.GetStringFromName("fieldNameSeparator");
+          this._updateWarningNote = ({data} = {}) => {
+            let categories = (data && data.categories) ? data.categories : this._allFieldCategories;
+            // If the length of categories is 1, that means all the fillable fields are in the same
+            // category. We will change the way to inform user according to this flag. When the value
+            // is true, we show "Also autofills ...", otherwise, show "Autofills ..." only.
+            let hasExtraCategories = categories.length > 1;
             // Show the categories in certain order to conform with the spec.
             let orderedCategoryList = ["address", "name", "organization", "tel", "email"];
             let showCategories = hasExtraCategories ?
               orderedCategoryList.filter(category => categories.includes(category) && category != this._focusedCategory) :
               [this._focusedCategory];
-            let categoriesText = showCategories.map(category => this._stringBundle.GetStringFromName(namespace + category)).join(sep);
+
+            let separator = this._stringBundle.GetStringFromName("fieldNameSeparator");
+            let warningTextTmplKey = hasExtraCategories ? "phishingWarningMessage" : "phishingWarningMessage2";
+            let categoriesText = showCategories.map(category => this._stringBundle.GetStringFromName(namespace + category)).join(separator);
 
             this._warningTextBox.textContent = this._stringBundle.formatStringFromName(warningTextTmplKey,
               [categoriesText], 1);
             this.parentNode.parentNode.adjustHeight();
           };
 
-          /**
-           * A handler for updating warning message once selectedIndex has been changed.
-           *
-           * There're three different states of warning message:
-           * 1. None of addresses were selected: We show all the categories in the form.
-           * 2. An address was selested: Show the additional categories that will also be filled.
-           * 3. An address was selected, but the focused category is the same as the only all categories: Only show
-           * the exact category that we're going to fill in.
-           *
-           * @private
-           * @param {string[]} categories
-           *        The categories of all the fields contained in the selected address.
-           */
-          this._updateWarningMsgHandler = ({data: {categories}} = {data: {}}) => {
-            let hasSelectedAddress = this._focusedCategory && categories;
-            // If the length of categories is 1, that means all the fillable fields are in the same
-            // category. We will change the way to inform user according to this flag.
-            let hasExtraCategories = hasSelectedAddress && categories.length > 1;
-            if (!hasSelectedAddress) {
-              this._updateText();
-              return;
-            }
-
-            this._updateText(categories, hasExtraCategories);
-          };
-
           this._adjustAcItem();
         ]]>
       </constructor>
 
       <method name="_onCollapse">
         <body>
         <![CDATA[
           /* global messageManager */
 
           if (this.showWarningText) {
-            messageManager.removeMessageListener("FormAutofill:UpdateWarningMessage", this._updateWarningMsgHandler);
+            messageManager.removeMessageListener("FormAutofill:UpdateWarningMessage", this._updateWarningNote);
           }
 
           this._itemBox.removeAttribute("no-warning");
         ]]>
         </body>
       </method>
 
       <method name="_adjustAcItem">
@@ -263,19 +247,19 @@
 
           let value = JSON.parse(this.getAttribute("ac-value"));
 
           this._allFieldCategories = value.categories;
           this._focusedCategory = value.focusedCategory;
           this.showWarningText = this._allFieldCategories && this._focusedCategory;
 
           if (this.showWarningText) {
-            messageManager.addMessageListener("FormAutofill:UpdateWarningMessage", this._updateWarningMsgHandler);
+            messageManager.addMessageListener("FormAutofill:UpdateWarningMessage", this._updateWarningNote);
 
-            this._updateText();
+            this._updateWarningNote();
           } else {
             this._itemBox.setAttribute("no-warning", "true");
           }
         ]]>
         </body>
       </method>
     </implementation>
   </binding>
--- a/browser/extensions/formautofill/test/browser/browser_autocomplete_footer.js
+++ b/browser/extensions/formautofill/test/browser/browser_autocomplete_footer.js
@@ -9,19 +9,20 @@ async function expectWarningText(browser
 
   await BrowserTestUtils.waitForCondition(() => {
     return warningBox.textContent == expectedText;
   }, `Waiting for expected warning text: ${expectedText}, Got ${warningBox.textContent}`);
   ok(true, `Got expected warning text: ${expectedText}`);
 }
 
 add_task(async function setup_storage() {
-  await saveAddress(TEST_ADDRESS_1);
   await saveAddress(TEST_ADDRESS_2);
   await saveAddress(TEST_ADDRESS_3);
+  await saveAddress(TEST_ADDRESS_4);
+  await saveAddress(TEST_ADDRESS_5);
 });
 
 add_task(async function test_click_on_footer() {
   await BrowserTestUtils.withNewTab({gBrowser, url: URL}, async function(browser) {
     const {autoCompletePopup: {richlistbox: itemsBox}} = browser;
 
     await openPopupOn(browser, "#organization");
     // Click on the footer
@@ -49,27 +50,38 @@ add_task(async function test_press_enter
     await BrowserTestUtils.synthesizeKey("VK_RETURN", {}, browser);
     await BrowserTestUtils.removeTab(await prefTabPromise);
     ok(true, "Tab: preferences#privacy was successfully opened by pressing enter on the footer");
 
     await closePopup(browser);
   });
 });
 
-add_task(async function test_phishing_warning() {
+add_task(async function test_phishing_warning_single_category() {
   await BrowserTestUtils.withNewTab({gBrowser, url: URL}, async function(browser) {
     const {autoCompletePopup: {richlistbox: itemsBox}} = browser;
 
-    await openPopupOn(browser, "#street-address");
+    await openPopupOn(browser, "#tel");
     const warningBox = itemsBox.querySelector(".autocomplete-richlistitem:last-child")._warningTextBox;
     ok(warningBox, "Got phishing warning box");
-    await expectWarningText(browser, "Also autofills company, phone, email");
-    await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
-    await expectWarningText(browser, "Also autofills company, phone, email");
+
+    await expectWarningText(browser, "Autofills phone");
+
+    await closePopup(browser);
+  });
+});
+
+add_task(async function test_phishing_warning_complex_categories() {
+  await BrowserTestUtils.withNewTab({gBrowser, url: URL}, async function(browser) {
+    await openPopupOn(browser, "#street-address");
+
+    await expectWarningText(browser, "Also autofills company, email");
     await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
     await expectWarningText(browser, "Autofills address");
     await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
     await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
-    await expectWarningText(browser, "Also autofills company, phone, email");
+    await expectWarningText(browser, "Also autofills company, email");
+    await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
+    await expectWarningText(browser, "Also autofills company, email");
 
     await closePopup(browser);
   });
 });
--- a/browser/extensions/formautofill/test/browser/head.js
+++ b/browser/extensions/formautofill/test/browser/head.js
@@ -1,10 +1,10 @@
 /* exported MANAGE_ADDRESSES_DIALOG_URL, EDIT_ADDRESS_DIALOG_URL, BASE_URL,
-            TEST_ADDRESS_1, TEST_ADDRESS_2, TEST_ADDRESS_3,
+            TEST_ADDRESS_1, TEST_ADDRESS_2, TEST_ADDRESS_3, TEST_ADDRESS_4, TEST_ADDRESS_5,
             TEST_CREDIT_CARD_1, TEST_CREDIT_CARD_2, TEST_CREDIT_CARD_3,
             sleep, expectPopupOpen, openPopupOn, expectPopupClose, closePopup, clickDoorhangerButton,
             getAddresses, saveAddress, removeAddresses, saveCreditCard,
             getDisplayedPopupItems */
 
 "use strict";
 
 const MANAGE_ADDRESSES_DIALOG_URL = "chrome://formautofill/content/manageAddresses.xhtml";
@@ -30,16 +30,29 @@ const TEST_ADDRESS_2 = {
   country: "US",
 };
 
 const TEST_ADDRESS_3 = {
   "street-address": "Other Address",
   "postal-code": "12345",
 };
 
+const TEST_ADDRESS_4 = {
+  "given-name": "Timothy",
+  "family-name": "Berners-Lee",
+  organization: "World Wide Web Consortium",
+  "street-address": "32 Vassar Street\nMIT Room 32-G524",
+  country: "US",
+  email: "timbl@w3.org",
+};
+
+const TEST_ADDRESS_5 = {
+  tel: "+16172535702",
+};
+
 const TEST_CREDIT_CARD_1 = {
   "cc-name": "John Doe",
   "cc-number": "1234567812345678",
   // "cc-number-encrypted": "",
   "cc-exp-month": 4,
   "cc-exp-year": 2017,
 };