Bug 1365895 - Select element fires events properly when autofilled. r=MattN draft
authorScott Wu <scottcwwu@gmail.com>
Tue, 13 Jun 2017 14:46:21 +0800
changeset 593091 7afa52ffa0f15369eb6f6a4fa6aa4d8e4a59cd1a
parent 592979 2a3a253806d129c0bb6f2b76bf75630457a24492
child 633020 6148aaeeaa461d2a1713d2b925b09e9972adb63b
push id63598
push userbmo:scwwu@mozilla.com
push dateTue, 13 Jun 2017 07:22:37 +0000
reviewersMattN
bugs1365895
milestone56.0a1
Bug 1365895 - Select element fires events properly when autofilled. r=MattN MozReview-Commit-ID: 6OZX9ieQlw5
browser/extensions/formautofill/FormAutofillHandler.jsm
browser/extensions/formautofill/test/mochitest/test_basic_autocomplete_form.html
browser/extensions/formautofill/test/unit/test_autofillFormFields.js
toolkit/modules/tests/modules/MockDocument.jsm
--- a/browser/extensions/formautofill/FormAutofillHandler.jsm
+++ b/browser/extensions/formautofill/FormAutofillHandler.jsm
@@ -119,21 +119,19 @@ FormAutofillHandler.prototype = {
       } else if (element instanceof Ci.nsIDOMHTMLSelectElement) {
         for (let option of element.options) {
           if (value === option.textContent || value === option.value) {
             // Do not change value if the option is already selected.
             // Use case for multiple select is not considered here.
             if (option.selected) {
               break;
             }
-            // TODO: Using dispatchEvent does not 100% simulate select change.
-            //       Should investigate further in Bug 1365895.
             option.selected = true;
-            element.dispatchEvent(new Event("input", {"bubbles": true}));
-            element.dispatchEvent(new Event("change", {"bubbles": true}));
+            element.dispatchEvent(new element.ownerGlobal.UIEvent("input", {bubbles: true}));
+            element.dispatchEvent(new element.ownerGlobal.Event("change", {bubbles: true}));
             this.changeFieldState(fieldDetail, "AUTO_FILLED");
             break;
           }
         }
       }
 
       // Unlike using setUserInput directly, FormFillController dispatches an
       // asynchronous "DOMAutoComplete" event with an "input" event follows right
--- a/browser/extensions/formautofill/test/mochitest/test_basic_autocomplete_form.html
+++ b/browser/extensions/formautofill/test/mochitest/test_basic_autocomplete_form.html
@@ -42,23 +42,32 @@ function expectPopup() {
 function popupShownListener() {
   info("popup shown for test ");
   if (expectingPopup) {
     expectingPopup();
     expectingPopup = null;
   }
 }
 
-function checkInputFilled(element, expectedvalue) {
-  return new Promise(resolve => {
-    element.addEventListener("change", function onChange() {
-      is(element.value, expectedvalue, "Checking " + element.name + " field");
-      resolve();
-    }, {once: true});
-  });
+function checkElementFilled(element, expectedvalue) {
+  return [
+    new Promise(resolve => {
+      element.addEventListener("input", function onInput() {
+        ok(true, "Checking " + element.name + " field fires input event");
+        resolve();
+      }, {once: true});
+    }),
+    new Promise(resolve => {
+      element.addEventListener("change", function onChange() {
+        ok(true, "Checking " + element.name + " field fires change event");
+        is(element.value, expectedvalue, "Checking " + element.name + " field");
+        resolve();
+      }, {once: true});
+    }),
+  ];
 }
 
 function checkAutoCompleteInputFilled(element, expectedvalue) {
   return new Promise(resolve => {
     element.addEventListener("DOMAutoComplete", function onChange() {
       is(element.value, expectedvalue, "Checking " + element.name + " field");
       resolve();
     }, {once: true});
@@ -67,17 +76,17 @@ function checkAutoCompleteInputFilled(el
 
 function checkFormFilled(address) {
   let promises = [];
   for (let prop in address) {
     let element = document.getElementById(prop);
     if (document.activeElement == element) {
       promises.push(checkAutoCompleteInputFilled(element, address[prop]));
     } else {
-      promises.push(checkInputFilled(element, address[prop]));
+      promises.push(...checkElementFilled(element, address[prop]));
     }
   }
   doKey("return");
   return Promise.all(promises);
 }
 
 async function setupAddressStorage() {
   await addAddress(MOCK_STORAGE[0]);
--- a/browser/extensions/formautofill/test/unit/test_autofillFormFields.js
+++ b/browser/extensions/formautofill/test/unit/test_autofillFormFields.js
@@ -268,51 +268,62 @@ function do_test(testcases, testFn) {
         handler.fieldDetails.forEach((field, index) => {
           let element = doc.querySelectorAll("input, select")[index];
           field.elementWeakRef = Cu.getWeakReference(element);
           if (!testcase.profileData[field.fieldName]) {
             // Avoid waiting for `change` event of a input with a blank value to
             // be filled.
             return;
           }
-          promises.push(testFn(testcase, element));
+          promises.push(...testFn(testcase, element));
         });
 
         handler.autofillFormFields(testcase.profileData);
         Assert.equal(handler.filledProfileGUID, testcase.profileData.guid,
                      "Check if filledProfileGUID is set correctly");
         await Promise.all(promises);
       });
     })();
   }
 }
 
 do_test(TESTCASES, (testcase, element) => {
-  return new Promise(resolve => {
-    element.addEventListener("change", () => {
-      let id = element.id;
-      Assert.equal(element.value, testcase.expectedResult[id],
-                  "Check the " + id + " field was filled with correct data");
-      resolve();
-    }, {once: true});
-  });
+  let id = element.id;
+  return [
+    new Promise(resolve => {
+      element.addEventListener("input", () => {
+        Assert.ok(true, "Checking " + id + " field fires input event");
+        resolve();
+      }, {once: true});
+    }),
+    new Promise(resolve => {
+      element.addEventListener("change", () => {
+        Assert.ok(true, "Checking " + id + " field fires change event");
+        Assert.equal(element.value, testcase.expectedResult[id],
+                    "Check the " + id + " field was filled with correct data");
+        resolve();
+      }, {once: true});
+    }),
+  ];
 });
 
 do_test(TESTCASES_INPUT_UNCHANGED, (testcase, element) => {
-  return new Promise((resolve, reject) => {
-    // Make sure no change or input event is fired when no change occurs.
-    let cleaner;
-    let timer = setTimeout(() => {
-      let id = element.id;
-      element.removeEventListener("change", cleaner);
-      element.removeEventListener("input", cleaner);
-      Assert.equal(element.value, testcase.expectedResult[id],
-                  "Check no value is changed on the " + id + " field");
-      resolve();
-    }, 1000);
-    cleaner = event => {
-      clearTimeout(timer);
-      reject(`${event.type} event should not fire`);
-    };
-    element.addEventListener("change", cleaner);
-    element.addEventListener("input", cleaner);
-  });
+  return [
+    new Promise((resolve, reject) => {
+      // Make sure no change or input event is fired when no change occurs.
+      let cleaner;
+      let timer = setTimeout(() => {
+        let id = element.id;
+        element.removeEventListener("change", cleaner);
+        element.removeEventListener("input", cleaner);
+        Assert.equal(element.value, testcase.expectedResult[id],
+                    "Check no value is changed on the " + id + " field");
+        resolve();
+      }, 1000);
+      cleaner = event => {
+        clearTimeout(timer);
+        reject(`${event.type} event should not fire`);
+      };
+      element.addEventListener("change", cleaner);
+      element.addEventListener("input", cleaner);
+    }),
+  ];
 });
--- a/toolkit/modules/tests/modules/MockDocument.jsm
+++ b/toolkit/modules/tests/modules/MockDocument.jsm
@@ -18,23 +18,26 @@ const MockDocument = {
    * Create a document for the given URL containing the given HTML with the ownerDocument of all <form>s having a mocked location.
    */
   createTestDocument(aDocumentURL, aContent = "<form>", aType = "text/html") {
     let parser = Cc["@mozilla.org/xmlextras/domparser;1"].
                  createInstance(Ci.nsIDOMParser);
     parser.init();
     let parsedDoc = parser.parseFromString(aContent, aType);
 
-    // Assign onwerGlobal to documentElement as well for the form-less
+    // Assign ownerGlobal to documentElement as well for the form-less
     // inputs treating it as rootElement.
     this.mockOwnerGlobalProperty(parsedDoc.documentElement);
 
-    for (let element of parsedDoc.forms) {
-      this.mockOwnerDocumentProperty(element, parsedDoc, aDocumentURL);
-      this.mockOwnerGlobalProperty(element);
+    for (let form of parsedDoc.forms) {
+      this.mockOwnerDocumentProperty(form, parsedDoc, aDocumentURL);
+      this.mockOwnerGlobalProperty(form);
+      for (let field of form.elements) {
+        this.mockOwnerGlobalProperty(field);
+      }
     }
     return parsedDoc;
   },
 
   mockOwnerDocumentProperty(aElement, aDoc, aURL) {
     // Mock the document.location object so we can unit test without a frame. We use a proxy
     // instead of just assigning to the property since it's not configurable or writable.
     let document = new Proxy(aDoc, {
@@ -57,16 +60,18 @@ const MockDocument = {
   mockOwnerGlobalProperty(aElement) {
     Object.defineProperty(aElement, "ownerGlobal", {
       value: {
         QueryInterface: XPCOMUtils.generateQI([Ci.nsIInterfaceRequestor]),
         getInterface: () => ({
           addManuallyManagedState() {},
           removeManuallyManagedState() {},
         }),
+        UIEvent: Event,
+        Event,
       },
       configurable: true,
     });
   },
 
   createTestDocumentFromFile(aDocumentURL, aFile) {
     let fileStream = Cc["@mozilla.org/network/file-input-stream;1"].
                      createInstance(Ci.nsIFileInputStream);