Bug 1423805 - Handle auto-filled input correctness upon target set instead of section-level. r=lchang draft
authorRay Lin <ralin@mozilla.com>
Thu, 07 Dec 2017 15:09:36 +0800
changeset 713296 6c1d7e33f5df75c8dd64424c62a5e839e13af6ee
parent 713235 1624b88874765bf57e9feba176d30149c748d9d2
child 744314 beccce15fb9861b12918087301fb76cc2ed5fba1
push id93619
push userbmo:ralin@mozilla.com
push dateWed, 20 Dec 2017 04:21:54 +0000
reviewerslchang
bugs1423805
milestone59.0a1
Bug 1423805 - Handle auto-filled input correctness upon target set instead of section-level. r=lchang MozReview-Commit-ID: K79dmtJ2aFy
browser/extensions/formautofill/FormAutofillHandler.jsm
browser/extensions/formautofill/test/mochitest/mochitest.ini
browser/extensions/formautofill/test/mochitest/test_clear_form.html
--- a/browser/extensions/formautofill/FormAutofillHandler.jsm
+++ b/browser/extensions/formautofill/FormAutofillHandler.jsm
@@ -433,19 +433,16 @@ class FormAutofillSection {
         if (!option.selected) {
           option.selected = true;
           element.dispatchEvent(new element.ownerGlobal.UIEvent("input", {bubbles: true}));
           element.dispatchEvent(new element.ownerGlobal.Event("change", {bubbles: true}));
         }
         // Autofill highlight appears regardless if value is changed or not
         this._changeFieldState(fieldDetail, FIELD_STATES.AUTO_FILLED);
       }
-      if (fieldDetail.state == FIELD_STATES.AUTO_FILLED) {
-        element.addEventListener("input", this, {mozSystemGroup: true});
-      }
     }
   }
 
   /**
    * Populates result to the preview layers with given profile.
    *
    * @param {Object} profile
    *        A profile to be previewed with
@@ -529,17 +526,16 @@ class FormAutofillSection {
       }
 
       // Only reset value for input element.
       if (fieldDetail.state == FIELD_STATES.AUTO_FILLED &&
           element instanceof Ci.nsIDOMHTMLInputElement) {
         element.setUserInput("");
       }
     }
-    this.resetFieldStates();
   }
 
   /**
    * Change the state of a field to correspond with different presentations.
    *
    * @param {Object} fieldDetail
    *        A fieldDetail of which its element is about to update the state.
    * @param {string} nextState
@@ -551,31 +547,47 @@ class FormAutofillSection {
     if (!element) {
       log.warn(fieldDetail.fieldName, "is unreachable while changing state");
       return;
     }
     if (!(nextState in this._FIELD_STATE_ENUM)) {
       log.warn(fieldDetail.fieldName, "is trying to change to an invalid state");
       return;
     }
+    if (fieldDetail.state == nextState) {
+      return;
+    }
 
     for (let [state, mmStateValue] of Object.entries(this._FIELD_STATE_ENUM)) {
       // The NORMAL state is simply the absence of other manually
       // managed states so we never need to add or remove it.
       if (!mmStateValue) {
         continue;
       }
 
       if (state == nextState) {
         this.winUtils.addManuallyManagedState(element, mmStateValue);
       } else {
         this.winUtils.removeManuallyManagedState(element, mmStateValue);
       }
     }
 
+    switch (nextState) {
+      case FIELD_STATES.NORMAL: {
+        if (fieldDetail.state == FIELD_STATES.AUTO_FILLED) {
+          element.removeEventListener("input", this, {mozSystemGroup: true});
+        }
+        break;
+      }
+      case FIELD_STATES.AUTO_FILLED: {
+        element.addEventListener("input", this, {mozSystemGroup: true});
+        break;
+      }
+    }
+
     fieldDetail.state = nextState;
   }
 
   resetFieldStates() {
     for (let fieldDetail of this._validDetails) {
       const element = fieldDetail.elementWeakRef.get();
       element.removeEventListener("input", this, {mozSystemGroup: true});
       this._changeFieldState(fieldDetail, FIELD_STATES.NORMAL);
@@ -758,24 +770,42 @@ class FormAutofillSection {
 
   handleEvent(event) {
     switch (event.type) {
       case "input": {
         if (!event.isTrusted) {
           return;
         }
         const target = event.target;
-        const fieldDetail = this.getFieldDetailByElement(target);
+        const targetFieldDetail = this.getFieldDetailByElement(target);
         const targetSet = this._getTargetSet(target);
-        this._changeFieldState(fieldDetail, FIELD_STATES.NORMAL);
+
+        this._changeFieldState(targetFieldDetail, FIELD_STATES.NORMAL);
+
+        let isAutofilled = false;
+        let dimFieldDetails = [];
+        for (const fieldDetail of targetSet.fieldDetails) {
+          const element = fieldDetail.elementWeakRef.get();
 
-        if (!targetSet.fieldDetails.some(detail => detail.state == FIELD_STATES.AUTO_FILLED)) {
+          if (ChromeUtils.getClassName(element) === "HTMLSelectElement") {
+            // Dim fields are those we don't attempt to revert their value
+            // when clear the target set, such as <select>.
+            dimFieldDetails.push(fieldDetail);
+          } else {
+            isAutofilled |= fieldDetail.state == FIELD_STATES.AUTO_FILLED;
+          }
+        }
+        if (!isAutofilled) {
+          // Restore the dim fields to initial state as well once we knew
+          // that user had intention to clear the filled form manually.
+          for (const fieldDetail of dimFieldDetails) {
+            this._changeFieldState(fieldDetail, FIELD_STATES.NORMAL);
+          }
           targetSet.filledRecordGUID = null;
         }
-        target.removeEventListener("input", this, {mozSystemGroup: true});
         break;
       }
     }
   }
 }
 
 /**
  * Handles profile autofill for a DOM Form element.
--- a/browser/extensions/formautofill/test/mochitest/mochitest.ini
+++ b/browser/extensions/formautofill/test/mochitest/mochitest.ini
@@ -7,16 +7,17 @@ support-files =
   formautofill_common.js
   formautofill_parent_utils.js
 
 [test_autofocus_form.html]
 [test_basic_autocomplete_form.html]
 [test_basic_creditcard_autocomplete_form.html]
 scheme=https
 [test_clear_form.html]
+scheme=https
 [test_creditcard_autocomplete_off.html]
 scheme=https
 [test_form_changes.html]
 [test_formautofill_preview_highlight.html]
 skip-if = webrender # bug 1424752
 [test_multi_locale_CA_address_form.html]
 [test_multiple_forms.html]
 [test_on_address_submission.html]
--- a/browser/extensions/formautofill/test/mochitest/test_clear_form.html
+++ b/browser/extensions/formautofill/test/mochitest/test_clear_form.html
@@ -14,86 +14,132 @@ Form autofill test: clear form button
 
 <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";
 
-const MOCK_STORAGE = [{
+const MOCK_ADDR_STORAGE = [{
   organization: "Sesame Street",
   "street-address": "2 Harrison St\nline2\nline3",
   tel: "+13453453456",
 }, {
   organization: "Mozilla",
   "street-address": "331 E. Evelyn Avenue",
 }, {
   organization: "Tel org",
   tel: "+12223334444",
 }];
+const MOCK_CC_STORAGE = [{
+  "cc-name": "John Doe",
+  "cc-number": "1234567812345678",
+  "cc-exp-month": 4,
+  "cc-exp-year": 2017,
+}, {
+  "cc-name": "Timothy Berners-Lee",
+  "cc-number": "1111222233334444",
+  "cc-exp-month": 12,
+  "cc-exp-year": 2022,
+}];
 
 initPopupListener();
 
 add_task(async function setup_storage() {
-  await addAddress(MOCK_STORAGE[0]);
-  await addAddress(MOCK_STORAGE[1]);
-  await addAddress(MOCK_STORAGE[2]);
+  await addAddress(MOCK_ADDR_STORAGE[0]);
+  await addAddress(MOCK_ADDR_STORAGE[1]);
+  await addAddress(MOCK_ADDR_STORAGE[2]);
+
+  await addCreditCard(MOCK_CC_STORAGE[0]);
+  await addCreditCard(MOCK_CC_STORAGE[1]);
 });
 
+
 function checkIsFormCleared(patch = {}) {
   const form = document.getElementById("form1");
 
   for (const elem of form.elements) {
     const expectedValue = patch[elem.id] || "";
-    is(elem.value, expectedValue, "checking value");
+    checkFieldValue(elem, expectedValue);
     checkFieldHighlighted(elem, false);
     checkFieldPreview(elem, "");
   }
 }
 
 add_task(async function simple_clear() {
   await triggerPopupAndHoverItem("#organization", 0);
-  await triggerAutofillAndCheckProfile(MOCK_STORAGE[0]);
+  await triggerAutofillAndCheckProfile(MOCK_ADDR_STORAGE[0]);
 
   await triggerPopupAndHoverItem("#tel", 0);
   doKey("return");
   checkIsFormCleared();
 });
 
 add_task(async function clear_adapted_record() {
   await triggerPopupAndHoverItem("#street-address", 0);
-  await triggerAutofillAndCheckProfile(MOCK_STORAGE[0]);
+  await triggerAutofillAndCheckProfile(MOCK_ADDR_STORAGE[0]);
 
   await triggerPopupAndHoverItem("#street-address", 0);
   doKey("return");
   checkIsFormCleared();
 });
 
 add_task(async function clear_modified_form() {
   await triggerPopupAndHoverItem("#organization", 0);
-  await triggerAutofillAndCheckProfile(MOCK_STORAGE[0]);
+  await triggerAutofillAndCheckProfile(MOCK_ADDR_STORAGE[0]);
 
   await setInput("#tel", "+1111111111", true);
 
   await triggerPopupAndHoverItem("#street-address", 0);
   doKey("return");
   checkIsFormCleared({tel: "+1111111111"});
 });
+
+add_task(async function clear_distinct_section() {
+  document.getElementById("form1").reset();
+  await triggerPopupAndHoverItem("#cc-name", 0);
+  await triggerAutofillAndCheckProfile(MOCK_CC_STORAGE[0]);
+
+  await triggerPopupAndHoverItem("#organization", 0);
+  await triggerAutofillAndCheckProfile(MOCK_ADDR_STORAGE[0]);
+  await triggerPopupAndHoverItem("#street-address", 0);
+  doKey("return");
+
+  for (const [id, val] of Object.entries(MOCK_CC_STORAGE[0])) {
+    const element = document.getElementById(id);
+    if (!element) {
+      return;
+    }
+    checkFieldValue(element, val);
+    checkFieldHighlighted(element, true);
+  }
+
+  await triggerPopupAndHoverItem("#cc-name", 0);
+  doKey("return");
+  checkIsFormCleared();
+});
+
 </script>
 
 <p id="display"></p>
 
 <div id="content">
 
   <form id="form1">
     <p>This is a basic form.</p>
     <p><label>organization: <input id="organization" autocomplete="organization"></label></p>
     <p><label>streetAddress: <input id="street-address" autocomplete="street-address"></label></p>
     <p><label>tel: <input id="tel" autocomplete="tel"></label></p>
     <p><label>country: <input id="country" autocomplete="country"></label></p>
+
+    <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>
 
 </div>
 
 <pre id="test"></pre>
 </body>
 </html>