Commit 46813c48 authored by Matthew Noorenberghe's avatar Matthew Noorenberghe
Browse files

Bug 1476204 - Handle autofill record update state changes in the unpriv. PR forms. r=jaws

This is easier to understand as we don't have to round-trip the whole success and error states to the privileged wrapper which could potentially lead to stale state changes.
This is also much simpler for the basic-card-form as it doesn't need a lot of the complexity of the previous implementation.

* Move selectedStateKey from page to address-page since it doesn't apply to basic-card-page

MozReview-Commit-ID: B4kiZNWElGI

--HG--
extra : rebase_source : 183a3bd44ed33566fccdc024eabdccef83554d50
parent ae5eb481
Loading
Loading
Loading
Loading
+27 −49
Original line number Diff line number Diff line
@@ -560,69 +560,52 @@ var paymentDialogWrapper = {
    window.close();
  },

  async onUpdateAutofillRecord(collectionName, record, guid, {
    errorStateChange,
    preserveOldProperties,
    selectedStateKey,
    successStateChange,
  }) {
  async onUpdateAutofillRecord(collectionName, record, guid, messageID) {
    let responseMessage = {
      guid,
      messageID,
      stateChange: {},
    };
    try {
      if (collectionName == "creditCards" && !guid && !record.isTemporary) {
        // We need to be logged in so we can encrypt the credit card number and
        // that's only supported when we're adding a new record.
        // TODO: "MasterPassword.ensureLoggedIn" can be removed after the storage
        // APIs are refactored to be async functions (bug 1399367).
        if (!await MasterPassword.ensureLoggedIn()) {
        Cu.reportError("User canceled master password entry");
        return;
          throw new Error("User canceled master password entry");
        }
      }

      let isTemporary = record.isTemporary;
      let collection = isTemporary ? this.temporaryStore[collectionName] :
                                     formAutofillStorage[collectionName];

    try {
      if (guid) {
        let preserveOldProperties = true;
        await collection.update(guid, record, preserveOldProperties);
      } else {
        guid = await collection.add(record);
        responseMessage.guid = await collection.add(record);
      }

      if (isTemporary && collectionName == "addresses") {
        // there will be no formautofill-storage-changed event to update state
        // so add updated collection here
        Object.assign(successStateChange, {
        Object.assign(responseMessage.stateChange, {
          tempAddresses: this.temporaryStore.addresses.getAll(),
        });
      }
      if (isTemporary && collectionName == "creditCards") {
        // there will be no formautofill-storage-changed event to update state
        // so add updated collection here
        Object.assign(successStateChange, {
        Object.assign(responseMessage.stateChange, {
          tempBasicCards: this.temporaryStore.creditCards.getAll(),
        });
      }

      // Select the new record
      if (selectedStateKey) {
        if (selectedStateKey.length == 1) {
          Object.assign(successStateChange, {
            [selectedStateKey[0]]: guid,
          });
        } else if (selectedStateKey.length == 2) {
          // Need to keep properties like preserveFieldValues from getting removed.
          let subObj = Object.assign({}, successStateChange[selectedStateKey[0]]);
          subObj[selectedStateKey[1]] = guid;
          Object.assign(successStateChange, {
            [selectedStateKey[0]]: subObj,
          });
        } else {
          throw new Error(`selectedStateKey not supported: '${selectedStateKey}'`);
        }
      }

      this.sendMessageToContent("updateState", successStateChange);
    } catch (ex) {
      this.sendMessageToContent("updateState", errorStateChange);
      responseMessage.error = true;
    } finally {
      this.sendMessageToContent("updateAutofillRecord:Response", responseMessage);
    }
  },

@@ -695,12 +678,7 @@ var paymentDialogWrapper = {
        break;
      }
      case "updateAutofillRecord": {
        this.onUpdateAutofillRecord(data.collectionName, data.record, data.guid, {
          errorStateChange: data.errorStateChange,
          preserveOldProperties: data.preserveOldProperties,
          selectedStateKey: data.selectedStateKey,
          successStateChange: data.successStateChange,
        });
        this.onUpdateAutofillRecord(data.collectionName, data.record, data.guid, data.messageID);
        break;
      }
    }
+41 −19
Original line number Diff line number Diff line
@@ -243,7 +243,7 @@ export default class AddressForm extends PaymentStateSubscriberMixin(PaymentRequ
    }
  }

  saveRecord() {
  async saveRecord() {
    let record = this.formHandler.buildFormObject();
    let currentState = this.requestStore.getState();
    let {
@@ -258,22 +258,15 @@ export default class AddressForm extends PaymentStateSubscriberMixin(PaymentRequ
      record.isTemporary = true;
    }

    let state = {
      errorStateChange: {
        page: {
          id: "address-page",
          onboardingWizard: page.onboardingWizard,
          error: this.dataset.errorGenericSave,
        },
        "address-page": addressPage,
      },
      preserveOldProperties: true,
      selectedStateKey: page.selectedStateKey,
    };

    let successStateChange;
    const previousId = page.previousId;
    if (page.onboardingWizard && !Object.keys(savedBasicCards).length) {
      state.successStateChange = {
      successStateChange = {
        "basic-card-page": {
          // Preserve field values as the user may have already edited the card
          // page and went back to the address page to make a correction.
          preserveFieldValues: true,
        },
        page: {
          id: "basic-card-page",
          previousId: "address-page",
@@ -281,7 +274,7 @@ export default class AddressForm extends PaymentStateSubscriberMixin(PaymentRequ
        },
      };
    } else {
      state.successStateChange = {
      successStateChange = {
        page: {
          id: previousId || "payment-summary",
          onboardingWizard: page.onboardingWizard,
@@ -290,11 +283,40 @@ export default class AddressForm extends PaymentStateSubscriberMixin(PaymentRequ
    }

    if (previousId) {
      state.successStateChange[previousId] = Object.assign({}, currentState[previousId]);
      state.successStateChange[previousId].preserveFieldValues = true;
      successStateChange[previousId] = Object.assign({}, currentState[previousId]);
      successStateChange[previousId].preserveFieldValues = true;
    }

    paymentRequest.updateAutofillRecord("addresses", record, addressPage.guid, state);
    try {
      let {guid} = await paymentRequest.updateAutofillRecord("addresses", record, addressPage.guid);
      let selectedStateKey = addressPage.selectedStateKey;

      if (selectedStateKey.length == 1) {
        Object.assign(successStateChange, {
          [selectedStateKey[0]]: guid,
        });
      } else if (selectedStateKey.length == 2) {
        // Need to keep properties like preserveFieldValues from getting removed.
        let subObj = Object.assign({}, successStateChange[selectedStateKey[0]]);
        subObj[selectedStateKey[1]] = guid;
        Object.assign(successStateChange, {
          [selectedStateKey[0]]: subObj,
        });
      } else {
        throw new Error(`selectedStateKey not supported: '${selectedStateKey}'`);
      }

      this.requestStore.setState(successStateChange);
    } catch (ex) {
      log.warn("saveRecord: error:", ex);
      this.requestStore.setState({
        page: {
          id: "address-page",
          onboardingWizard: page.onboardingWizard,
          error: this.dataset.errorGenericSave,
        },
      });
    }
  }
}

+1 −1
Original line number Diff line number Diff line
@@ -144,10 +144,10 @@ export default class AddressPicker extends RichPicker {
    let nextState = {
      page: {
        id: "address-page",
        selectedStateKey: [this.selectedStateKey],
      },
      "address-page": {
        addressFields: this.getAttribute("address-fields"),
        selectedStateKey: [this.selectedStateKey],
      },
    };

+15 −20
Original line number Diff line number Diff line
@@ -203,10 +203,10 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(PaymentRe
          page: {
            id: "address-page",
            previousId: "basic-card-page",
            selectedStateKey: ["basic-card-page", "billingAddressGUID"],
          },
          "address-page": {
            guid: null,
            selectedStateKey: ["basic-card-page", "billingAddressGUID"],
            title: this.dataset.billingAddressTitleAdd,
          },
          "basic-card-page": {
@@ -284,11 +284,10 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(PaymentRe
    this.saveButton.disabled = !this.form.checkValidity();
  }

  saveRecord() {
  async saveRecord() {
    let record = this.formHandler.buildFormObject();
    let currentState = this.requestStore.getState();
    let {
      page,
      tempBasicCards,
      "basic-card-page": basicCardPage,
    } = currentState;
@@ -308,28 +307,24 @@ export default class BasicCardForm extends PaymentStateSubscriberMixin(PaymentRe
      record["cc-number"] = record["cc-number"] || "";
    }

    let state = {
      errorStateChange: {
        page: {
          id: "basic-card-page",
          error: this.dataset.errorGenericSave,
        },
      },
      preserveOldProperties: true,
      selectedStateKey: ["selectedPaymentCard"],
      successStateChange: {
    try {
      let {guid} = await paymentRequest.updateAutofillRecord("creditCards", record,
                                                             basicCardPage.guid);
      this.requestStore.setState({
        page: {
          id: "payment-summary",
        },
        selectedPaymentCard: guid,
      });
    } catch (ex) {
      log.warn("saveRecord: error:", ex);
      this.requestStore.setState({
        page: {
          id: "basic-card-page",
          error: this.dataset.errorGenericSave,
        },
    };

    const previousId = page.previousId;
    if (previousId) {
      state.successStateChange[previousId] = Object.assign({}, currentState[previousId]);
      });
    }

    paymentRequest.updateAutofillRecord("creditCards", record, basicCardPage.guid, state);
  }
}

+1 −0
Original line number Diff line number Diff line
@@ -59,6 +59,7 @@
        <section class="group">
          <fieldset>
            <legend>User Data Errors</legend>
            <button id="saveVisibleForm" title="Bypasses field validation">Save Visible Form</button>
            <button id="setShippingError">Shipping Error</button>
            <button id="setAddressErrors">Address Errors</button>
          </fieldset>
Loading