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 685113 bcf857349e96adcbe0620d4e004ca854b5aa4ec5
parent 684511 ce1a86d3b4db161c95d1147676bbed839d7a4732
child 737052 2dd21b23a10cda725b77ab4eed87d9ae14276363
push id85823
push userbmo:selee@mozilla.com
push dateTue, 24 Oct 2017 03:05:00 +0000
reviewersMattN, lchang
bugs1400112
milestone58.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
--- a/browser/extensions/formautofill/FormAutofillContent.jsm
+++ b/browser/extensions/formautofill/FormAutofillContent.jsm
@@ -499,17 +499,17 @@ 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.updateFormIfNeeded(element)) {
       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,91 @@ 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 {boolean} FormAutofillHandler.form is updated or not.
+  */
+  updateFormIfNeeded(element) {
+    // When the following condition happens, FormAutofillHandler.form should 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 _formLike;
+    let getFormLike = () => {
+      if (!_formLike) {
+        _formLike = FormLikeFactory.createFromField(element);
+      }
+      return _formLike;
+    };
+
+    let currentForm = element.form;
+    if (!currentForm) {
+      currentForm = getFormLike();
+    }
+
+    if (currentForm.elements.length != this.form.elements.length) {
+      log.debug("The count of form elements is changed.");
+      this._updateForm(getFormLike());
+      return true;
+    }
+
+    if (this.form.elements.indexOf(element) === -1) {
+      log.debug("The element can not be found in the current form.");
+      this._updateForm(getFormLike());
+      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(
@@ -398,24 +448,25 @@ 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);
+    if (!focusedDetail) {
+      throw new Error("No fieldDetail for the focused input.");
+    }
     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);
@@ -517,17 +568,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"]) {
@@ -563,17 +614,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
@@ -9,22 +9,28 @@ let expectingPopup = null;
 
 const {FormAutofillUtils} = SpecialPowers.Cu.import("resource://formautofill/FormAutofillUtils.jsm");
 
 async function sleep(ms = 500, reason = "Intentionally wait for UI ready") {
   SimpleTest.requestFlakyTimeout(reason);
   await new Promise(resolve => setTimeout(resolve, ms));
 }
 
-async function focusAndWaitForFieldsIdentified(input) {
+async function focusAndWaitForFieldsIdentified(input, mustBeIdentified = false) {
+  if (typeof input === "string") {
+    input = document.querySelector(input);
+  }
   const rootElement = input.form || input.ownerDocument.documentElement;
   const previouslyFocused = input != document.activeElement;
 
   input.focus();
 
+  if (mustBeIdentified) {
+    rootElement.removeAttribute("test-formautofill-identified");
+  }
   if (rootElement.hasAttribute("test-formautofill-identified")) {
     return;
   }
   if (!previouslyFocused) {
     await new Promise(resolve => {
       formFillChromeScript.addMessageListener("FormAutofillTest:FieldsIdentified", function onIdentified() {
         formFillChromeScript.removeMessageListener("FormAutofillTest:FieldsIdentified", onIdentified);
         resolve();
--- a/browser/extensions/formautofill/test/mochitest/mochitest.ini
+++ b/browser/extensions/formautofill/test/mochitest/mochitest.ini
@@ -8,11 +8,12 @@ support-files =
   formautofill_parent_utils.js
 
 [test_autofocus_form.html]
 [test_basic_autocomplete_form.html]
 [test_basic_creditcard_autocomplete_form.html]
 scheme=https
 [test_creditcard_autocomplete_off.html]
 scheme=https
+[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 focusAndWaitForFieldsIdentified(`#${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 focusAndWaitForFieldsIdentified(`#${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 focusAndWaitForFieldsIdentified(`#${formId} input[name=address-level2]`, true);
+  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>