Bug 1402963 - Part 1: Deduplicate credit card by checking credit card storage and untouched fields. r=lchang draft
authorsteveck-chung <schung@mozilla.com>
Wed, 27 Sep 2017 18:38:13 +0800
changeset 693946 14fe87b077a88bd6aee3445ff6c5dc70a5e92a32
parent 693830 c2fe4b3b1b930b3e7fdb84eae44cec165394f322
child 693947 a6d7fb501e43b3e388697c170e94463c44664ff3
child 693983 14d71984b75e814fdef13261f1879c02a20f1c19
push id87998
push userbmo:schung@mozilla.com
push dateTue, 07 Nov 2017 07:08:12 +0000
reviewerslchang
bugs1402963
milestone58.0a1
Bug 1402963 - Part 1: Deduplicate credit card by checking credit card storage and untouched fields. r=lchang MozReview-Commit-ID: tuw36eyQnl
browser/extensions/formautofill/FormAutofillParent.jsm
browser/extensions/formautofill/ProfileStorage.jsm
browser/extensions/formautofill/test/browser/browser_creditCard_doorhanger.js
browser/extensions/formautofill/test/browser/head.js
--- a/browser/extensions/formautofill/FormAutofillParent.jsm
+++ b/browser/extensions/formautofill/FormAutofillParent.jsm
@@ -426,34 +426,49 @@ FormAutofillParent.prototype = {
         Services.telemetry.scalarAdd("formautofill.addresses.fill_type_manual", 1);
       }
     }
   },
 
   async _onCreditCardSubmit(creditCard, target, timeStartedFillingMS) {
     // We'll show the credit card doorhanger if:
     //   - User applys autofill and changed
-    //   - User fills form manually
-    if (creditCard.guid &&
-        Object.keys(creditCard.record).every(key => creditCard.untouchedFields.includes(key))) {
-      // Add probe to record credit card autofill(without modification).
-      Services.telemetry.scalarAdd("formautofill.creditCards.fill_type_autofill", 1);
-      this._recordFormFillingTime("creditCard", "autofill", timeStartedFillingMS);
-      return;
-    }
+    //   - User fills form manually and the filling data is not duplicated to storage
+    if (creditCard.guid) {
+      let originalCCData = this.profileStorage.creditCards.get(creditCard.guid);
+      let unchanged = Object.keys(creditCard.record).every(field => {
+        if (creditCard.record[field] === "" && !originalCCData[field]) {
+          return true;
+        }
+        return creditCard.untouchedFields.includes(field);
+      });
 
-    // Add the probe to record credit card manual filling or autofill but modified case.
-    if (creditCard.guid) {
+      if (unchanged) {
+        this.profileStorage.creditCards.notifyUsed(creditCard.guid);
+        // Add probe to record credit card autofill(without modification).
+        Services.telemetry.scalarAdd("formautofill.creditCards.fill_type_autofill", 1);
+        this._recordFormFillingTime("creditCard", "autofill", timeStartedFillingMS);
+        return;
+      }
+      // Add the probe to record credit card autofill with modification.
       Services.telemetry.scalarAdd("formautofill.creditCards.fill_type_autofill_modified", 1);
       this._recordFormFillingTime("creditCard", "autofill-update", timeStartedFillingMS);
     } else {
+      // Add the probe to record credit card manual filling.
       Services.telemetry.scalarAdd("formautofill.creditCards.fill_type_manual", 1);
       this._recordFormFillingTime("creditCard", "manual", timeStartedFillingMS);
     }
 
+    // Early return if it's a duplicate data
+    let dupGuid = this.profileStorage.creditCards.getDuplicateGuid(creditCard.record);
+    if (dupGuid) {
+      this.profileStorage.creditCards.notifyUsed(dupGuid);
+      return;
+    }
+
     let state = await FormAutofillDoorhanger.show(target, "creditCard");
     if (state == "cancel") {
       return;
     }
 
     if (state == "disable") {
       Services.prefs.setBoolPref("extensions.formautofill.creditCards.enabled", false);
       return;
--- a/browser/extensions/formautofill/ProfileStorage.jsm
+++ b/browser/extensions/formautofill/ProfileStorage.jsm
@@ -1555,16 +1555,43 @@ class CreditCards extends AutofillRecord
       } else if (expYear < 100) {
         // Enforce 4 digits years.
         creditCard["cc-exp-year"] = expYear + 2000;
       } else {
         creditCard["cc-exp-year"] = expYear;
       }
     }
   }
+
+  /**
+   * Normailze the given record and retrun the first matched guid if storage has the same record.
+   * @param {Object} targetCreditCard
+   *        The credit card for duplication checking.
+   * @returns {string|null}
+   *          Return the first guid if storage has the same credit card and null otherwise.
+   */
+  getDuplicateGuid(targetCreditCard) {
+    let clonedTargetCreditCard = this._clone(targetCreditCard);
+    this._normalizeRecord(clonedTargetCreditCard);
+    for (let creditCard of this.data) {
+      let isDuplicate = this.VALID_FIELDS.every(field => {
+        if (!clonedTargetCreditCard[field]) {
+          return !creditCard[field];
+        }
+        if (field == "cc-number") {
+          return this._getMaskedCCNumber(clonedTargetCreditCard[field]) == creditCard[field];
+        }
+        return clonedTargetCreditCard[field] == creditCard[field];
+      });
+      if (isDuplicate) {
+        return creditCard.guid;
+      }
+    }
+    return null;
+  }
 }
 
 function ProfileStorage(path) {
   this._path = path;
   this._initializePromise = null;
   this.INTERNAL_FIELDS = INTERNAL_FIELDS;
 }
 
--- a/browser/extensions/formautofill/test/browser/browser_creditCard_doorhanger.js
+++ b/browser/extensions/formautofill/test/browser/browser_creditCard_doorhanger.js
@@ -40,34 +40,149 @@ add_task(async function test_submit_cred
       let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
                                                        "popupshown");
       await ContentTask.spawn(browser, null, async function() {
         let form = content.document.getElementById("form");
         let name = form.querySelector("#cc-name");
         name.focus();
         name.setUserInput("User 1");
 
-        let number = form.querySelector("#cc-number");
-        number.setUserInput("1111222233334444");
+        form.querySelector("#cc-number").setUserInput("1111222233334444");
+        form.querySelector("#cc-exp-month").setUserInput("12");
+        form.querySelector("#cc-exp-year").setUserInput("2017");
 
         // Wait 1000ms before submission to make sure the input value applied
         await new Promise(resolve => setTimeout(resolve, 1000));
         form.querySelector("input[type=submit]").click();
       });
 
       await promiseShown;
       await clickDoorhangerButton(MAIN_BUTTON);
     }
   );
 
   let creditCards = await getCreditCards();
   is(creditCards.length, 1, "1 credit card in storage");
   is(creditCards[0]["cc-name"], "User 1", "Verify the name field");
 });
 
+add_task(async function test_submit_untouched_creditCard_form() {
+  await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
+    async function(browser) {
+      await openPopupOn(browser, "form #cc-name");
+      await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
+      await BrowserTestUtils.synthesizeKey("VK_RETURN", {}, browser);
+      await ContentTask.spawn(browser, null, async function() {
+        let form = content.document.getElementById("form");
+
+        // Wait 1000ms before submission to make sure the input value applied
+        await new Promise(resolve => setTimeout(resolve, 1000));
+        form.querySelector("input[type=submit]").click();
+      });
+
+      await sleep(1000);
+      is(PopupNotifications.panel.state, "closed", "Doorhanger is hidden");
+    }
+  );
+
+  let creditCards = await getCreditCards();
+  is(creditCards.length, 1, "Still 1 credit card in storage");
+  is(creditCards[0]["cc-name"], "User 1", "Verify the name field");
+  is(creditCards[0].timesUsed, 1, "timesUsed field set to 1");
+});
+
+add_task(async function test_submit_changed_creditCard_form() {
+  await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
+    async function(browser) {
+      let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
+                                                       "popupshown");
+      await openPopupOn(browser, "form #cc-name");
+      await BrowserTestUtils.synthesizeKey("VK_DOWN", {}, browser);
+      await BrowserTestUtils.synthesizeKey("VK_RETURN", {}, browser);
+      await ContentTask.spawn(browser, null, async function() {
+        let form = content.document.getElementById("form");
+        let name = form.querySelector("#cc-name");
+
+        name.focus();
+        await new Promise(resolve => setTimeout(resolve, 1000));
+        name.setUserInput("");
+        // Wait 1000ms before submission to make sure the input value applied
+        await new Promise(resolve => setTimeout(resolve, 1000));
+        form.querySelector("input[type=submit]").click();
+      });
+
+      await promiseShown;
+      await clickDoorhangerButton(SECONDARY_BUTTON);
+    }
+  );
+
+  let creditCards = await getCreditCards();
+  is(creditCards.length, 1, "Still 1 credit card in storage");
+  is(creditCards[0]["cc-name"], "User 1", "Verify the name field");
+});
+
+add_task(async function test_submit_duplicate_creditCard_form() {
+  await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
+    async function(browser) {
+      await ContentTask.spawn(browser, null, async function() {
+        let form = content.document.getElementById("form");
+        let name = form.querySelector("#cc-name");
+        name.focus();
+
+        name.setUserInput("User 1");
+        form.querySelector("#cc-number").setUserInput("1111222233334444");
+        form.querySelector("#cc-exp-month").setUserInput("12");
+        form.querySelector("#cc-exp-year").setUserInput("2017");
+
+        // Wait 1000ms before submission to make sure the input value applied
+        await new Promise(resolve => setTimeout(resolve, 1000));
+        form.querySelector("input[type=submit]").click();
+      });
+
+      await sleep(1000);
+      is(PopupNotifications.panel.state, "closed", "Doorhanger is hidden");
+    }
+  );
+
+  let creditCards = await getCreditCards();
+  is(creditCards.length, 1, "Still 1 credit card in storage");
+  is(creditCards[0]["cc-name"], "User 1", "Verify the name field");
+  is(creditCards[0].timesUsed, 1, "timesUsed field set to 1");
+});
+
+add_task(async function test_submit_unnormailzed_creditCard_form() {
+  await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
+    async function(browser) {
+      await ContentTask.spawn(browser, null, async function() {
+        let form = content.document.getElementById("form");
+        let name = form.querySelector("#cc-name");
+        name.focus();
+
+        name.setUserInput("User 1");
+
+        form.querySelector("#cc-number").setUserInput("1111222233334444");
+        form.querySelector("#cc-exp-month").setUserInput("12");
+        // Set unnormalized year
+        form.querySelector("#cc-exp-year").setUserInput("17");
+
+        // Wait 1000ms before submission to make sure the input value applied
+        await new Promise(resolve => setTimeout(resolve, 1000));
+        form.querySelector("input[type=submit]").click();
+      });
+
+      await sleep(1000);
+      is(PopupNotifications.panel.state, "closed", "Doorhanger is hidden");
+    }
+  );
+
+  let creditCards = await getCreditCards();
+  is(creditCards.length, 1, "Still 1 credit card in storage");
+  is(creditCards[0]["cc-exp-year"], "2017", "Verify the expiry year field");
+});
+
 add_task(async function test_submit_creditCard_never_save() {
   await BrowserTestUtils.withNewTab({gBrowser, url: CREDITCARD_FORM_URL},
     async function(browser) {
       let promiseShown = BrowserTestUtils.waitForEvent(PopupNotifications.panel,
                                                        "popupshown");
       await ContentTask.spawn(browser, null, async function() {
         let form = content.document.getElementById("form");
         let name = form.querySelector("#cc-name");
--- a/browser/extensions/formautofill/test/browser/head.js
+++ b/browser/extensions/formautofill/test/browser/head.js
@@ -14,17 +14,17 @@ Cu.import("resource://testing-common/Log
 
 const MANAGE_ADDRESSES_DIALOG_URL = "chrome://formautofill/content/manageAddresses.xhtml";
 const MANAGE_CREDIT_CARDS_DIALOG_URL = "chrome://formautofill/content/manageCreditCards.xhtml";
 const EDIT_ADDRESS_DIALOG_URL = "chrome://formautofill/content/editAddress.xhtml";
 const EDIT_CREDIT_CARD_DIALOG_URL = "chrome://formautofill/content/editCreditCard.xhtml";
 const BASE_URL = "http://mochi.test:8888/browser/browser/extensions/formautofill/test/browser/";
 const FORM_URL = "http://mochi.test:8888/browser/browser/extensions/formautofill/test/browser/autocomplete_basic.html";
 const CREDITCARD_FORM_URL =
-  "http://mochi.test:8888/browser/browser/extensions/formautofill/test/browser/autocomplete_creditcard_basic.html";
+  "https://example.org/browser/browser/extensions/formautofill/test/browser/autocomplete_creditcard_basic.html";
 const FTU_PREF = "extensions.formautofill.firstTimeUse";
 const ENABLED_AUTOFILL_ADDRESSES_PREF = "extensions.formautofill.addresses.enabled";
 const AUTOFILL_CREDITCARDS_AVAILABLE_PREF = "extensions.formautofill.creditCards.available";
 const ENABLED_AUTOFILL_CREDITCARDS_PREF = "extensions.formautofill.creditCards.enabled";
 const SYNC_USERNAME_PREF = "services.sync.username";
 const SYNC_ADDRESSES_PREF = "services.sync.engine.addresses";
 const SYNC_CREDITCARDS_PREF = "services.sync.engine.creditcards";
 const SYNC_CREDITCARDS_AVAILABLE_PREF = "services.sync.engine.creditcards.available";