Bug 1358943 - [Form Autofill] Support "address-line*" fields. r=MattN draft
authorLuke Chang <lchang@mozilla.com>
Mon, 24 Apr 2017 10:58:37 +0800
changeset 569775 1f82ba9b4d779dd148f63eea1e18dc85375ee132
parent 569701 2cca333f546f38860f84940d4c72d7470a3410f4
child 569793 3e82501bd00aaf2f88973e7621e96fb134f554e9
push id56278
push userbmo:lchang@mozilla.com
push dateThu, 27 Apr 2017 22:30:48 +0000
reviewersMattN
bugs1358943
milestone55.0a1
Bug 1358943 - [Form Autofill] Support "address-line*" fields. r=MattN MozReview-Commit-ID: 2Kut671GHQh
browser/extensions/formautofill/FormAutofillHeuristics.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/unit/test_transformFields.js
browser/extensions/formautofill/test/unit/xpcshell.ini
--- a/browser/extensions/formautofill/FormAutofillHeuristics.jsm
+++ b/browser/extensions/formautofill/FormAutofillHeuristics.jsm
@@ -17,16 +17,19 @@ const {classes: Cc, interfaces: Ci, util
  */
 this.FormAutofillHeuristics = {
   VALID_FIELDS: [
     "given-name",
     "additional-name",
     "family-name",
     "organization",
     "street-address",
+    "address-line1",
+    "address-line2",
+    "address-line3",
     "address-level2",
     "address-level1",
     "postal-code",
     "country",
     "tel",
     "email",
   ],
 
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -1,37 +1,44 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /*
  * Implements an interface of the storage of Form Autofill.
  *
- * The data is stored in JSON format, without indentation, using UTF-8 encoding.
- * With indentation applied, the file would look like this:
+ * The data is stored in JSON format, without indentation and the computed
+ * fields, using UTF-8 encoding. With indentation and computed fields applied,
+ * the schema would look like this:
  *
  * {
  *   version: 1,
  *   profiles: [
  *     {
  *       guid,             // 12 character...
  *
  *       // profile
  *       given-name,
  *       additional-name,
  *       family-name,
  *       organization,     // Company
- *       street-address,    // (Multiline)
- *       address-level2,    // City/Town
- *       address-level1,    // Province (Standardized code if possible)
+ *       street-address,   // (Multiline)
+ *       address-level2,   // City/Town
+ *       address-level1,   // Province (Standardized code if possible)
  *       postal-code,
  *       country,          // ISO 3166
  *       tel,
  *       email,
  *
+ *       // computed fields (These fields are not stored in the file as they are
+ *       // generated at runtime.)
+ *       address-line1,
+ *       address-line2,
+ *       address-line3,
+ *
  *       // metadata
  *       timeCreated,      // in ms
  *       timeLastUsed,     // in ms
  *       timeLastModified, // in ms
  *       timesUsed
  *     },
  *     {
  *       // ...
@@ -61,17 +68,16 @@ XPCOMUtils.defineLazyServiceGetter(this,
                                    "@mozilla.org/uuid-generator;1",
                                    "nsIUUIDGenerator");
 
 this.log = null;
 FormAutofillUtils.defineLazyLogGetter(this, this.EXPORTED_SYMBOLS[0]);
 
 const SCHEMA_VERSION = 1;
 
-// Name-related fields will be handled in follow-up bugs due to the complexity.
 const VALID_FIELDS = [
   "given-name",
   "additional-name",
   "family-name",
   "organization",
   "street-address",
   "address-level2",
   "address-level1",
@@ -123,17 +129,18 @@ ProfileStorage.prototype = {
    *
    * @param {Profile} profile
    *        The new profile for saving.
    */
   add(profile) {
     log.debug("add:", profile);
     this._store.ensureDataReady();
 
-    let profileToSave = this._normalizeProfile(profile);
+    let profileToSave = this._clone(profile);
+    this._normalizeProfile(profileToSave);
 
     profileToSave.guid = gUUIDGenerator.generateUUID().toString()
                                        .replace(/[{}-]/g, "").substring(0, 12);
 
     // Metadata
     let now = Date.now();
     profileToSave.timeCreated = now;
     profileToSave.timeLastModified = now;
@@ -158,17 +165,19 @@ ProfileStorage.prototype = {
     log.debug("update:", guid, profile);
     this._store.ensureDataReady();
 
     let profileFound = this._findByGUID(guid);
     if (!profileFound) {
       throw new Error("No matching profile.");
     }
 
-    let profileToUpdate = this._normalizeProfile(profile);
+    let profileToUpdate = this._clone(profile);
+    this._normalizeProfile(profileToUpdate);
+
     for (let field of VALID_FIELDS) {
       if (profileToUpdate[field] !== undefined) {
         profileFound[field] = profileToUpdate[field];
       } else {
         delete profileFound[field];
       }
     }
 
@@ -229,88 +238,122 @@ ProfileStorage.prototype = {
     this._store.ensureDataReady();
 
     let profileFound = this._findByGUID(guid);
     if (!profileFound) {
       throw new Error("No matching profile.");
     }
 
     // Profile is cloned to avoid accidental modifications from outside.
-    return this._clone(profileFound);
+    let clonedProfile = this._clone(profileFound);
+    this._computeFields(clonedProfile);
+    return clonedProfile;
   },
 
   /**
    * Returns all profiles.
    *
    * @returns {Array.<Profile>}
    *          An array containing clones of all profiles.
    */
   getAll() {
     log.debug("getAll");
     this._store.ensureDataReady();
 
     // Profiles are cloned to avoid accidental modifications from outside.
-    return this._store.data.profiles.map(this._clone);
+    let clonedProfiles = this._store.data.profiles.map(this._clone);
+    clonedProfiles.forEach(this._computeFields);
+    return clonedProfiles;
   },
 
   /**
    * Returns the filtered profiles based on input's information and searchString.
    *
    * @returns {Array.<Profile>}
    *          An array containing clones of matched profiles.
    */
   getByFilter({info, searchString}) {
     log.debug("getByFilter:", info, searchString);
-    this._store.ensureDataReady();
+
+    let lcSearchString = searchString.toLowerCase();
+    let result = this.getAll().filter(profile => {
+      // Return true if string is not provided and field exists.
+      // TODO: We'll need to check if the address is for billing or shipping.
+      //       (Bug 1358941)
+      let name = profile[info.fieldName];
 
-    // Profiles are cloned to avoid accidental modifications from outside.
-    let result = this._findByFilter({info, searchString}).map(this._clone);
+      if (!searchString) {
+        return !!name;
+      }
+
+      return name.toLowerCase().startsWith(lcSearchString);
+    });
+
     log.debug("getByFilter: Returning", result.length, "result(s)");
     return result;
   },
 
   _clone(profile) {
     return Object.assign({}, profile);
   },
 
   _findByGUID(guid) {
     return this._store.data.profiles.find(profile => profile.guid == guid);
   },
 
-  _findByFilter({info, searchString}) {
-    let profiles = this._store.data.profiles;
-    let lcSearchString = searchString.toLowerCase();
+  _computeFields(profile) {
+    if (profile["street-address"]) {
+      let streetAddress = profile["street-address"].split("\n");
+      // TODO: we should prevent the dataloss by concatenating the rest of lines
+      //       with a locale-specific character in the future (bug 1360114).
+      for (let i = 0; i < 3; i++) {
+        if (streetAddress[i]) {
+          profile["address-line" + (i + 1)] = streetAddress[i];
+        }
+      }
+    }
+  },
 
-    return profiles.filter(profile => {
-      // Return true if string is not provided and field exists.
-      // TODO: We'll need to check if the address is for billing or shipping.
-      let name = profile[info.fieldName];
-
-      if (!searchString) {
-        return !!name;
+  _normalizeAddress(profile) {
+    if (profile["address-line1"] || profile["address-line2"] ||
+        profile["address-line3"]) {
+      // Treat "street-address" as "address-line1" if it contains only one line
+      // and "address-line1" is omitted.
+      if (!profile["address-line1"] && profile["street-address"] &&
+          !profile["street-address"].includes("\n")) {
+        profile["address-line1"] = profile["street-address"];
+        delete profile["street-address"];
       }
 
-      return name.toLowerCase().startsWith(lcSearchString);
-    });
+      // Remove "address-line*" but keep the values.
+      let addressLines = [1, 2, 3].map(i => {
+        let value = profile["address-line" + i];
+        delete profile["address-line" + i];
+        return value;
+      });
+
+      // Concatenate "address-line*" if "street-address" is omitted.
+      if (!profile["street-address"]) {
+        profile["street-address"] = addressLines.join("\n");
+      }
+    }
   },
 
   _normalizeProfile(profile) {
-    let result = {};
+    this._normalizeAddress(profile);
+
     for (let key in profile) {
       if (!VALID_FIELDS.includes(key)) {
         throw new Error(`"${key}" is not a valid field.`);
       }
       if (typeof profile[key] !== "string" &&
           typeof profile[key] !== "number") {
         throw new Error(`"${key}" contains invalid data type.`);
       }
-
-      result[key] = profile[key];
     }
-    return result;
   },
 
   _dataPostProcessor(data) {
     data.version = SCHEMA_VERSION;
     if (!data.profiles) {
       data.profiles = MOCK_MODE ? MOCK_STORAGE : [];
     }
     return data;
new file mode 100644
--- /dev/null
+++ b/browser/extensions/formautofill/test/unit/test_transformFields.js
@@ -0,0 +1,173 @@
+/**
+ * Tests the transform algorithm in profileStorage.
+ */
+
+"use strict";
+
+Cu.import("resource://gre/modules/Task.jsm");
+Cu.import("resource://formautofill/ProfileStorage.jsm");
+
+const TEST_STORE_FILE_NAME = "test-profile.json";
+
+const COMPUTE_TESTCASES = [
+  // Empty
+  {
+    description: "Empty profile",
+    profile: {
+    },
+    expectedResult: {
+    },
+  },
+
+  // Address
+  {
+    description: "\"street-address\" with single line",
+    profile: {
+      "street-address": "single line",
+    },
+    expectedResult: {
+      "street-address": "single line",
+      "address-line1": "single line",
+    },
+  },
+  {
+    description: "\"street-address\" with multiple lines",
+    profile: {
+      "street-address": "line1\nline2\nline3",
+    },
+    expectedResult: {
+      "street-address": "line1\nline2\nline3",
+      "address-line1": "line1",
+      "address-line2": "line2",
+      "address-line3": "line3",
+    },
+  },
+  {
+    description: "\"street-address\" with multiple lines but line2 is omitted",
+    profile: {
+      "street-address": "line1\n\nline3",
+    },
+    expectedResult: {
+      "street-address": "line1\n\nline3",
+      "address-line1": "line1",
+      "address-line2": "",
+      "address-line3": "line3",
+    },
+  },
+  {
+    description: "\"street-address\" with 4 lines",
+    profile: {
+      "street-address": "line1\nline2\nline3\nline4",
+    },
+    expectedResult: {
+      "street-address": "line1\nline2\nline3\nline4",
+      "address-line1": "line1",
+      "address-line2": "line2",
+      "address-line3": "line3",
+    },
+  },
+];
+
+const NORMALIZE_TESTCASES = [
+  // Empty
+  {
+    description: "Empty profile",
+    profile: {
+    },
+    expectedResult: {
+    },
+  },
+
+  // Address
+  {
+    description: "Has \"address-line1~3\" and \"street-address\" is omitted",
+    profile: {
+      "address-line1": "line1",
+      "address-line2": "line2",
+      "address-line3": "line3",
+    },
+    expectedResult: {
+      "street-address": "line1\nline2\nline3",
+    },
+  },
+  {
+    description: "Has both \"address-line1~3\" and \"street-address\"",
+    profile: {
+      "street-address": "street address",
+      "address-line1": "line1",
+      "address-line2": "line2",
+      "address-line3": "line3",
+    },
+    expectedResult: {
+      "street-address": "street address",
+    },
+  },
+  {
+    description: "Has \"address-line2~3\" and single-line \"street-address\"",
+    profile: {
+      "street-address": "street address",
+      "address-line2": "line2",
+      "address-line3": "line3",
+    },
+    expectedResult: {
+      "street-address": "street address\nline2\nline3",
+    },
+  },
+  {
+    description: "Has \"address-line2~3\" and multiple-line \"street-address\"",
+    profile: {
+      "street-address": "street address\nstreet address line 2",
+      "address-line2": "line2",
+      "address-line3": "line3",
+    },
+    expectedResult: {
+      "street-address": "street address\nstreet address line 2",
+    },
+  },
+];
+
+let do_check_profile_matches = (expectedProfile, profile) => {
+  for (let key in expectedProfile) {
+    do_check_eq(expectedProfile[key], profile[key] || "");
+  }
+};
+
+add_task(function* test_computeFields() {
+  let path = getTempFile(TEST_STORE_FILE_NAME).path;
+
+  let profileStorage = new ProfileStorage(path);
+  yield profileStorage.initialize();
+
+  COMPUTE_TESTCASES.forEach(testcase => profileStorage.add(testcase.profile));
+  yield profileStorage._saveImmediately();
+
+  profileStorage = new ProfileStorage(path);
+  yield profileStorage.initialize();
+
+  let profiles = profileStorage.getAll();
+
+  for (let i in profiles) {
+    do_print("Verify testcase: " + COMPUTE_TESTCASES[i].description);
+    do_check_profile_matches(COMPUTE_TESTCASES[i].expectedResult, profiles[i]);
+  }
+});
+
+add_task(function* test_normalizeProfile() {
+  let path = getTempFile(TEST_STORE_FILE_NAME).path;
+
+  let profileStorage = new ProfileStorage(path);
+  yield profileStorage.initialize();
+
+  NORMALIZE_TESTCASES.forEach(testcase => profileStorage.add(testcase.profile));
+  yield profileStorage._saveImmediately();
+
+  profileStorage = new ProfileStorage(path);
+  yield profileStorage.initialize();
+
+  let profiles = profileStorage.getAll();
+
+  for (let i in profiles) {
+    do_print("Verify testcase: " + NORMALIZE_TESTCASES[i].description);
+    do_check_profile_matches(NORMALIZE_TESTCASES[i].expectedResult, profiles[i]);
+  }
+});
--- a/browser/extensions/formautofill/test/unit/xpcshell.ini
+++ b/browser/extensions/formautofill/test/unit/xpcshell.ini
@@ -10,8 +10,9 @@ support-files =
 [test_getFormInputDetails.js]
 [test_isCJKName.js]
 [test_markAsAutofillField.js]
 [test_nameUtils.js]
 [test_onFormSubmitted.js]
 [test_profileAutocompleteResult.js]
 [test_profileStorage.js]
 [test_savedFieldNames.js]
+[test_transformFields.js]