Bug 1372843 - Identify the focused form only when applying heuristics. r=MattN

MozReview-Commit-ID: 9y6GgYxlQt3

--HG--
extra : rebase_source : 476d33b9b040d95cd039dc555be699d28e0fed59

browser/extensions/formautofill/FormAutofillContent.jsm

@@ -440,54 +440,42 @@ var FormAutofillContent = {
     return formDetails.map(record => record.fieldName);
   },
 
-  identifyAutofillFields(doc) {
-    this.log.debug("identifyAutofillFields:", "" + doc.location);
+  identifyAutofillFields(element) {
+    this.log.debug("identifyAutofillFields:", "" + element.ownerDocument.location);
 
     if (!this.savedFieldNames) {
       this.log.debug("identifyAutofillFields: savedFieldNames are not known yet");
       Services.cpmm.sendAsyncMessage("FormAutofill:InitStorage");
     }
 
-    let forms = [];
-
-    // Collects root forms from inputs.
-    for (let field of FormAutofillUtils.autofillFieldSelector(doc)) {
-      if (!FormAutofillUtils.isFieldEligibleForAutofill(field)) {
-        continue;
-      }
-
-      // For now skip consider fields in forms we've already seen before even
-      // if the specific field wasn't seen before. Ideally whether the field is
-      // already in the handler's form details would be considered.
-      if (this.getFormHandler(field)) {
-        continue;
-      }
-
-      let formLike = FormLikeFactory.createFromField(field);
-      if (!forms.some(form => form.rootElement === formLike.rootElement)) {
-        forms.push(formLike);
-      }
+    if (!FormAutofillUtils.isFieldEligibleForAutofill(element)) {
+      this.log.debug("Not an eligible field.");
+      return;
     }
 
-    this.log.debug("Found", forms.length, "forms");
+    let formHandler = this.getFormHandler(element);
+    if (!formHandler) {
+      let formLike = FormLikeFactory.createFromField(element);
+      formHandler = new FormAutofillHandler(formLike);
+    } else if (!formHandler.isFormChangedSinceLastCollection) {
+      this.log.debug("No control is removed or inserted since last collection.");
+      return;
+    }
 
-    // Collects the fields that can be autofilled from each form and marks them
-    // as autofill fields if the amount is above the threshold.
-    forms.forEach(form => {
-      let formHandler = new FormAutofillHandler(form);
-      formHandler.collectFormFields();
-      if (formHandler.fieldDetails.length < AUTOFILL_FIELDS_THRESHOLD) {
-        this.log.debug("Ignoring form since it has only", formHandler.fieldDetails.length,
-                       "field(s)");
-        return;
-      }
+    formHandler.collectFormFields();
 
-      this._formsDetails.set(form.rootElement, formHandler);
-      this.log.debug("Adding form handler to _formsDetails:", formHandler);
-      formHandler.fieldDetails.forEach(detail =>
-        this._markAsAutofillField(detail.elementWeakRef.get())
-      );
-    });
+    this._formsDetails.set(formHandler.form.rootElement, formHandler);
+    this.log.debug("Adding form handler to _formsDetails:", formHandler);
+
+    if (formHandler.fieldDetails.length < AUTOFILL_FIELDS_THRESHOLD) {
+      this.log.debug("Ignoring form since it has only", formHandler.fieldDetails.length,
+                     "field(s)");
+      return;
+    }
+
+    formHandler.fieldDetails.forEach(detail =>
+      this._markAsAutofillField(detail.elementWeakRef.get())
+    );
   },
 
   _markAsAutofillField(field) {

browser/extensions/formautofill/FormAutofillHandler.jsm

@@ -39,6 +39,8 @@ FormAutofillHandler.prototype = {
    */
   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
@@ -76,10 +78,19 @@ FormAutofillHandler.prototype = {
     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;
+  },
+
   /**
    * Set fieldDetails from the form about fields that can be autofilled.
    */
   collectFormFields() {
+    this._formFieldCount = this.form.elements.length;
     let fieldDetails = FormAutofillHeuristics.getFormInfo(this.form);
     this.fieldDetails = fieldDetails ? fieldDetails : [];
     log.debug("Collected details on", this.fieldDetails.length, "fields");

browser/extensions/formautofill/content/FormAutofillFrameScript.js

@@ -48,7 +48,7 @@ var FormAutofillFrameScript = {
         }
 
         let doIdentifyAutofillFields =
-          () => setTimeout(() => FormAutofillContent.identifyAutofillFields(doc));
+          () => setTimeout(() => FormAutofillContent.identifyAutofillFields(element));
 
         if (doc.readyState === "loading") {
           doc.addEventListener("DOMContentLoaded", doIdentifyAutofillFields, {once: true});

browser/extensions/formautofill/test/unit/test_getFormInputDetails.js

@@ -84,10 +84,10 @@ TESTCASES.forEach(testcase => {
 
     let doc = MockDocument.createTestDocument(
               "http://localhost:8080/test/", testcase.document);
-    FormAutofillContent.identifyAutofillFields(doc);
 
     for (let i in testcase.targetInput) {
       let input = doc.getElementById(testcase.targetInput[i]);
+      FormAutofillContent.identifyAutofillFields(input);
 
       // Put the input element reference to `element` to make sure the result of
       // `getInputDetails` contains the same input element.

browser/extensions/formautofill/test/unit/test_markAsAutofillField.js

@@ -17,6 +17,7 @@ const TESTCASES = [
                  <input id="without-autocomplete-1">
                  <input id="without-autocomplete-2">
                </form>`,
+    targetElementId: "given-name",
     expectedResult: [
       "given-name",
       "additional-name",
@@ -36,6 +37,7 @@ const TESTCASES = [
                  <input id="without-autocomplete-1">
                  <input id="without-autocomplete-2">
                </form>`,
+    targetElementId: "street-addr",
     expectedResult: [],
   },
   {
@@ -47,6 +49,7 @@ const TESTCASES = [
                <input id="tel" autocomplete="tel">
                <input id="without-autocomplete-1">
                <input id="without-autocomplete-2">`,
+    targetElementId: "street-addr",
     expectedResult: [
       "street-addr",
       "city",
@@ -70,7 +73,8 @@ TESTCASES.forEach(testcase => {
 
     let doc = MockDocument.createTestDocument(
       "http://localhost:8080/test/", testcase.document);
-    FormAutofillContent.identifyAutofillFields(doc);
+    let element = doc.getElementById(testcase.targetElementId);
+    FormAutofillContent.identifyAutofillFields(element);
 
     Assert.deepEqual(markedFieldId, testcase.expectedResult,
       "Check the fields were marked correctly.");

browser/extensions/formautofill/test/unit/test_onFormSubmitted.js

@@ -11,6 +11,7 @@ const MOCK_DOC = MockDocument.createTestDocument("http://localhost:8080/test/",
                       <input id="tel" autocomplete="tel">
                       <input id="submit" type="submit">
                     </form>`);
+const TARGET_ELEMENT_ID = "street-addr";
 
 const TESTCASES = [
   {
@@ -110,7 +111,8 @@ TESTCASES.forEach(testcase => {
     }
     sinon.stub(FormAutofillContent, "_onFormSubmit");
 
-    FormAutofillContent.identifyAutofillFields(MOCK_DOC);
+    let element = MOCK_DOC.getElementById(TARGET_ELEMENT_ID);
+    FormAutofillContent.identifyAutofillFields(element);
     FormAutofillContent.notify(form);
 
     do_check_eq(FormAutofillContent._onFormSubmit.called,