Bug 1401411 - All name related fields should be counted as 1 field only when creating the records. r=lchang draft
authorSean Lee <selee@mozilla.com>
Mon, 25 Sep 2017 17:14:48 +0800
changeset 672478 67dce6aff754b286e7ed5a875a85027b247e1fc4
parent 672296 e6c32278f32cd5f7d159627b2157396b62d0c4a9
child 733817 4e1a38e3a6262fd75bc6e85b2198d40f01be9073
push id82262
push userbmo:selee@mozilla.com
push dateFri, 29 Sep 2017 09:32:53 +0000
reviewerslchang
bugs1401411
milestone58.0a1
Bug 1401411 - All name related fields should be counted as 1 field only when creating the records. r=lchang MozReview-Commit-ID: 468B9tlFH3p
browser/extensions/formautofill/FormAutofillHandler.jsm
browser/extensions/formautofill/test/unit/test_createRecords.js
--- a/browser/extensions/formautofill/FormAutofillHandler.jsm
+++ b/browser/extensions/formautofill/FormAutofillHandler.jsm
@@ -511,16 +511,36 @@ FormAutofillHandler.prototype = {
       } else {
         this.winUtils.removeManuallyManagedState(element, mmStateValue);
       }
     }
 
     fieldDetail.state = nextState;
   },
 
+  _isAddressRecordCreatable(record) {
+    let hasName = 0;
+    let length = 0;
+    for (let key of Object.keys(record)) {
+      if (!record[key]) {
+        continue;
+      }
+      if (FormAutofillUtils.getCategoryFromFieldName(key) == "name") {
+        hasName = 1;
+        continue;
+      }
+      length++;
+    }
+    return (length + hasName) >= FormAutofillUtils.AUTOFILL_FIELDS_THRESHOLD;
+  },
+
+  _isCreditCardRecordCreatable(record) {
+    return record["cc-number"] && FormAutofillUtils.isCCNumber(record["cc-number"]);
+  },
+
   /**
    * Return the records that is converted from address/creditCard fieldDetails and
    * only valid form records are included.
    *
    * @returns {Object}
    *          Consists of two record objects: address, creditCard. Each one can
    *          be omitted if there's no valid fields. A record object consists of
    *          three properties:
@@ -576,27 +596,24 @@ FormAutofillHandler.prototype = {
         if (detail.state == "AUTO_FILLED") {
           data[type].untouchedFields.push(detail.fieldName);
         }
       });
     });
 
     this._normalizeAddress(data.address);
 
-    if (data.address &&
-        Object.values(data.address.record).filter(v => v).length <
-        FormAutofillUtils.AUTOFILL_FIELDS_THRESHOLD) {
+    if (data.address && !this._isAddressRecordCreatable(data.address.record)) {
       log.debug("No address record saving since there are only",
                      Object.keys(data.address.record).length,
                      "usable fields");
       delete data.address;
     }
 
-    if (data.creditCard && (!data.creditCard.record["cc-number"] ||
-        !FormAutofillUtils.isCCNumber(data.creditCard.record["cc-number"]))) {
+    if (data.creditCard && !this._isCreditCardRecordCreatable(data.creditCard.record)) {
       log.debug("No credit card record saving since card number is invalid");
       delete data.creditCard;
     }
 
     // If both address and credit card exists, skip this metrics because it not a
     // general case and each specific histogram might contains insufficient data set.
     if (data.address && data.creditCard) {
       this.timeStartedFillingMS = null;
--- a/browser/extensions/formautofill/test/unit/test_createRecords.js
+++ b/browser/extensions/formautofill/test/unit/test_createRecords.js
@@ -21,143 +21,241 @@ const TESTCASES = [
     expectedRecord: {
       address: undefined,
     },
   },
   {
     description: "\"country\" using @autocomplete shouldn't be identified aggressively",
     document: `<form>
                 <input id="given-name" autocomplete="given-name">
-                <input id="family-name" autocomplete="family-name">
+                <input id="organization" autocomplete="organization">
                 <input id="country" autocomplete="country">
                </form>`,
     formValue: {
       "given-name": "John",
-      "family-name": "Doe",
-      country: "United States",
+      "organization": "Mozilla",
+      "country": "United States",
     },
     expectedRecord: {
       address: {
         "given-name": "John",
-        "family-name": "Doe",
-        country: "United States",
+        "organization": "Mozilla",
+        "country": "United States",
       },
     },
   },
   {
     description: "\"country\" using heuristics should be identified aggressively",
     document: `<form>
                 <input id="given-name" autocomplete="given-name">
-                <input id="family-name" autocomplete="family-name">
+                <input id="organization" autocomplete="organization">
                 <input id="country" name="country">
                </form>`,
     formValue: {
       "given-name": "John",
-      "family-name": "Doe",
-      country: "United States",
+      "organization": "Mozilla",
+      "country": "United States",
     },
     expectedRecord: {
       address: {
         "given-name": "John",
-        "family-name": "Doe",
-        country: "US",
+        "organization": "Mozilla",
+        "country": "US",
       },
     },
   },
   {
     description: "\"tel\" related fields should be concatenated",
     document: `<form>
                 <input id="given-name" autocomplete="given-name">
-                <input id="family-name" autocomplete="family-name">
+                <input id="organization" autocomplete="organization">
                 <input id="tel-country-code" autocomplete="tel-country-code">
                 <input id="tel-national" autocomplete="tel-national">
                </form>`,
     formValue: {
       "given-name": "John",
-      "family-name": "Doe",
+      "organization": "Mozilla",
       "tel-country-code": "+1",
       "tel-national": "1234567890",
     },
     expectedRecord: {
       address: {
         "given-name": "John",
-        "family-name": "Doe",
+        "organization": "Mozilla",
         "tel": "+11234567890",
       },
     },
   },
   {
     description: "\"tel\" should be removed if it's too short",
     document: `<form>
                 <input id="given-name" autocomplete="given-name">
-                <input id="family-name" autocomplete="family-name">
                 <input id="organization" autocomplete="organization">
+                <input id="country" autocomplete="country">
                 <input id="tel" autocomplete="tel-national">
                </form>`,
     formValue: {
       "given-name": "John",
-      "family-name": "Doe",
       "organization": "Mozilla",
+      "country": "United States",
       "tel": "1234",
     },
     expectedRecord: {
       address: {
         "given-name": "John",
-        "family-name": "Doe",
         "organization": "Mozilla",
+        "country": "United States",
         "tel": "",
       },
     },
   },
   {
     description: "\"tel\" should be removed if it's too long",
     document: `<form>
                 <input id="given-name" autocomplete="given-name">
-                <input id="family-name" autocomplete="family-name">
                 <input id="organization" autocomplete="organization">
+                <input id="country" autocomplete="country">
                 <input id="tel" autocomplete="tel-national">
                </form>`,
     formValue: {
       "given-name": "John",
-      "family-name": "Doe",
       "organization": "Mozilla",
+      "country": "United States",
       "tel": "1234567890123456",
     },
     expectedRecord: {
       address: {
         "given-name": "John",
-        "family-name": "Doe",
         "organization": "Mozilla",
+        "country": "United States",
         "tel": "",
       },
     },
   },
   {
     description: "\"tel\" should be removed if it contains invalid characters",
     document: `<form>
                 <input id="given-name" autocomplete="given-name">
+                <input id="organization" autocomplete="organization">
+                <input id="country" autocomplete="country">
+                <input id="tel" autocomplete="tel-national">
+               </form>`,
+    formValue: {
+      "given-name": "John",
+      "organization": "Mozilla",
+      "country": "United States",
+      "tel": "12345###!!!",
+    },
+    expectedRecord: {
+      address: {
+        "given-name": "John",
+        "organization": "Mozilla",
+        "country": "United States",
+        "tel": "",
+      },
+    },
+  },
+  {
+    description: "All name related fields should be counted as 1 field only.",
+    document: `<form>
+                <input id="given-name" autocomplete="given-name">
                 <input id="family-name" autocomplete="family-name">
                 <input id="organization" autocomplete="organization">
-                <input id="tel" autocomplete="tel-national">
                </form>`,
     formValue: {
       "given-name": "John",
       "family-name": "Doe",
       "organization": "Mozilla",
-      "tel": "12345###!!!",
+    },
+    expectedRecord: {
+      address: undefined,
+    },
+  },
+  {
+    description: "All telephone related fields should be counted as 1 field only.",
+    document: `<form>
+                <input id="tel-country-code" autocomplete="tel-country-code">
+                <input id="tel-area-code" autocomplete="tel-area-code">
+                <input id="tel-local" autocomplete="tel-local">
+                <input id="organization" autocomplete="organization">
+               </form>`,
+    formValue: {
+      "tel-country-code": "+1",
+      "tel-area-code": "123",
+      "tel-local": "4567890",
+      "organization": "Mozilla",
+    },
+    expectedRecord: {
+      address: undefined,
+    },
+  },
+  {
+    description: "A credit card form with the value of cc-number, cc-exp, and cc-name.",
+    document: `<form>
+                <input id="cc-number" autocomplete="cc-number">
+                <input id="cc-name" autocomplete="cc-name">
+                <input id="cc-exp" autocomplete="cc-exp">
+               </form>`,
+    formValue: {
+      "cc-number": "4444000022220000",
+      "cc-name": "Foo Bar",
+      "cc-exp": "2022-06",
     },
     expectedRecord: {
-      address: {
-        "given-name": "John",
-        "family-name": "Doe",
-        "organization": "Mozilla",
-        "tel": "",
+      creditCard: {
+        "cc-number": "4444000022220000",
+        "cc-name": "Foo Bar",
+        "cc-exp": "2022-06",
+      },
+    },
+  },
+  {
+    description: "A credit card form with cc-number value only.",
+    document: `<form>
+                <input id="cc-number" autocomplete="cc-number">
+               </form>`,
+    formValue: {
+      "cc-number": "4444000022220000",
+    },
+    expectedRecord: {
+      creditCard: {
+        "cc-number": "4444000022220000",
       },
     },
   },
+  {
+    description: "A credit card form must have cc-number value.",
+    document: `<form>
+                <input id="cc-number" autocomplete="cc-number">
+                <input id="cc-name" autocomplete="cc-name">
+                <input id="cc-exp" autocomplete="cc-exp">
+               </form>`,
+    formValue: {
+      "cc-number": "",
+      "cc-name": "Foo Bar",
+      "cc-exp": "2022-06",
+    },
+    expectedRecord: {
+      creditCard: undefined,
+    },
+  },
+  {
+    description: "A credit card form must have cc-number field.",
+    document: `<form>
+                <input id="cc-name" autocomplete="cc-name">
+                <input id="cc-exp" autocomplete="cc-exp">
+               </form>`,
+    formValue: {
+      "cc-name": "Foo Bar",
+      "cc-exp": "2022-06",
+    },
+    expectedRecord: {
+      creditCard: undefined,
+    },
+  },
 ];
 
 for (let testcase of TESTCASES) {
   add_task(async function() {
     do_print("Starting testcase: " + testcase.description);
 
     let doc = MockDocument.createTestDocument("http://localhost:8080/test/", testcase.document);
     let form = doc.querySelector("form");