Bug 1496069 - Properly pass noValidate to EditAddress and set @novalidate. r=jaws
authorMatthew Noorenberghe <mozilla@noorenberghe.ca>
Tue, 16 Oct 2018 18:01:35 +0000
changeset 489877 d71434f407554db9cf5320fbf8098c2a767f91e4
parent 489876 e0aa704cdd63d38f2cf4137240d9f7bb5f7a3829
child 489878 ed09a03420aef61b84d91d1977ec969fb01cf94a
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersjaws
bugs1496069
milestone64.0a1
Bug 1496069 - Properly pass noValidate to EditAddress and set @novalidate. r=jaws Differential Revision: https://phabricator.services.mozilla.com/D8808
browser/extensions/formautofill/content/autofillEditForms.js
browser/extensions/formautofill/content/editAddress.xhtml
browser/extensions/formautofill/content/editCreditCard.xhtml
browser/extensions/formautofill/content/manageDialog.js
browser/extensions/formautofill/test/browser/browser_editAddressDialog.js
browser/extensions/formautofill/test/browser/browser_editCreditCardDialog.js
browser/extensions/formautofill/test/browser/head.js
--- a/browser/extensions/formautofill/content/autofillEditForms.js
+++ b/browser/extensions/formautofill/content/autofillEditForms.js
@@ -118,16 +118,17 @@ class EditAutofillForm {
 class EditAddress extends EditAutofillForm {
   /**
    * @param {HTMLElement[]} elements
    * @param {object} record
    * @param {object} config
    * @param {string[]} config.DEFAULT_REGION
    * @param {function} config.getFormFormat Function to return form layout info for a given country.
    * @param {string[]} config.countries
+   * @param {boolean} [config.noValidate=undefined] Whether to validate the form
    */
   constructor(elements, record, config) {
     super(elements);
 
     Object.assign(this, config);
     let {form} = this._elements;
     Object.assign(this._elements, {
       addressLevel3Label: form.querySelector("#address-level3-container > .label-text"),
@@ -138,19 +139,17 @@ class EditAddress extends EditAutofillFo
     });
 
     this.populateCountries();
     // Need to populate the countries before trying to set the initial country.
     // Also need to use this._record so it has the default country selected.
     this.loadRecord(record);
     this.attachEventListeners();
 
-    if (config.novalidate) {
-      this.form.setAttribute("novalidate", "true");
-    }
+    form.noValidate = !!config.noValidate;
   }
 
   loadRecord(record) {
     this._record = record;
     if (!record) {
       record = {
         country: this.DEFAULT_REGION,
       };
--- a/browser/extensions/formautofill/content/editAddress.xhtml
+++ b/browser/extensions/formautofill/content/editAddress.xhtml
@@ -85,27 +85,30 @@
 
     let {
       DEFAULT_REGION,
       countries,
     } = FormAutofill;
     let {
       getFormFormat,
     } = FormAutofillUtils;
-    let record = window.arguments && window.arguments[0];
-    let novalidate = window.arguments && window.arguments[1] == "novalidate";
+    let args = window.arguments || [];
+    let {
+      record,
+      noValidate,
+    } = args[0] || {};
 
     /* import-globals-from autofillEditForms.js */
     var fieldContainer = new EditAddress({
       form: document.getElementById("form"),
     }, record, {
       DEFAULT_REGION,
       getFormFormat: getFormFormat.bind(FormAutofillUtils),
       countries,
-      novalidate,
+      noValidate,
     });
 
     /* import-globals-from editDialog.js */
     new EditAddressDialog({
       title: document.querySelector("title"),
       fieldContainer,
       controlsContainer: document.getElementById("controls-container"),
       cancel: document.getElementById("cancel"),
--- a/browser/extensions/formautofill/content/editCreditCard.xhtml
+++ b/browser/extensions/formautofill/content/editCreditCard.xhtml
@@ -77,17 +77,21 @@
     "use strict";
 
     (async () => {
       let {
         getAddressLabel,
         isCCNumber,
         getCreditCardNetworks,
       } = FormAutofillUtils;
-      let record = window.arguments && window.arguments[0];
+      let args = window.arguments || [];
+      let {
+        record,
+      } = args[0] || {};
+
       let addresses = {};
       for (let address of await formAutofillStorage.addresses.getAll()) {
         addresses[address.guid] = address;
       }
 
       /* import-globals-from autofillEditForms.js */
       let fieldContainer = new EditCreditCard({
         form: document.getElementById("form"),
--- a/browser/extensions/formautofill/content/manageDialog.js
+++ b/browser/extensions/formautofill/content/manageDialog.js
@@ -289,17 +289,22 @@ class ManageAddresses extends ManageReco
   }
 
   /**
    * Open the edit address dialog to create/edit an address.
    *
    * @param  {object} address [optional]
    */
   openEditDialog(address) {
-    this.prefWin.gSubDialog.open(EDIT_ADDRESS_URL, null, address, "novalidate");
+    this.prefWin.gSubDialog.open(EDIT_ADDRESS_URL, null, {
+      record: address,
+      // Don't validate in preferences since it's fine for fields to be missing
+      // for autofill purposes. For PaymentRequest addresses get more validation.
+      noValidate: true,
+    });
   }
 
   getLabel(address) {
     return FormAutofillUtils.getAddressLabel(address);
   }
 }
 
 class ManageCreditCards extends ManageRecords {
@@ -324,17 +329,19 @@ class ManageCreditCards extends ManageRe
     // If master password is set, ask for password if user is trying to edit an
     // existing credit card.
     if (!creditCard || !this._hasMasterPassword || await MasterPassword.ensureLoggedIn(true)) {
       let decryptedCCNumObj = {};
       if (creditCard) {
         decryptedCCNumObj["cc-number"] = await MasterPassword.decrypt(creditCard["cc-number-encrypted"]);
       }
       let decryptedCreditCard = Object.assign({}, creditCard, decryptedCCNumObj);
-      this.prefWin.gSubDialog.open(EDIT_CREDIT_CARD_URL, "resizable=no", decryptedCreditCard);
+      this.prefWin.gSubDialog.open(EDIT_CREDIT_CARD_URL, "resizable=no", {
+        record: decryptedCreditCard,
+      });
     }
   }
 
   /**
    * Get credit card display label. It should display masked numbers and the
    * cardholder's name, separated by a comma. If `showCreditCards` is set to
    * true, decrypted credit card numbers are shown instead.
    *
--- a/browser/extensions/formautofill/test/browser/browser_editAddressDialog.js
+++ b/browser/extensions/formautofill/test/browser/browser_editAddressDialog.js
@@ -96,27 +96,49 @@ add_task(async function test_saveAddress
 
 add_task(async function test_editAddress() {
   let addresses = await getAddresses();
   await testDialog(EDIT_ADDRESS_DIALOG_URL, win => {
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_RIGHT", {}, win);
     EventUtils.synthesizeKey("test", {}, win);
     win.document.querySelector("#save").click();
-  }, addresses[0]);
+  }, {
+    record: addresses[0],
+  });
   addresses = await getAddresses();
 
   is(addresses.length, 1, "only one address is in storage");
   is(addresses[0]["given-name"], TEST_ADDRESS_1["given-name"] + "test", "given-name changed");
   await removeAddresses([addresses[0].guid]);
 
   addresses = await getAddresses();
   is(addresses.length, 0, "Address storage is empty");
 });
 
+add_task(async function test_editSparseAddress() {
+  let record = {...TEST_ADDRESS_1};
+  info("delete some usually required properties");
+  delete record["street-address"];
+  delete record["address-level1"];
+  delete record["address-level2"];
+  await testDialog(EDIT_ADDRESS_DIALOG_URL, win => {
+    is(win.document.querySelectorAll(":-moz-ui-invalid").length, 0,
+       "Check no fields are visually invalid");
+    EventUtils.synthesizeKey("VK_TAB", {}, win);
+    EventUtils.synthesizeKey("VK_RIGHT", {}, win);
+    EventUtils.synthesizeKey("test", {}, win);
+    is(win.document.querySelector("#save").disabled, false,
+       "Save button should be enabled after an edit");
+    win.document.querySelector("#cancel").click();
+  }, {
+    record,
+  });
+});
+
 add_task(async function test_saveAddressCA() {
   await testDialog(EDIT_ADDRESS_DIALOG_URL, async win => {
     let doc = win.document;
     // Change country to verify labels
     doc.querySelector("#country").focus();
     EventUtils.synthesizeKey("Canada", {}, win);
 
     await TestUtils.waitForCondition(() => {
--- a/browser/extensions/formautofill/test/browser/browser_editCreditCardDialog.js
+++ b/browser/extensions/formautofill/test/browser/browser_editCreditCardDialog.js
@@ -133,17 +133,19 @@ add_task(async function test_editCreditC
   await testDialog(EDIT_CREDIT_CARD_DIALOG_URL, (win) => {
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_RIGHT", {}, win);
     EventUtils.synthesizeKey("test", {}, win);
     win.document.querySelector("#save").click();
-  }, creditCards[0]);
+  }, {
+    record: creditCards[0],
+  });
   ok(true, "Edit credit card dialog is closed");
   creditCards = await getCreditCards();
 
   is(creditCards.length, 1, "only one credit card is in storage");
   is(creditCards[0]["cc-name"], TEST_CREDIT_CARD_1["cc-name"] + "test", "cc name changed");
   await removeCreditCards([creditCards[0].guid]);
 
   creditCards = await getCreditCards();
@@ -163,17 +165,19 @@ add_task(async function test_editCreditC
   await testDialog(EDIT_CREDIT_CARD_DIALOG_URL, (win) => {
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_RIGHT", {}, win);
     EventUtils.synthesizeKey("test", {}, win);
     win.document.querySelector("#save").click();
-  }, creditCards[0]);
+  }, {
+    record: creditCards[0],
+  });
   ok(true, "Edit credit card dialog is closed");
   creditCards = await getCreditCards();
 
   is(creditCards.length, 1, "only one credit card is in storage");
   is(creditCards[0]["cc-name"], TEST_CREDIT_CARD["cc-name"] + "test", "cc name changed");
   is(creditCards[0].billingAddressGUID, undefined,
      "unknown GUID removed upon manual save");
   await removeCreditCards([creditCards[0].guid]);
@@ -223,17 +227,19 @@ add_task(async function test_editCardWit
   await testDialog(EDIT_CREDIT_CARD_DIALOG_URL, (win) => {
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_TAB", {}, win);
     EventUtils.synthesizeKey("VK_RIGHT", {}, win);
     EventUtils.synthesizeKey("test", {}, win);
     win.document.querySelector("#save").click();
-  }, creditCards[0]);
+  }, {
+    record: creditCards[0],
+  });
   ok(true, "Edit credit card dialog is closed");
   creditCards = await getCreditCards();
 
   is(creditCards.length, 1, "only one credit card is in storage");
   is(creditCards[0]["cc-name"], TEST_CREDIT_CARD["cc-name"] + "test", "cc name changed");
   is(creditCards[0]["cc-type"], undefined,
      "unknown cc-type removed upon manual save");
   await removeCreditCards([creditCards[0].guid]);
--- a/browser/extensions/formautofill/test/browser/head.js
+++ b/browser/extensions/formautofill/test/browser/head.js
@@ -359,19 +359,19 @@ async function removeAllRecords() {
 async function waitForFocusAndFormReady(win) {
   return Promise.all([
     new Promise(resolve => waitForFocus(resolve, win)),
     BrowserTestUtils.waitForEvent(win, "FormReady"),
   ]);
 }
 
 async function testDialog(url, testFn, arg = undefined) {
-  if (url == EDIT_CREDIT_CARD_DIALOG_URL && arg) {
-    arg = Object.assign({}, arg, {
-      "cc-number": await MasterPassword.decrypt(arg["cc-number-encrypted"]),
+  if (url == EDIT_CREDIT_CARD_DIALOG_URL && arg && arg.record) {
+    arg.record = Object.assign({}, arg.record, {
+      "cc-number": await MasterPassword.decrypt(arg.record["cc-number-encrypted"]),
     });
   }
   let win = window.openDialog(url, null, "width=600,height=600", arg);
   await waitForFocusAndFormReady(win);
   let unloadPromise = BrowserTestUtils.waitForEvent(win, "unload");
   await testFn(win);
   return unloadPromise;
 }