Bug 1379588 - Part 1: Add address/creditCard object in handler and refactor collectFormFields for less cross-module call. r=lchang,seanlee

MozReview-Commit-ID: 6FkfGH8nre1 --HG-- extra : rebase_source : 32b68fd0aebc92e94a4bf432ff6fc8639f40bd53
2017-07-24 12:15:24 +08:00 · 2017-07-24 12:15:24 +08:00 · 0cd16b2ca9
--- a/browser/extensions/formautofill/FormAutofillContent.jsm
+++ b/browser/extensions/formautofill/FormAutofillContent.jsm
@ -95,7 +95,7 @@ AutofillProfileAutoCompleteSearch.prototype = {
    let info = FormAutofillContent.getInputDetails(focusedInput);

    if (!FormAutofillContent.savedFieldNames.has(info.fieldName) ||
-        FormAutofillContent.getFormHandler(focusedInput).filledProfileGUID) {
+        FormAutofillContent.getFormHandler(focusedInput).address.filledRecordGUID) {
      let formHistory = Cc["@mozilla.org/autocomplete/search;1?name=form-history"]
                          .createInstance(Ci.nsIAutoCompleteSearch);
      formHistory.startSearch(searchString, searchParam, previousResult, {
@ -385,7 +385,7 @@ var FormAutofillContent = {

    this._onFormSubmit({
      address: {
-        guid: handler.filledProfileGUID,
+        guid: handler.address.filledRecordGUID,
        record: pendingAddress,
      },
      // creditCard: {}
@ -484,28 +484,14 @@ var FormAutofillContent = {
      return;
    }

-    formHandler.collectFormFields();
+    let validDetails = formHandler.collectFormFields();

    this._formsDetails.set(formHandler.form.rootElement, formHandler);
    this.log.debug("Adding form handler to _formsDetails:", formHandler);

-    if (formHandler.isValidAddressForm) {
-      formHandler.addressFieldDetails.forEach(
-        detail => this._markAsAutofillField(detail.elementWeakRef.get())
-      );
-    } else {
-      this.log.debug("Ignoring address related fields since it has only",
-                     formHandler.addressFieldDetails.length,
-                     "field(s)");
-    }
-
-    if (formHandler.isValidCreditCardForm) {
-      formHandler.creditCardFieldDetails.forEach(
-        detail => this._markAsAutofillField(detail.elementWeakRef.get())
-      );
-    } else {
-      this.log.debug("Ignoring credit card related fields since it's without credit card number field");
-    }
+    validDetails.forEach(detail =>
+      this._markAsAutofillField(detail.elementWeakRef.get())
+    );
  },

  _markAsAutofillField(field) {
--- a/browser/extensions/formautofill/FormAutofillHandler.jsm
+++ b/browser/extensions/formautofill/FormAutofillHandler.jsm
@ -31,6 +31,28 @@ function FormAutofillHandler(form) {
  this.fieldDetails = [];
  this.winUtils = this.form.rootElement.ownerGlobal.QueryInterface(Ci.nsIInterfaceRequestor)
    .getInterface(Ci.nsIDOMWindowUtils);
+
+  this.address = {
+    /**
+     * Similar to the `fieldDetails` above but contains address fields only.
+     */
+    fieldDetails: [],
+    /**
+     * String of the filled address' guid.
+     */
+    filledRecordGUID: null,
+  };
+
+  this.creditCard = {
+    /**
+     * Similar to the `fieldDetails` above but contains credit card fields only.
+     */
+    fieldDetails: [],
+    /**
+     * String of the filled creditCard's guid.
+     */
+    filledRecordGUID: null,
+  };
 }

 FormAutofillHandler.prototype = {
@ -57,29 +79,14 @@ FormAutofillHandler.prototype = {
  fieldDetails: null,

  /**
-   * Similiar to `fieldDetails`, and `addressFieldDetails` contains the address
-   * records only.
+   * Subcategory of handler that contains address related data.
   */
-  addressFieldDetails: null,
+  address: null,

  /**
-   * Similiar to `fieldDetails`, and `creditCardFieldDetails` contains the
-   * Credit Card records only.
+   * Subcategory of handler that contains credit card related data.
   */
-  creditCardFieldDetails: null,
-
-  get isValidAddressForm() {
-    return this.addressFieldDetails.length >= FormAutofillUtils.AUTOFILL_FIELDS_THRESHOLD;
-  },
-
-  get isValidCreditCardForm() {
-    return this.creditCardFieldDetails.some(i => i.fieldName == "cc-number");
-  },
-
-  /**
-   * String of the filled profile's guid.
-   */
-  filledProfileGUID: null,
+  creditCard: null,

  /**
   * A WindowUtils reference of which Window the form belongs
@ -108,6 +115,8 @@ FormAutofillHandler.prototype = {

  /**
   * Set fieldDetails from the form about fields that can be autofilled.
+
+   * @returns {Array} The valid address and credit card details.
   */
  collectFormFields() {
    this._cacheValue.allFieldNames = null;
@ -116,12 +125,27 @@ FormAutofillHandler.prototype = {
    this.fieldDetails = fieldDetails ? fieldDetails : [];
    log.debug("Collected details on", this.fieldDetails.length, "fields");

-    this.addressFieldDetails = this.fieldDetails.filter(
+    this.address.fieldDetails = this.fieldDetails.filter(
      detail => FormAutofillUtils.isAddressField(detail.fieldName)
    );
-    this.creditCardFieldDetails = this.fieldDetails.filter(
+    this.creditCard.fieldDetails = this.fieldDetails.filter(
      detail => FormAutofillUtils.isCreditCardField(detail.fieldName)
    );
+
+    if (this.address.fieldDetails.length < FormAutofillUtils.AUTOFILL_FIELDS_THRESHOLD) {
+      log.debug("Ignoring address related fields since it has only",
+                this.address.fieldDetails.length,
+                "field(s)");
+      this.address.fieldDetails = [];
+    }
+
+    if (!this.creditCard.fieldDetails.some(i => i.fieldName == "cc-number")) {
+      log.debug("Ignoring credit card related fields since it's without credit card number field");
+      this.creditCard.fieldDetails = [];
+    }
+
+    return Array.of(...(this.address.fieldDetails),
+                    ...(this.creditCard.fieldDetails));
  },

  getFieldDetailByName(fieldName) {
@ -193,8 +217,8 @@ FormAutofillHandler.prototype = {
  autofillFormFields(profile, focusedInput) {
    log.debug("profile in autofillFormFields:", profile);

-    this.filledProfileGUID = profile.guid;
-    for (let fieldDetail of this.addressFieldDetails) {
+    this.address.filledRecordGUID = profile.guid;
+    for (let fieldDetail of this.address.fieldDetails) {
      // Avoid filling field value in the following cases:
      // 1. the focused input which is filled in FormFillController.
      // 2. a non-empty input field
@ -255,7 +279,7 @@ FormAutofillHandler.prototype = {
        return;
      }

-      for (let fieldDetail of this.addressFieldDetails) {
+      for (let fieldDetail of this.address.fieldDetails) {
        let element = fieldDetail.elementWeakRef.get();

        if (!element) {
@ -273,7 +297,7 @@ FormAutofillHandler.prototype = {
      if (!hasFilledFields) {
        this.form.rootElement.removeEventListener("input", onChangeHandler);
        this.form.rootElement.removeEventListener("reset", onChangeHandler);
-        this.filledProfileGUID = null;
+        this.address.filledRecordGUID = null;
      }
    };

@ -290,7 +314,7 @@ FormAutofillHandler.prototype = {
  previewFormFields(profile) {
    log.debug("preview profile in autofillFormFields:", profile);

-    for (let fieldDetail of this.addressFieldDetails) {
+    for (let fieldDetail of this.address.fieldDetails) {
      let element = fieldDetail.elementWeakRef.get();
      let value = profile[fieldDetail.fieldName] || "";

@ -322,7 +346,7 @@ FormAutofillHandler.prototype = {
  clearPreviewedFormFields() {
    log.debug("clear previewed fields in:", this.form);

-    for (let fieldDetail of this.addressFieldDetails) {
+    for (let fieldDetail of this.address.fieldDetails) {
      let element = fieldDetail.elementWeakRef.get();
      if (!element) {
        log.warn(fieldDetail.fieldName, "is unreachable");
@ -387,7 +411,7 @@ FormAutofillHandler.prototype = {
  createProfile() {
    let profile = {};

-    this.addressFieldDetails.forEach(detail => {
+    this.address.fieldDetails.forEach(detail => {
      let element = detail.elementWeakRef.get();
      // Remove the unnecessary spaces
      let value = element && element.value.trim();
--- a/browser/extensions/formautofill/test/unit/test_autofillFormFields.js
+++ b/browser/extensions/formautofill/test/unit/test_autofillFormFields.js
@ -323,8 +323,8 @@ function do_test(testcases, testFn) {
        let handler = new FormAutofillHandler(formLike);
        let promises = [];

-        handler.addressFieldDetails = testcase.addressFieldDetails;
-        handler.addressFieldDetails.forEach((field, index) => {
+        handler.address.fieldDetails = testcase.addressFieldDetails;
+        handler.address.fieldDetails.forEach((field, index) => {
          let element = doc.querySelectorAll("input, select")[index];
          field.elementWeakRef = Cu.getWeakReference(element);
          if (!testcase.profileData[field.fieldName]) {
@ -336,8 +336,8 @@ function do_test(testcases, testFn) {
        });

        handler.autofillFormFields(testcase.profileData);
-        Assert.equal(handler.filledProfileGUID, testcase.profileData.guid,
-                     "Check if filledProfileGUID is set correctly");
+        Assert.equal(handler.address.filledRecordGUID, testcase.profileData.guid,
+                     "Check if filledRecordGUID is set correctly");
        await Promise.all(promises);
      });
    })();
--- a/browser/extensions/formautofill/test/unit/test_collectFormFields.js
+++ b/browser/extensions/formautofill/test/unit/test_collectFormFields.js
@ -22,10 +22,15 @@ const TESTCASES = [
      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel"},
    ],
    creditCardFieldDetails: [],
-    isValidForm: {
-      address: true,
-      creditCard: false,
-    },
+    validFieldDetails: [
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "given-name"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "family-name"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "address-line1"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "address-level2"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "country"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "email"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel"},
+    ],
    ids: ["given-name", "family-name", "street-addr", "city", "country", "email", "phone"],
  },
  {
@ -58,10 +63,19 @@ const TESTCASES = [
      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-month"},
      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-year"},
    ],
-    isValidForm: {
-      address: true,
-      creditCard: true,
-    },
+    validFieldDetails: [
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "given-name"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "family-name"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "street-address"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "address-level2"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "country"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "email"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-number"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-name"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-month"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-year"},
+    ],
  },
  {
    description: "An address form with autocomplete properties and 2 tokens",
@ -82,10 +96,15 @@ const TESTCASES = [
      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel"},
    ],
    creditCardFieldDetails: [],
-    isValidForm: {
-      address: true,
-      creditCard: false,
-    },
+    validFieldDetails: [
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "given-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "family-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "street-address"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "address-level2"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "country"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "email"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel"},
+    ],
  },
  {
    description: "Form with autocomplete properties and profile is partly matched",
@ -106,10 +125,15 @@ const TESTCASES = [
      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel"},
    ],
    creditCardFieldDetails: [],
-    isValidForm: {
-      address: true,
-      creditCard: false,
-    },
+    validFieldDetails: [
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "given-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "family-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "street-address"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "address-level2"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "country"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "email"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel"},
+    ],
  },
  {
    description: "It's a valid address and credit card form.",
@ -127,10 +151,12 @@ const TESTCASES = [
    creditCardFieldDetails: [
      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "cc-number"},
    ],
-    isValidForm: {
-      address: true,
-      creditCard: true,
-    },
+    validFieldDetails: [
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "given-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "family-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "street-address"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "cc-number"},
+    ],
  },
  {
    description: "It's an invalid address and credit form.",
@ -141,19 +167,9 @@ const TESTCASES = [
               <input id="cc-exp-month" autocomplete="cc-exp-month">
               <input id="cc-exp-year" autocomplete="cc-exp-year">
               </form>`,
-    addressFieldDetails: [
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "given-name"},
-      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "address-level2"},
-    ],
-    creditCardFieldDetails: [
-      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-name"},
-      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-month"},
-      {"section": "", "addressType": "", "contactType": "", "fieldName": "cc-exp-year"},
-    ],
-    isValidForm: {
-      address: false,
-      creditCard: false,
-    },
+    addressFieldDetails: [],
+    creditCardFieldDetails: [],
+    validFieldDetails: [],
  },
  {
    description: "Three sets of adjacent phone number fields",
@ -186,10 +202,19 @@ const TESTCASES = [
      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel-local-suffix"},
    ],
    creditCardFieldDetails: [],
-    isValidForm: {
-      address: true,
-      creditCard: false,
-    },
+    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-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",
@ -216,10 +241,13 @@ const TESTCASES = [
      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel"},
    ],
    creditCardFieldDetails: [],
-    isValidForm: {
-      address: true,
-      creditCard: false,
-    },
+    validFieldDetails: [
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "given-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "family-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "street-address"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "email"},
+      {"section": "", "addressType": "", "contactType": "", "fieldName": "tel"},
+    ],
    ids: ["i1", "i2", "i3", "i4", "homePhone"],
  },
  {
@ -249,10 +277,16 @@ const TESTCASES = [
      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "tel-local-suffix"},
    ],
    creditCardFieldDetails: [],
-    isValidForm: {
-      address: true,
-      creditCard: false,
-    },
+    validFieldDetails: [
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "given-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "family-name"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "street-address"},
+      {"section": "", "addressType": "shipping", "contactType": "", "fieldName": "email"},
+      {"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"],
  },
@ -269,23 +303,27 @@ for (let tc of TESTCASES) {
      let form = doc.querySelector("form");
      let formLike = FormLikeFactory.createFromForm(form);

-      Array.of(
-        ...testcase.addressFieldDetails,
-        ...testcase.creditCardFieldDetails
-      ).forEach((detail, index) => {
-        let elementRef;
-        if (testcase.ids && testcase.ids[index]) {
-          elementRef = doc.getElementById(testcase.ids[index]);
-        } else {
-          elementRef = doc.querySelector("*[autocomplete*='" + detail.fieldName + "']");
+      function setElementWeakRef(details) {
+        if (!details) {
+          return;
        }
-        detail.elementWeakRef = Cu.getWeakReference(elementRef);
-      });
-      let handler = new FormAutofillHandler(formLike);

-      handler.collectFormFields();
+        details.forEach((detail, index) => {
+          let elementRef;
+          if (testcase.ids && testcase.ids[index]) {
+            elementRef = doc.getElementById(testcase.ids[index]);
+          } else {
+            elementRef = doc.querySelector("*[autocomplete*='" + detail.fieldName + "']");
+          }
+          detail.elementWeakRef = Cu.getWeakReference(elementRef);
+        });
+      }

      function verifyDetails(handlerDetails, testCaseDetails) {
+        if (handlerDetails === null) {
+          Assert.equal(handlerDetails, testCaseDetails);
+          return;
+        }
        Assert.equal(handlerDetails.length, testCaseDetails.length);
        handlerDetails.forEach((detail, index) => {
          Assert.equal(detail.fieldName, testCaseDetails[index].fieldName, "fieldName");
@ -295,12 +333,18 @@ for (let tc of TESTCASES) {
          Assert.equal(detail.elementWeakRef.get(), testCaseDetails[index].elementWeakRef.get(), "DOM reference");
        });
      }
+      [
+        testcase.addressFieldDetails,
+        testcase.creditCardFieldDetails,
+        testcase.validFieldDetails,
+      ].forEach(details => setElementWeakRef(details));

-      verifyDetails(handler.addressFieldDetails, testcase.addressFieldDetails);
-      verifyDetails(handler.creditCardFieldDetails, testcase.creditCardFieldDetails);
+      let handler = new FormAutofillHandler(formLike);
+      let validFieldDetails = handler.collectFormFields();

-      Assert.equal(handler.isValidAddressForm, testcase.isValidForm.address, "Valid Address Form Checking");
-      Assert.equal(handler.isValidCreditCardForm, testcase.isValidForm.creditCard, "Valid Credit Card Form Checking");
+      verifyDetails(handler.address.fieldDetails, testcase.addressFieldDetails);
+      verifyDetails(handler.creditCard.fieldDetails, testcase.creditCardFieldDetails);
+      verifyDetails(validFieldDetails, testcase.validFieldDetails);
    });
  })();
 }