Bug 1383058 - Always adopt the info from autocomplete attribute. r=MattN
☠☠ backed out by 632a4aed2e2c ☠ ☠
authorSean Lee <selee@mozilla.com>
Fri, 21 Jul 2017 22:17:17 +0800
changeset 647746 a315cc6a25bb695567bd6970040dd6d4e628d9b7
parent 647745 a917b05e1a23d169ef134868db369cd263c7f279
child 647747 cc542de6ec53a2ac57e51dc4f41c54db9bb4cc07
push id74539
push userbmo:mkelly@mozilla.com
push dateWed, 16 Aug 2017 22:39:32 +0000
reviewersMattN
bugs1383058
milestone57.0a1
Bug 1383058 - Always adopt the info from autocomplete attribute. r=MattN MozReview-Commit-ID: CdAX9XZX7R0
browser/extensions/formautofill/FormAutofillHandler.jsm
browser/extensions/formautofill/FormAutofillHeuristics.jsm
browser/extensions/formautofill/test/unit/head.js
browser/extensions/formautofill/test/unit/test_collectFormFields.js
--- a/browser/extensions/formautofill/FormAutofillHandler.jsm
+++ b/browser/extensions/formautofill/FormAutofillHandler.jsm
@@ -110,23 +110,26 @@ FormAutofillHandler.prototype = {
     // can be recognized as there is no element changed. However, we should
     // improve the function to detect the element changes. e.g. a tel field
     // is changed from type="hidden" to type="tel".
     return this._formFieldCount != this.form.elements.length;
   },
 
   /**
    * Set fieldDetails from the form about fields that can be autofilled.
-
+   *
+   * @param {boolean} allowDuplicates
+   *        true to remain any duplicated field details otherwise to remove the
+   *        duplicated ones.
    * @returns {Array} The valid address and credit card details.
    */
-  collectFormFields() {
+  collectFormFields(allowDuplicates = false) {
     this._cacheValue.allFieldNames = null;
     this._formFieldCount = this.form.elements.length;
-    let fieldDetails = FormAutofillHeuristics.getFormInfo(this.form);
+    let fieldDetails = FormAutofillHeuristics.getFormInfo(this.form, allowDuplicates);
     this.fieldDetails = fieldDetails ? fieldDetails : [];
     log.debug("Collected details on", this.fieldDetails.length, "fields");
 
     this.address.fieldDetails = this.fieldDetails.filter(
       detail => FormAutofillUtils.isAddressField(detail.fieldName)
     );
     this.creditCard.fieldDetails = this.fieldDetails.filter(
       detail => FormAutofillUtils.isCreditCardField(detail.fieldName)
--- a/browser/extensions/formautofill/FormAutofillHeuristics.jsm
+++ b/browser/extensions/formautofill/FormAutofillHeuristics.jsm
@@ -118,16 +118,20 @@ class FieldScanner {
     let fieldInfo = {
       section: info.section,
       addressType: info.addressType,
       contactType: info.contactType,
       fieldName: info.fieldName,
       elementWeakRef: Cu.getWeakReference(element),
     };
 
+    if (info._reason) {
+      fieldInfo._reason = info._reason;
+    }
+
     // Store the association between the field metadata and the element.
     if (this.findSameField(info) != -1) {
       // A field with the same identifier already exists.
       log.debug("Not collecting a field matching another with the same info:", info);
       fieldInfo._duplicated = true;
     }
 
     this.fieldDetails.push(fieldInfo);
@@ -199,17 +203,17 @@ this.FormAutofillHeuristics = {
     let matchingResult;
 
     const GRAMMARS = this.PHONE_FIELD_GRAMMARS;
     for (let i = 0; i < GRAMMARS.length; i++) {
       let detailStart = fieldScanner.parsingIndex;
       let ruleStart = i;
       for (; i < GRAMMARS.length && GRAMMARS[i][0] && fieldScanner.elementExisting(detailStart); i++, detailStart++) {
         let detail = fieldScanner.getFieldDetailByIndex(detailStart);
-        if (!detail || GRAMMARS[i][0] != detail.fieldName) {
+        if (!detail || GRAMMARS[i][0] != detail.fieldName || detail._reason == "autocomplete") {
           break;
         }
         let element = detail.elementWeakRef.get();
         if (!element) {
           break;
         }
         if (GRAMMARS[i][2] && (!element.maxLength || GRAMMARS[i][2] < element.maxLength)) {
           break;
@@ -283,44 +287,65 @@ this.FormAutofillHeuristics = {
       fieldScanner.updateFieldName(fieldScanner.parsingIndex, addressLines[i]);
       fieldScanner.parsingIndex++;
       parsedFields = true;
     }
 
     return parsedFields;
   },
 
-  getFormInfo(form) {
+  /**
+   * This function should provide all field details of a form. The details
+   * contain the autocomplete info (e.g. fieldName, section, etc).
+   *
+   * `allowDuplicates` is used for the xpcshell-test purpose currently because
+   * the heuristics should be verified that some duplicated elements still can
+   * be predicted correctly.
+   *
+   * @param {HTMLFormElement} form
+   *        the elements in this form to be predicted the field info.
+   * @param {boolean} allowDuplicates
+   *        true to remain any duplicated field details otherwise to remove the
+   *        duplicated ones.
+   * @returns {Array<Object>}
+   *        all field details in the form.
+   */
+  getFormInfo(form, allowDuplicates = false) {
     if (form.autocomplete == "off" || form.elements.length <= 0) {
       return [];
     }
 
     let fieldScanner = new FieldScanner(form.elements);
     while (!fieldScanner.parsingFinished) {
       let parsedPhoneFields = this._parsePhoneFields(fieldScanner);
       let parsedAddressFields = this._parseAddressFields(fieldScanner);
 
       // If there is no any field parsed, the parsing cursor can be moved
       // forward to the next one.
       if (!parsedPhoneFields && !parsedAddressFields) {
         fieldScanner.parsingIndex++;
       }
     }
-    return fieldScanner.trimmedFieldDetail;
+    if (allowDuplicates) {
+      return fieldScanner.fieldDetails;
+    } else {
+      return fieldScanner.trimmedFieldDetail;
+    }
   },
 
   getInfo(element) {
     if (!FormAutofillUtils.isFieldEligibleForAutofill(element)) {
       return null;
     }
 
     let info = element.getAutocompleteInfo();
     // An input[autocomplete="on"] will not be early return here since it stll
     // needs to find the field name.
     if (info && info.fieldName && info.fieldName != "on") {
+      info._reason = "autocomplete";
       return info;
     }
 
     if (!this._prefEnabled) {
       return null;
     }
 
     // "email" type of input is accurate for heuristics to determine its Email
--- a/browser/extensions/formautofill/test/unit/head.js
+++ b/browser/extensions/formautofill/test/unit/head.js
@@ -144,16 +144,17 @@ function runHeuristicsTest(patterns, fix
       Assert.equal(forms.length, testPattern.expectedResult.length, "Expected form count.");
 
       forms.forEach((form, formIndex) => {
         let formInfo = FormAutofillHeuristics.getFormInfo(form);
         do_print("FieldName Prediction Results: " + formInfo.map(i => i.fieldName));
         Assert.equal(formInfo.length, testPattern.expectedResult[formIndex].length, "Expected field count.");
         formInfo.forEach((field, fieldIndex) => {
           let expectedField = testPattern.expectedResult[formIndex][fieldIndex];
+          delete field._reason;
           expectedField.elementWeakRef = field.elementWeakRef;
           Assert.deepEqual(field, expectedField);
         });
       });
     });
   });
 }
 
--- a/browser/extensions/formautofill/test/unit/test_collectFormFields.js
+++ b/browser/extensions/formautofill/test/unit/test_collectFormFields.js
@@ -169,61 +169,62 @@ const TESTCASES = [
                </form>`,
     addressFieldDetails: [],
     creditCardFieldDetails: [],
     validFieldDetails: [],
   },
   {
     description: "Three sets of adjacent phone number fields",
     document: `<form>
-                 <input id="shippingAreaCode" autocomplete="shipping tel" maxlength="3">
-                 <input id="shippingPrefix" autocomplete="shipping tel" maxlength="3">
-                 <input id="shippingSuffix" autocomplete="shipping tel" maxlength="4">
-                 <input id="shippingTelExt" autocomplete="shipping tel-extension">
+                 <input id="shippingAC" name="phone" maxlength="3">
+                 <input id="shippingPrefix" name="phone" maxlength="3">
+                 <input id="shippingSuffix" name="phone" maxlength="4">
+                 <input id="shippingTelExt" name="extension">
 
-                 <input id="billingAreaCode" autocomplete="billing tel" maxlength="3">
-                 <input id="billingPrefix" autocomplete="billing tel" maxlength="3">
-                 <input id="billingSuffix" autocomplete="billing tel" maxlength="4">
+                 <input id="billingAC" name="phone" maxlength="3">
+                 <input id="billingPrefix" name="phone" maxlength="3">
+                 <input id="billingSuffix" name="phone" maxlength="4">
 
-                 <input id="otherCountryCode" autocomplete="tel" maxlength="3">
-                 <input id="otherAreaCode" autocomplete="tel" maxlength="3">
-                 <input id="otherPrefix" autocomplete="tel" maxlength="3">
-                 <input id="otherSuffix" autocomplete="tel" maxlength="4">
+                 <input id="otherCC" name="phone" maxlength="3">
+                 <input id="otherAC" name="phone" maxlength="3">
+                 <input id="otherPrefix" name="phone" maxlength="3">
+                 <input id="otherSuffix" name="phone" maxlength="4">
                </form>`,
+    allowDuplicates: true,
     addressFieldDetails: [
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-area-code"},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-local-prefix"},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-local-suffix"},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-extension"},
-      {"section": "", "addressType": "billing", "contactType": "", "fieldName": "tel-area-code"},
-      {"section": "", "addressType": "billing", "contactType": "", "fieldName": "tel-local-prefix"},
-      {"section": "", "addressType": "billing", "contactType": "", "fieldName": "tel-local-suffix"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-area-code"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-prefix"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-suffix"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-extension"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-area-code"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-prefix"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-suffix"},
       {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-country-code"},
       {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-area-code"},
       {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-prefix"},
       {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-suffix"},
     ],
     creditCardFieldDetails: [],
     validFieldDetails: [
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-area-code"},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-local-prefix"},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-local-suffix"},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-extension"},
-      {"section": "", "addressType": "billing", "contactType": "", "fieldName": "tel-area-code"},
-      {"section": "", "addressType": "billing", "contactType": "", "fieldName": "tel-local-prefix"},
-      {"section": "", "addressType": "billing", "contactType": "", "fieldName": "tel-local-suffix"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-area-code"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-prefix"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-suffix"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-extension"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-area-code"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-prefix"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-suffix"},
       {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-country-code"},
       {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-area-code"},
       {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-prefix"},
       {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-suffix"},
     ],
     ids: [
-      "shippingAreaCode", "shippingPrefix", "shippingSuffix", "shippingTelExt",
-      "billingAreaCode", "billingPrefix", "billingSuffix",
-      "otherCountryCode", "otherAreaCode", "otherPrefix", "otherSuffix",
+      "shippingAC", "shippingPrefix", "shippingSuffix", "shippingTelExt",
+      "billingAC", "billingPrefix", "billingSuffix",
+      "otherCC", "otherAC", "otherPrefix", "otherSuffix",
     ],
   },
   {
     description: "Dedup the same field names of the different telephone fields.",
     document: `<form>
                  <input id="i1" autocomplete="shipping given-name">
                  <input id="i2" autocomplete="shipping family-name">
                  <input id="i3" autocomplete="shipping street-address">
@@ -285,16 +286,38 @@ const TESTCASES = [
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel"},
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-area-code"},
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-local-prefix"},
       {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-local-suffix"},
     ],
     ids: ["i1", "i2", "i3", "i4", "singlePhone",
       "shippingAreaCode", "shippingPrefix", "shippingSuffix"],
   },
+  {
+    description: "Always adopt the info from autocomplete attribute.",
+    document: `<form>
+                 <input id="given-name" autocomplete="shipping given-name">
+                 <input id="family-name" autocomplete="shipping family-name">
+                 <input id="dummyAreaCode" autocomplete="shipping tel" maxlength="3">
+                 <input id="dummyPrefix" autocomplete="shipping tel" maxlength="3">
+                 <input id="dummySuffix" autocomplete="shipping tel" maxlength="4">
+               </form>`,
+    addressFieldDetails: [
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "given-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "family-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel"},
+    ],
+    creditCardFieldDetails: [],
+    validFieldDetails: [
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "given-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "family-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel"},
+    ],
+    ids: ["given-name", "family-name", "dummyAreaCode"],
+  },
 ];
 
 for (let tc of TESTCASES) {
   (function() {
     let testcase = tc;
     add_task(async function() {
       do_print("Starting testcase: " + testcase.description);
 
@@ -335,16 +358,16 @@ for (let tc of TESTCASES) {
       }
       [
         testcase.addressFieldDetails,
         testcase.creditCardFieldDetails,
         testcase.validFieldDetails,
       ].forEach(details => setElementWeakRef(details));
 
       let handler = new FormAutofillHandler(formLike);
-      let validFieldDetails = handler.collectFormFields();
+      let validFieldDetails = handler.collectFormFields(testcase.allowDuplicates);
 
       verifyDetails(handler.address.fieldDetails, testcase.addressFieldDetails);
       verifyDetails(handler.creditCard.fieldDetails, testcase.creditCardFieldDetails);
       verifyDetails(validFieldDetails, testcase.validFieldDetails);
     });
   })();
 }