Bug 1400112 - Refresh `FormAutofillHandler.form` when the related form is changed. r=MattN,lchang draft
authorSean Lee <selee@mozilla.com>
Fri, 15 Sep 2017 11:32:13 +0800
changeset 666707 82acacaa50e9bd8a397c3bca4f89e028d566b782
parent 666274 ffe6cc09ccf38cca6f0e727837bbc6cb722d1e71
child 732182 7ed3c1af723c4707e1e1a53dd5a4fd12791974df
push id80487
push userbmo:selee@mozilla.com
push dateTue, 19 Sep 2017 04:06:44 +0000
reviewersMattN, lchang
bugs1400112
milestone57.0a1
Bug 1400112 - Refresh `FormAutofillHandler.form` when the related form is changed. r=MattN,lchang MozReview-Commit-ID: EmSID172pWo
browser/extensions/formautofill/FormAutofillContent.jsm
browser/extensions/formautofill/FormAutofillHandler.jsm
browser/extensions/formautofill/test/mochitest/formautofill_common.js
browser/extensions/formautofill/test/mochitest/mochitest.ini
browser/extensions/formautofill/test/mochitest/test_form_changes.html
browser/extensions/formautofill/test/mochitest/test_formautofill_preview_highlight.html
--- a/browser/extensions/formautofill/FormAutofillContent.jsm
+++ b/browser/extensions/formautofill/FormAutofillContent.jsm
@@ -488,17 +488,20 @@ var FormAutofillContent = {
       this.log.debug("identifyAutofillFields: savedFieldNames are not known yet");
       Services.cpmm.sendAsyncMessage("FormAutofill:InitStorage");
     }
 
     let formHandler = this.getFormHandler(element);
     if (!formHandler) {
       let formLike = FormLikeFactory.createFromField(element);
       formHandler = new FormAutofillHandler(formLike);
-    } else if (!formHandler.isFormChangedSinceLastCollection) {
+    } else if (formHandler.isFormUpdateNeeded(element)) {
+      this.log.debug("form update is needed!!!");
+      formHandler.updateForm(FormLikeFactory.createFromField(element));
+    } else {
       this.log.debug("No control is removed or inserted since last collection.");
       return;
     }
 
     let validDetails = formHandler.collectFormFields();
 
     this._formsDetails.set(formHandler.form.rootElement, formHandler);
     this.log.debug("Adding form handler to _formsDetails:", formHandler);
--- a/browser/extensions/formautofill/FormAutofillHandler.jsm
+++ b/browser/extensions/formautofill/FormAutofillHandler.jsm
@@ -14,27 +14,28 @@ const {classes: Cc, interfaces: Ci, util
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 Cu.import("resource://formautofill/FormAutofillUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "FormAutofillHeuristics",
                                   "resource://formautofill/FormAutofillHeuristics.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "FormLikeFactory",
+                                  "resource://gre/modules/FormLikeFactory.jsm");
 
 this.log = null;
 FormAutofillUtils.defineLazyLogGetter(this, this.EXPORTED_SYMBOLS[0]);
 
 /**
  * Handles profile autofill for a DOM Form element.
  * @param {FormLike} form Form that need to be auto filled
  */
 function FormAutofillHandler(form) {
-  this.form = form;
-  this.fieldDetails = [];
+  this.updateForm(form);
   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: [],
@@ -63,18 +64,16 @@ function FormAutofillHandler(form) {
 }
 
 FormAutofillHandler.prototype = {
   /**
    * DOM Form element to which this object is attached.
    */
   form: null,
 
-  _formFieldCount: 0,
-
   /**
    * Array of collected data about relevant form fields.  Each item is an object
    * storing the identifying details of the field and a reference to the
    * originally associated element from the form.
    *
    * The "section", "addressType", "contactType", and "fieldName" values are
    * used to identify the exact field when the serializable data is received
    * from the backend.  There cannot be multiple fields which have
@@ -107,40 +106,77 @@ FormAutofillHandler.prototype = {
     // not themed
     NORMAL: null,
     // highlighted
     AUTO_FILLED: "-moz-autofill",
     // highlighted && grey color text
     PREVIEW: "-moz-autofill-preview",
   },
 
-  get isFormChangedSinceLastCollection() {
-    // When the number of form controls is the same with last collection, it
-    // 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;
-  },
-
   /**
    * Time in milliseconds since epoch when a user started filling in the form.
    */
   timeStartedFillingMS: null,
 
   /**
+  * Check the form is necessary to be updated. This function should be able to
+  * detect any changes including all control elements in the form.
+  * @param {HTMLElement} element The element supposed to be in the form.
+  * @returns {boolena} Need update form or not.
+  */
+  isFormUpdateNeeded(element) {
+    // When the following condition happens, it considers the form needs to be
+    // updated:
+    // * The count of form controls is changed.
+    // * When the element can not be found in the current form.
+    //
+    // However, we should improve the function to detect the element changes.
+    // e.g. a tel field is changed from type="hidden" to type="tel".
+
+    let currentForm = element.form ? element.form : FormLikeFactory.createFromField(element);
+    if (currentForm.elements.length != this.form.elements.length) {
+      log.debug("The count of form elements is changed.");
+      return true;
+    }
+
+    if (!Array.from(this.form.elements).find(e => e === element)) {
+      log.debug("The element can not be found in the current form.");
+      return true;
+    }
+
+    return false;
+  },
+
+  /**
+  * Update the form with a new FormLike, and the related fields should be
+  * updated or clear to ensure the data consistency.
+  * @param {FormLike} form a new FormLike to replace the original one.
+  */
+  updateForm(form) {
+    this.form = form;
+    this.fieldDetails = [];
+
+    if (this.address) {
+      this.address.fieldDetails = [];
+    }
+    if (this.creditCard) {
+      this.creditCard.fieldDetails = [];
+    }
+  },
+
+  /**
    * 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(allowDuplicates = false) {
     this._cacheValue.allFieldNames = null;
-    this._formFieldCount = this.form.elements.length;
     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(
@@ -170,20 +206,27 @@ FormAutofillHandler.prototype = {
 
     return validDetails;
   },
 
   getFieldDetailByName(fieldName) {
     return this.fieldDetails.find(detail => detail.fieldName == fieldName);
   },
 
-  getFieldDetailsByElement(element) {
-    let fieldDetail = this.fieldDetails.find(
+  getFieldDetailByElement(element) {
+    return this.fieldDetails.find(
       detail => detail.elementWeakRef.get() == element
     );
+  },
+
+  getFieldDetailsByElement(element) {
+    let fieldDetail = this.getFieldDetailByElement(element);
+    if (!fieldDetail) {
+      return [];
+    }
     if (FormAutofillUtils.isAddressField(fieldDetail.fieldName)) {
       return this.address.fieldDetails;
     }
     if (FormAutofillUtils.isCreditCardField(fieldDetail.fieldName)) {
       return this.creditCard.fieldDetails;
     }
     return [];
   },
@@ -276,24 +319,22 @@ FormAutofillHandler.prototype = {
   },
 
   /**
    * Processes form fields that can be autofilled, and populates them with the
    * profile provided by backend.
    *
    * @param {Object} profile
    *        A profile to be filled in.
-   * @param {Object} focusedInput
+   * @param {HTMLElement} focusedInput
    *        A focused input element needed to determine the address or credit
    *        card field.
    */
   async autofillFormFields(profile, focusedInput) {
-    let focusedDetail = this.fieldDetails.find(
-      detail => detail.elementWeakRef.get() == focusedInput
-    );
+    let focusedDetail = this.getFieldDetailByElement(focusedInput);
     let targetSet;
     if (FormAutofillUtils.isCreditCardField(focusedDetail.fieldName)) {
       // When Master Password is enabled by users, the decryption process
       // should prompt Master Password dialog to get the decrypted credit
       // card number. Otherwise, the number can be decrypted with the default
       // password.
       if (profile["cc-number-encrypted"]) {
         let decrypted = await this._decrypt(profile["cc-number-encrypted"], true);
@@ -395,17 +436,17 @@ FormAutofillHandler.prototype = {
     this.form.rootElement.addEventListener("reset", onChangeHandler);
   },
 
   /**
    * Populates result to the preview layers with given profile.
    *
    * @param {Object} profile
    *        A profile to be previewed with
-   * @param {Object} focusedInput
+   * @param {HTMLElement} focusedInput
    *        A focused input element for determining credit card or address fields.
    */
   previewFormFields(profile, focusedInput) {
     log.debug("preview profile in autofillFormFields:", profile);
 
     // Always show the decrypted credit card number when Master Password is
     // disabled.
     if (profile["cc-number-decrypted"]) {
@@ -441,17 +482,17 @@ FormAutofillHandler.prototype = {
       element.previewValue = value;
       this.changeFieldState(fieldDetail, value ? "PREVIEW" : "NORMAL");
     }
   },
 
   /**
    * Clear preview text and background highlight of all fields.
    *
-   * @param {Object} focusedInput
+   * @param {HTMLElement} focusedInput
    *        A focused input element for determining credit card or address fields.
    */
   clearPreviewedFormFields(focusedInput) {
     log.debug("clear previewed fields in:", this.form);
 
     let fieldDetails = this.getFieldDetailsByElement(focusedInput);
     for (let fieldDetail of fieldDetails) {
       let element = fieldDetail.elementWeakRef.get();
--- a/browser/extensions/formautofill/test/mochitest/formautofill_common.js
+++ b/browser/extensions/formautofill/test/mochitest/formautofill_common.js
@@ -7,31 +7,38 @@
 let formFillChromeScript;
 let expectingPopup = null;
 
 async function sleep(ms = 500, reason = "Intentionally wait for UI ready") {
   SimpleTest.requestFlakyTimeout(reason);
   await new Promise(resolve => setTimeout(resolve, ms));
 }
 
-async function setInput(selector, value) {
-  let input = document.querySelector("input" + selector);
-  input.value = value;
+async function focusAndWaitCollectionReady(input) {
+  if (typeof input === "string") {
+    input = document.querySelector(input);
+  }
   input.focus();
 
   // "identifyAutofillFields" is invoked asynchronously in "focusin" event. We
   // should make sure fields are ready for popup before doing tests.
   //
   // TODO: "sleep" is used here temporarily because there's no event to
   //       notify us of the state of "identifyAutofillFields" for now. We should
   //       figure out a better way after the heuristics land.
   await sleep(500, "Guarantee asynchronous identifyAutofillFields is invoked");
   return input;
 }
 
+async function setInput(selector, value) {
+  let input = document.querySelector("input" + selector);
+  input.value = value;
+  return focusAndWaitCollectionReady(input);
+}
+
 function clickOnElement(selector) {
   let element = document.querySelector(selector);
 
   if (!element) {
     throw new Error("Can not find the element");
   }
 
   SimpleTest.executeSoon(() => element.click());
--- a/browser/extensions/formautofill/test/mochitest/mochitest.ini
+++ b/browser/extensions/formautofill/test/mochitest/mochitest.ini
@@ -2,11 +2,12 @@
 support-files =
   ../../../../../toolkit/components/satchel/test/satchel_common.js
   ../../../../../toolkit/components/satchel/test/parent_utils.js
   formautofill_common.js
   formautofill_parent_utils.js
 
 [test_autofocus_form.html]
 [test_basic_autocomplete_form.html]
+[test_form_changes.html]
 [test_formautofill_preview_highlight.html]
 [test_multiple_forms.html]
 [test_on_address_submission.html]
new file mode 100644
--- /dev/null
+++ b/browser/extensions/formautofill/test/mochitest/test_form_changes.html
@@ -0,0 +1,107 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test basic autofill</title>
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="formautofill_common.js"></script>
+  <script type="text/javascript" src="satchel_common.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
+</head>
+<body>
+Form autofill test: autocomplete on an autofocus form
+
+<script>
+/* import-globals-from ../../../../../testing/mochitest/tests/SimpleTest/SpawnTask.js */
+/* import-globals-from ../../../../../toolkit/components/satchel/test/satchel_common.js */
+/* import-globals-from formautofill_common.js */
+
+"use strict";
+
+let MOCK_STORAGE = [{
+  name: "John Doe",
+  organization: "Sesame Street",
+  "address-level2": "Austin",
+  tel: "+13453453456",
+}, {
+  name: "Foo Bar",
+  organization: "Mozilla",
+  "address-level2": "San Francisco",
+  tel: "+16509030800",
+}];
+
+initPopupListener();
+
+async function setupAddressStorage() {
+  await addAddress(MOCK_STORAGE[0]);
+  await addAddress(MOCK_STORAGE[1]);
+}
+
+function addInputField(form, className) {
+  let newElem = document.createElement("input");
+  newElem.name = className;
+  newElem.autocomplete = className;
+  newElem.type = "text";
+  form.appendChild(newElem);
+}
+
+async function checkFormChangeHappened(formId) {
+  await focusAndWaitCollectionReady(`#${formId} input[name=tel]`);
+  doKey("down");
+  await expectPopup();
+  checkMenuEntries(MOCK_STORAGE.map(address =>
+    JSON.stringify({primary: address.tel, secondary: address.name})
+  ));
+
+  // This is for checking the changes of element count.
+  addInputField(document.querySelector(`#${formId}`), "address-level2");
+
+  await focusAndWaitCollectionReady(`#${formId} input[name=name]`);
+  doKey("down");
+  await expectPopup();
+  checkMenuEntries(MOCK_STORAGE.map(address =>
+    JSON.stringify({primary: address.name, secondary: address["address-level2"]})
+  ));
+
+  // This is for checking the changes of element removed and added then.
+  document.querySelector(`#${formId} input[name=address-level2]`).remove();
+  addInputField(document.querySelector(`#${formId}`), "address-level2");
+
+  await focusAndWaitCollectionReady(`#${formId} input[name=address-level2]`);
+  doKey("down");
+  await expectPopup();
+  checkMenuEntries(MOCK_STORAGE.map(address =>
+    JSON.stringify({primary: address["address-level2"], secondary: address.name})
+  ));
+}
+
+add_task(async function init_storage() {
+  await setupAddressStorage();
+});
+
+add_task(async function check_change_happened_in_form() {
+  await checkFormChangeHappened("form1");
+});
+
+add_task(async function check_change_happened_in_body() {
+  await checkFormChangeHappened("form2");
+});
+</script>
+
+<p id="display"></p>
+<div id="content">
+  <form id="form1">
+    <p><label>organization: <input name="organization" autocomplete="organization" type="text"></label></p>
+    <p><label>tel: <input name="tel" autocomplete="tel" type="text"></label></p>
+    <p><label>name: <input name="name" autocomplete="name" type="text"></label></p>
+  </form>
+  <div id="form2">
+    <p><label>organization: <input name="organization" autocomplete="organization" type="text"></label></p>
+    <p><label>tel: <input name="tel" autocomplete="tel" type="text"></label></p>
+    <p><label>name: <input name="name" autocomplete="name" type="text"></label></p>
+  </div>
+</div>
+<pre id="test"></pre>
+</body>
+</html>
--- a/browser/extensions/formautofill/test/mochitest/test_formautofill_preview_highlight.html
+++ b/browser/extensions/formautofill/test/mochitest/test_formautofill_preview_highlight.html
@@ -93,35 +93,45 @@ add_task(async function setup_storage() 
 
   await addAddress(MOCK_STORAGE[0]);
   await addAddress(MOCK_STORAGE[1]);
   await addAddress(MOCK_STORAGE[2]);
 });
 
 add_task(async function check_preview() {
   const focusedInput = await setInput("#organization", "");
+  dump("QQQ - 1");
+  dump(focusedInput);
 
   doKey("down");
   await expectPopup();
   checkFormPreviewFields();
+  dump("QQQ - 2");
+  dump(focusedInput);
 
   for (let i = 0; i < MOCK_STORAGE.length; i++) {
     doKey("down");
     await notifySelectedIndex(i);
     checkFormPreviewFields(MOCK_STORAGE[i]);
   }
+  dump("QQQ - 3");
+  dump(focusedInput);
 
   // Navigate to the footer
   doKey("down");
   await notifySelectedIndex(MOCK_STORAGE.length);
   checkFormPreviewFields();
+  dump("QQQ - 4");
+  dump(focusedInput);
 
   doKey("down");
   await notifySelectedIndex(-1);
   checkFormPreviewFields();
+  dump("QQQ - 5");
+  dump(focusedInput);
 
   focusedInput.blur();
 });
 
 add_task(async function check_filled_highlight() {
   const focusedInput = await setInput("#organization", "");
 
   doKey("down");