Bug 1411990 - Add consecutive cc-exp-* regex check in form autofill heuristics to enhance expiration date pattern matching. r=lchang, seanlee draft
authorRay Lin <ralin@mozilla.com>
Fri, 10 Nov 2017 01:28:43 +0800
changeset 698838 63378b230a39a88db9ca9364795014eec4ec6e98
parent 697940 f0c0fb9182d695081edf170d8e3bcb8164f2c96a
child 740445 a97f7eff0cd3a3aaad00057d0d21d4be8f386ac5
push id89366
push userbmo:ralin@mozilla.com
push dateThu, 16 Nov 2017 04:17:06 +0000
reviewerslchang, seanlee
bugs1411990
milestone59.0a1
Bug 1411990 - Add consecutive cc-exp-* regex check in form autofill heuristics to enhance expiration date pattern matching. r=lchang, seanlee MozReview-Commit-ID: 5P2nSSJd2Dl
browser/extensions/formautofill/FormAutofillHeuristics.jsm
browser/extensions/formautofill/test/fixtures/heuristics_cc_exp.html
browser/extensions/formautofill/test/unit/heuristics/test_cc_exp.js
--- a/browser/extensions/formautofill/FormAutofillHeuristics.jsm
+++ b/browser/extensions/formautofill/FormAutofillHeuristics.jsm
@@ -497,30 +497,51 @@ this.FormAutofillHeuristics = {
     }
     fieldScanner.parsingIndex = savedIndex;
 
     // Determine the field name by checking if the fields are month select and year select
     // likely.
     if (this._isExpirationMonthLikely(element)) {
       fieldScanner.updateFieldName(fieldScanner.parsingIndex, "cc-exp-month");
       fieldScanner.parsingIndex++;
-      const nextDetail = fieldScanner.getFieldDetailByIndex(fieldScanner.parsingIndex);
-      const nextElement = nextDetail.elementWeakRef.get();
-      if (this._isExpirationYearLikely(nextElement) && !fieldScanner.parsingFinished) {
-        fieldScanner.updateFieldName(fieldScanner.parsingIndex, "cc-exp-year");
-        fieldScanner.parsingIndex++;
+      if (!fieldScanner.parsingFinished) {
+        const nextDetail = fieldScanner.getFieldDetailByIndex(fieldScanner.parsingIndex);
+        const nextElement = nextDetail.elementWeakRef.get();
+        if (this._isExpirationYearLikely(nextElement)) {
+          fieldScanner.updateFieldName(fieldScanner.parsingIndex, "cc-exp-year");
+          fieldScanner.parsingIndex++;
+          return true;
+        }
+      }
+    }
+    fieldScanner.parsingIndex = savedIndex;
 
-        return true;
+    // Verify that the following consecutive two fields can match cc-exp-month and cc-exp-year
+    // respectively.
+    if (this._findMatchedFieldName(element, ["cc-exp-month"])) {
+      fieldScanner.updateFieldName(fieldScanner.parsingIndex, "cc-exp-month");
+      fieldScanner.parsingIndex++;
+      if (!fieldScanner.parsingFinished) {
+        const nextDetail = fieldScanner.getFieldDetailByIndex(fieldScanner.parsingIndex);
+        const nextElement = nextDetail.elementWeakRef.get();
+        if (this._findMatchedFieldName(nextElement, ["cc-exp-year"])) {
+          fieldScanner.updateFieldName(fieldScanner.parsingIndex, "cc-exp-year");
+          fieldScanner.parsingIndex++;
+          return true;
+        }
       }
     }
     fieldScanner.parsingIndex = savedIndex;
 
     // If no possible regular expiration fields are detected in current parsing window
     // fallback to "cc-exp" as there's no such case that cc-exp-month or cc-exp-year
     // presents alone.
+    // TODO: bug 1392947 - We should eventually remove this fallback, since we don't
+    // want to mess up deduplication if meanwhile a birthday was fallback to cc-exp
+    // that preceding the actual expiration fields.
     fieldScanner.updateFieldName(fieldScanner.parsingIndex, "cc-exp");
     fieldScanner.parsingIndex++;
 
     return true;
   },
 
   /**
    * This function should provide all field details of a form. The details
@@ -660,50 +681,82 @@ this.FormAutofillHeuristics = {
       };
     }
 
     let regexps = this._getRegExpList(isAutoCompleteOff, element.tagName);
     if (regexps.length == 0) {
       return null;
     }
 
-    let labelStrings;
-    let getElementStrings = {};
-    getElementStrings[Symbol.iterator] = function* () {
-      yield element.id;
-      yield element.name;
-      if (!labelStrings) {
-        labelStrings = [];
-        let labels = LabelUtils.findLabelElements(element);
+    let matchedFieldName =  this._findMatchedFieldName(element, regexps);
+    if (matchedFieldName) {
+      return {
+        fieldName: matchedFieldName,
+        section: "",
+        addressType: "",
+        contactType: "",
+      };
+    }
+
+    return null;
+  },
+
+
+  /**
+   * @typedef ElementStrings
+   * @type {object}
+   * @yield {string} id - element id.
+   * @yield {string} name - element name.
+   * @yield {Array<string>} labels - extracted labels.
+   */
+
+  /**
+   * Extract all the signature strings of an element.
+   *
+   * @param {HTMLElement} element
+   * @returns {ElementStrings}
+   */
+  _getElementStrings(element) {
+    return {
+      * [Symbol.iterator]() {
+        yield element.id;
+        yield element.name;
+
+        const labels = LabelUtils.findLabelElements(element);
         for (let label of labels) {
-          labelStrings.push(...LabelUtils.extractLabelStrings(label));
+          yield *LabelUtils.extractLabelStrings(label);
         }
-      }
-      yield *labelStrings;
+      },
     };
+  },
 
+  /**
+   * Find the first matched field name of the element wih given regex list.
+   *
+   * @param {HTMLElement} element
+   * @param {Array<string>} regexps
+   *        The regex key names that correspond to pattern in the rule list.
+   * @returns {?string} The first matched field name
+   */
+  _findMatchedFieldName(element, regexps) {
+    const getElementStrings = this._getElementStrings(element);
     for (let regexp of regexps) {
       for (let string of getElementStrings) {
         // The original regexp "(?<!united )state|county|region|province" for
         // "address-line1" wants to exclude any "united state" string, so the
         // following code is to remove all "united state" string before applying
         // "addess-level1" regexp.
         //
         // Since "united state" string matches to the regexp of address-line2&3,
         // the two regexps should be excluded here.
         if (["address-level1", "address-line2", "address-line3"].includes(regexp)) {
           string = string.toLowerCase().split("united state").join("");
         }
         if (this.RULES[regexp].test(string)) {
-          return {
-            fieldName: regexp,
-            section: "",
-            addressType: "",
-            contactType: "",
-          };
+          return regexp;
         }
       }
     }
 
     return null;
   },
 
 /**
--- a/browser/extensions/formautofill/test/fixtures/heuristics_cc_exp.html
+++ b/browser/extensions/formautofill/test/fixtures/heuristics_cc_exp.html
@@ -1,32 +1,36 @@
 <!DOCTYPE html>
 <html>
 <head>
   <meta charset="utf-8">
   <title>Heuristics cc-exp field test page</title>
 </head>
 <body>
   <h1>Heuristics cc-exp field test page</h1>
-  <form id="form">
+
+  <form id="form1">
     <p><label>Name: <input id="cc-name" autocomplete="cc-name"></label></p>
     <p><label>Card Number: <input id="cc-number" autocomplete="cc-number"></label></p>
     <p><label>Expiration month: <input id="cc-exp-month" autocomplete="cc-exp-month"></label></p>
     <p><label>Expiration year: <input id="cc-exp-year" autocomplete="cc-exp-year"></label></p>
     <p><label>CSC: <input id="cc-csc" autocomplete="cc-csc"></label></p>
   </form>
-  <form id="formB">
+
+  <form id="form2">
     <p><label>Card Number: <input id="cc-number" autocomplete="cc-number"></label></p>
     <p><label>Expiration Date: <input autocomplete="cc-exp"></label></p>
   </form>
-  <form id="formC">
+
+  <form id="form3">
     <p><label>Card Number: <input id="cc-number" autocomplete="cc-number"></label></p>
     <p><label>Expiration Date: <input type="text"></label></p>
   </form>
-  <form id="formD">
+
+  <form id="form4">
     <p><label>Card Number: <input id="cc-number" autocomplete="cc-number"></label></p>
     <p>
       <label>Exp:
         <select>
           <option value="1"></option>
           <option value="2"></option>
           <option value="3"></option>
           <option value="4"></option>
@@ -54,10 +58,16 @@
           <option value="2022"></option>
           <option value="2023"></option>
           <option value="2024"></option>
           <option value="2025"></option>
         </select>
       </label>
     </p>
   </form>
+
+  <form id="form5">
+    <input class="expire-date" maxlength="2" id="expiry-month" placeholder="MM" name="expireMonth" type="text">
+    <input id="expiry-year" class="expire-date" placeholder="YY" maxlength="2" name="expireYear" type="text">
+    <input maxlength="3" name="cvc" type="text">
+  </form>
 </body>
 </html>
--- a/browser/extensions/formautofill/test/unit/heuristics/test_cc_exp.js
+++ b/browser/extensions/formautofill/test/unit/heuristics/test_cc_exp.js
@@ -21,12 +21,16 @@ runHeuristicsTest([
         {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-number"},
         {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp"},
       ],
       [
         {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-number"},
         {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-month"},
         {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-year"},
       ],
+      [
+        {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-month"},
+        {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-year"},
+      ],
     ],
   },
 ], "../../fixtures/");