Bug 1382148 - Cache the matching result of select elements so we don't need to calculate it every time when previewing and filling. r=seanlee draft
authorLuke Chang <lchang@mozilla.com>
Wed, 19 Jul 2017 16:59:27 +0800
changeset 618764 ab29031a0e24fe3d8e37d28fd37d89d6f9789763
parent 618757 44121dbcac6a9d3ff18ed087a09b3205e5a04db1
child 618843 d3b9faf67b4a2a3a4ff7b6b70976989b66d7da23
push id71444
push userbmo:lchang@mozilla.com
push dateTue, 01 Aug 2017 02:31:41 +0000
reviewersseanlee
bugs1382148
milestone56.0a1
Bug 1382148 - Cache the matching result of select elements so we don't need to calculate it every time when previewing and filling. r=seanlee MozReview-Commit-ID: 1VUVEwRz5cj
browser/extensions/formautofill/FormAutofillHandler.jsm
browser/extensions/formautofill/test/unit/test_autofillFormFields.js
browser/extensions/formautofill/test/unit/test_getAdaptedProfiles.js
--- a/browser/extensions/formautofill/FormAutofillHandler.jsm
+++ b/browser/extensions/formautofill/FormAutofillHandler.jsm
@@ -150,16 +150,17 @@ FormAutofillHandler.prototype = {
 
   getFieldDetailByName(fieldName) {
     return this.fieldDetails.find(detail => detail.fieldName == fieldName);
   },
 
   _cacheValue: {
     allFieldNames: null,
     oneLineStreetAddress: null,
+    matchingSelectOption: null,
   },
 
   get allFieldNames() {
     if (!this._cacheValue.allFieldNames) {
       this._cacheValue.allFieldNames = this.fieldDetails.map(record => record.fieldName);
     }
     return this._cacheValue.allFieldNames;
   },
@@ -193,19 +194,58 @@ FormAutofillHandler.prototype = {
             profile[f] = FormAutofillUtils.toOneLineAddress(waitForConcat);
           }
           waitForConcat = [];
         }
       }
     }
   },
 
+  _matchSelectOptions(profile) {
+    if (!this._cacheValue.matchingSelectOption) {
+      this._cacheValue.matchingSelectOption = new WeakMap();
+    }
+
+    for (let fieldName in profile) {
+      let fieldDetail = this.getFieldDetailByName(fieldName);
+      if (!fieldDetail) {
+        continue;
+      }
+
+      let element = fieldDetail.elementWeakRef.get();
+      if (!(element instanceof Ci.nsIDOMHTMLSelectElement)) {
+        continue;
+      }
+
+      let cache = this._cacheValue.matchingSelectOption.get(element) || {};
+      let value = profile[fieldName];
+      if (cache[value] && cache[value].get()) {
+        continue;
+      }
+
+      let option = FormAutofillUtils.findSelectOption(element, profile, fieldName);
+      if (option) {
+        cache[value] = Cu.getWeakReference(option);
+        this._cacheValue.matchingSelectOption.set(element, cache);
+      } else {
+        if (cache[value]) {
+          delete cache[value];
+          this._cacheValue.matchingSelectOption.set(element, cache);
+        }
+        // Delete the field so the phishing hint won't treat it as a "also fill"
+        // field.
+        delete profile[fieldName];
+      }
+    }
+  },
+
   getAdaptedProfiles(originalProfiles) {
     for (let profile of originalProfiles) {
       this._addressTransformer(profile);
+      this._matchSelectOptions(profile);
     }
     return originalProfiles;
   },
 
   /**
    * Processes form fields that can be autofilled, and populates them with the
    * profile provided by backend.
    *
@@ -232,17 +272,18 @@ FormAutofillHandler.prototype = {
 
       let value = profile[fieldDetail.fieldName];
       if (element instanceof Ci.nsIDOMHTMLInputElement && !element.value && value) {
         if (element !== focusedInput) {
           element.setUserInput(value);
         }
         this.changeFieldState(fieldDetail, "AUTO_FILLED");
       } else if (element instanceof Ci.nsIDOMHTMLSelectElement) {
-        let option = FormAutofillUtils.findSelectOption(element, profile, fieldDetail.fieldName);
+        let cache = this._cacheValue.matchingSelectOption.get(element) || {};
+        let option = cache[value] && cache[value].get();
         if (!option) {
           continue;
         }
         // Do not change value or dispatch events if the option is already selected.
         // Use case for multiple select is not considered here.
         if (!option.selected) {
           option.selected = true;
           element.dispatchEvent(new element.ownerGlobal.UIEvent("input", {bubbles: true}));
@@ -321,27 +362,31 @@ FormAutofillHandler.prototype = {
       // Skip the field that is null
       if (!element) {
         continue;
       }
 
       if (element instanceof Ci.nsIDOMHTMLSelectElement) {
         // Unlike text input, select element is always previewed even if
         // the option is already selected.
-        let option = FormAutofillUtils.findSelectOption(element, profile, fieldDetail.fieldName);
-        element.previewValue = option ? option.text : "";
-        this.changeFieldState(fieldDetail, option ? "PREVIEW" : "NORMAL");
-      } else {
-        // Skip the field if it already has text entered
-        if (element.value) {
-          continue;
+        if (value) {
+          let cache = this._cacheValue.matchingSelectOption.get(element) || {};
+          let option = cache[value] && cache[value].get();
+          if (option) {
+            value = option.text || "";
+          } else {
+            value = "";
+          }
         }
-        element.previewValue = value;
-        this.changeFieldState(fieldDetail, value ? "PREVIEW" : "NORMAL");
+      } else if (element.value) {
+        // Skip the field if it already has text entered.
+        continue;
       }
+      element.previewValue = value;
+      this.changeFieldState(fieldDetail, value ? "PREVIEW" : "NORMAL");
     }
   },
 
   /**
    * Clear preview text and background highlight of all fields.
    */
   clearPreviewedFormFields() {
     log.debug("clear previewed fields in:", this.form);
--- a/browser/extensions/formautofill/test/unit/test_autofillFormFields.js
+++ b/browser/extensions/formautofill/test/unit/test_autofillFormFields.js
@@ -318,29 +318,30 @@ function do_test(testcases, testFn) {
 
         let doc = MockDocument.createTestDocument("http://localhost:8080/test/",
                                                   testcase.document);
         let form = doc.querySelector("form");
         let formLike = FormLikeFactory.createFromForm(form);
         let handler = new FormAutofillHandler(formLike);
         let promises = [];
 
-        handler.address.fieldDetails = testcase.addressFieldDetails;
+        handler.fieldDetails = handler.address.fieldDetails = testcase.addressFieldDetails;
         handler.address.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));
         });
 
-        handler.autofillFormFields(testcase.profileData);
+        let [adaptedProfile] = handler.getAdaptedProfiles([testcase.profileData]);
+        handler.autofillFormFields(adaptedProfile);
         Assert.equal(handler.address.filledRecordGUID, testcase.profileData.guid,
                      "Check if filledRecordGUID is set correctly");
         await Promise.all(promises);
       });
     })();
   }
 }
 
--- a/browser/extensions/formautofill/test/unit/test_getAdaptedProfiles.js
+++ b/browser/extensions/formautofill/test/unit/test_getAdaptedProfiles.js
@@ -7,32 +7,36 @@
 Cu.import("resource://formautofill/FormAutofillHandler.jsm");
 
 const DEFAULT_PROFILE = {
   "guid": "123",
   "street-address": "2 Harrison St\nline2\nline3",
   "address-line1": "2 Harrison St",
   "address-line2": "line2",
   "address-line3": "line3",
+  "address-level1": "CA",
+  "country": "US",
 };
 
 const TESTCASES = [
   {
     description: "Form with street-address",
     document: `<form>
                <input id="street-addr" autocomplete="street-address">
                </form>`,
     profileData: [Object.assign({}, DEFAULT_PROFILE)],
     expectedResult: [{
       "guid": "123",
       "street-address": "2 Harrison St line2 line3",
       "-moz-street-address-one-line": "2 Harrison St line2 line3",
       "address-line1": "2 Harrison St",
       "address-line2": "line2",
       "address-line3": "line3",
+      "address-level1": "CA",
+      "country": "US",
     }],
   },
   {
     description: "Form with street-address, address-line[1, 2, 3]",
     document: `<form>
                <input id="street-addr" autocomplete="street-address">
                <input id="line1" autocomplete="address-line1">
                <input id="line2" autocomplete="address-line2">
@@ -41,32 +45,36 @@ const TESTCASES = [
     profileData: [Object.assign({}, DEFAULT_PROFILE)],
     expectedResult: [{
       "guid": "123",
       "street-address": "2 Harrison St line2 line3",
       "-moz-street-address-one-line": "2 Harrison St line2 line3",
       "address-line1": "2 Harrison St",
       "address-line2": "line2",
       "address-line3": "line3",
+      "address-level1": "CA",
+      "country": "US",
     }],
   },
   {
     description: "Form with street-address, address-line1",
     document: `<form>
                <input id="street-addr" autocomplete="street-address">
                <input id="line1" autocomplete="address-line1">
                </form>`,
     profileData: [Object.assign({}, DEFAULT_PROFILE)],
     expectedResult: [{
       "guid": "123",
       "street-address": "2 Harrison St line2 line3",
       "-moz-street-address-one-line": "2 Harrison St line2 line3",
       "address-line1": "2 Harrison St line2 line3",
       "address-line2": "line2",
       "address-line3": "line3",
+      "address-level1": "CA",
+      "country": "US",
     }],
   },
   {
     description: "Form with street-address, address-line[1, 2]",
     document: `<form>
                <input id="street-addr" autocomplete="street-address">
                <input id="line1" autocomplete="address-line1">
                <input id="line2" autocomplete="address-line2">
@@ -74,16 +82,18 @@ const TESTCASES = [
     profileData: [Object.assign({}, DEFAULT_PROFILE)],
     expectedResult: [{
       "guid": "123",
       "street-address": "2 Harrison St line2 line3",
       "-moz-street-address-one-line": "2 Harrison St line2 line3",
       "address-line1": "2 Harrison St",
       "address-line2": "line2 line3",
       "address-line3": "line3",
+      "address-level1": "CA",
+      "country": "US",
     }],
   },
   {
     description: "Form with street-address, address-line[1, 3]",
     document: `<form>
                <input id="street-addr" autocomplete="street-address">
                <input id="line1" autocomplete="address-line1">
                <input id="line3" autocomplete="address-line3">
@@ -91,16 +101,129 @@ const TESTCASES = [
     profileData: [Object.assign({}, DEFAULT_PROFILE)],
     expectedResult: [{
       "guid": "123",
       "street-address": "2 Harrison St line2 line3",
       "-moz-street-address-one-line": "2 Harrison St line2 line3",
       "address-line1": "2 Harrison St",
       "address-line2": "line2 line3",
       "address-line3": "line3",
+      "address-level1": "CA",
+      "country": "US",
+    }],
+  },
+  {
+    description: "Form with exact matching options in select",
+    document: `<form>
+               <select autocomplete="address-level1">
+                 <option id="option-address-level1-XX" value="XX">Dummy</option>
+                 <option id="option-address-level1-CA" value="CA">California</option>
+               </select>
+               </form>`,
+    profileData: [Object.assign({}, DEFAULT_PROFILE)],
+    expectedResult: [{
+      "guid": "123",
+      "street-address": "2 Harrison St\nline2\nline3",
+      "-moz-street-address-one-line": "2 Harrison St line2 line3",
+      "address-line1": "2 Harrison St",
+      "address-line2": "line2",
+      "address-line3": "line3",
+      "address-level1": "CA",
+      "country": "US",
+    }],
+    expectedOptionElements: [{
+      "address-level1": "option-address-level1-CA",
+    }],
+  },
+  {
+    description: "Form with inexact matching options in select",
+    document: `<form>
+               <select autocomplete="address-level1">
+                 <option id="option-address-level1-XX" value="XX">Dummy</option>
+                 <option id="option-address-level1-OO" value="OO">California</option>
+               </select>
+               </form>`,
+    profileData: [Object.assign({}, DEFAULT_PROFILE)],
+    expectedResult: [{
+      "guid": "123",
+      "street-address": "2 Harrison St\nline2\nline3",
+      "-moz-street-address-one-line": "2 Harrison St line2 line3",
+      "address-line1": "2 Harrison St",
+      "address-line2": "line2",
+      "address-line3": "line3",
+      "address-level1": "CA",
+      "country": "US",
+    }],
+    expectedOptionElements: [{
+      "address-level1": "option-address-level1-OO",
+    }],
+  },
+  {
+    description: "Form with value-omitted options in select",
+    document: `<form>
+               <select autocomplete="address-level1">
+                 <option id="option-address-level1-1" value="">Dummy</option>
+                 <option id="option-address-level1-2" value="">California</option>
+               </select>
+               </form>`,
+    profileData: [Object.assign({}, DEFAULT_PROFILE)],
+    expectedResult: [{
+      "guid": "123",
+      "street-address": "2 Harrison St\nline2\nline3",
+      "-moz-street-address-one-line": "2 Harrison St line2 line3",
+      "address-line1": "2 Harrison St",
+      "address-line2": "line2",
+      "address-line3": "line3",
+      "address-level1": "CA",
+      "country": "US",
+    }],
+    expectedOptionElements: [{
+      "address-level1": "option-address-level1-2",
+    }],
+  },
+  {
+    description: "Form with options with the same value in select",
+    document: `<form>
+               <select autocomplete="address-level1">
+                 <option id="option-address-level1-same1" value="same">Dummy</option>
+                 <option id="option-address-level1-same2" value="same">California</option>
+               </select>
+               </form>`,
+    profileData: [Object.assign({}, DEFAULT_PROFILE)],
+    expectedResult: [{
+      "guid": "123",
+      "street-address": "2 Harrison St\nline2\nline3",
+      "-moz-street-address-one-line": "2 Harrison St line2 line3",
+      "address-line1": "2 Harrison St",
+      "address-line2": "line2",
+      "address-line3": "line3",
+      "address-level1": "CA",
+      "country": "US",
+    }],
+    expectedOptionElements: [{
+      "address-level1": "option-address-level1-same2",
+    }],
+  },
+  {
+    description: "Form without matching options in select",
+    document: `<form>
+               <select autocomplete="address-level1">
+                 <option id="option-address-level1-dummy1" value="">Dummy</option>
+                 <option id="option-address-level1-dummy2" value="">Dummy 2</option>
+               </select>
+               </form>`,
+    profileData: [Object.assign({}, DEFAULT_PROFILE)],
+    expectedResult: [{
+      "guid": "123",
+      "street-address": "2 Harrison St\nline2\nline3",
+      "-moz-street-address-one-line": "2 Harrison St line2 line3",
+      "address-line1": "2 Harrison St",
+      "address-line2": "line2",
+      "address-line3": "line3",
+      "country": "US",
     }],
   },
 ];
 
 for (let testcase of TESTCASES) {
   add_task(async function() {
     do_print("Starting testcase: " + testcase.description);
 
@@ -108,11 +231,27 @@ for (let testcase of TESTCASES) {
                                               testcase.document);
     let form = doc.querySelector("form");
     let formLike = FormLikeFactory.createFromForm(form);
     let handler = new FormAutofillHandler(formLike);
 
     handler.collectFormFields();
     let adaptedAddresses = handler.getAdaptedProfiles(testcase.profileData);
     Assert.deepEqual(adaptedAddresses, testcase.expectedResult);
+
+    if (testcase.expectedOptionElements) {
+      testcase.expectedOptionElements.forEach((expectedOptionElement, i) => {
+        for (let field in expectedOptionElement) {
+          let select = form.querySelector(`[autocomplete=${field}]`);
+          let expectedOption = doc.getElementById(expectedOptionElement[field]);
+          Assert.notEqual(expectedOption, null);
+
+          let value = testcase.profileData[i][field];
+          let cache = handler._cacheValue.matchingSelectOption.get(select);
+          let targetOption = cache[value] && cache[value].get();
+          Assert.notEqual(targetOption, null);
+
+          Assert.equal(targetOption, expectedOption);
+        }
+      });
+    }
   });
 }
-