Bug 1402963 - Part 1: Deduplicate credit card by checking credit card storage and untouched fields. r=lchang
authorsteveck-chung <schung@mozilla.com>
Wed, 27 Sep 2017 18:38:13 +0800
changeset 443945 3eef0753a078b9de3ad6d0cd98c96e5f345edc5a
parent 443944 a22464e2572f83d2313bfc3537034b92edd75bd0
child 443946 8dbc7c83b48762894fd1e02fa1032bfc1a3fdb73
push id1618
push userCallek@gmail.com
push dateThu, 11 Jan 2018 17:45:48 +0000
treeherdermozilla-release@882ca853e05a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslchang
bugs1402963
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
@@ -1620,16 +1620,43 @@ class CreditCards extends AutofillRecord
         creditCard["cc-exp-month"] = expMonth;
         creditCard["cc-exp-year"] = expYear;
         break;
       }
     }
 
     delete creditCard["cc-exp"];
   }
+
+  /**
+   * 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";