From 41ef0f39c731e28e295f66ecf6680191f7dd0890 Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Wed, 8 Mar 2017 13:12:50 -0500 Subject: [PATCH 1/4] Make updateWith() reject the show() promise on validation failures Previously, it would ignore any bad data. Now it validates the data ahead of time before performing any updates. Closes #350. --- index.html | 372 +++++++++++++++++++++++++++++++---------------------- 1 file changed, 218 insertions(+), 154 deletions(-) diff --git a/index.html b/index.html index 4277a138..52201f1f 100644 --- a/index.html +++ b/index.html @@ -1137,14 +1137,14 @@

If an item in the sequence has the selected member set to true, then this is the shipping option that will be used by default and - shippingOption will be set to the id of this option without running - the shipping option changed algorithm. Authors SHOULD NOT - set selected to - true on more than one item. If more than one item in the sequence - has selected set to - true, then user agents MUST select the last one in the - sequence. + shippingOption will + be set to the id of this + option without running the shipping option changed + algorithm. Authors SHOULD NOT set selected to true on more than + one item. If more than one item in the sequence has selected set to true, then + user agents MUST select the last one in the sequence.

The shippingOptions member is only used if the @@ -2057,8 +2057,8 @@

The remaining steps are conditional on the detailsPromise settling. If detailsPromise never settles then the payment - request is blocked. Users should always be able to cancel a - payment request. Implementations may choose to implement a + request is blocked. Users SHOULD always be able to cancel a + payment request. Implementations MAY choose to implement a timeout for pending updates if detailsPromise doesn't settle in a reasonable amount of time. If an implementation chooses to implement a timeout, they must @@ -2069,28 +2069,11 @@

  • Upon rejection of detailsPromise:
      -
    1. Abort the current user interaction and close down any - remaining user interface. -
      2. -
    2. Set target.[[\state]] to "closed". -
      3. -
    3. Reject the promise - target.[[\acceptPromise]] with an - "AbortError" DOMException. +
    4. + Abort the update with an "AbortError" + DOMException.
    -
    - If the promise detailsPromise is rejected then this - is a fatal error for the payment request. This would - potentially leave the payment request in an inconsistent state - since the web page hasn't successfully handled the change - event. Consequently, if detailsPromise is rejected - then the payment request is aborted. -

    - User agents might show an error message to the user - when this occurs. -

    -
  • Upon fulfillment of detailsPromise with value @@ -2098,165 +2081,212 @@

    1. Let details be the result of converting value to a PaymentDetailsUpdate dictionary. - If this throws an exception, abort these substeps, and - optionally show an error message to the user. + If this throws an exception, abort the update + with the thrown exception.
      2. -
    2. If the total - member of details is present, then: -
        -
      1. Let value be - details.total.amount.value. -
        2. -
      2. If value is a valid decimal monetary - value and the first character of value is - not U+002D HYPHEN-MINUS, then copy - details.total to the total member - of target.[[\details]]. (Negative total - amounts are ignored.) -
        3. -
      +
    3. Let serializedModifierData be an empty list.
      4. -
    4. If the displayItems member of details is - present, then: -
        -
      1. If every PaymentItem in - details.displayItems has an amount.value containing a - valid decimal monetary value, then copy - details.displayItems to the - displayItems member of - target.[[\details]]. -
        2. -
      +
    5. Let selectedShippingOption be null. +
      6. +
    6. Let shippingOptions be an empty + sequence<PaymentShippingOption>.
      7. -
    7. If the modifiers member of details is - present, then: +
    8. Validate and canonicalize the details:
        -
      1. Let modifiers be the sequence - details.modifiers. -
        2. -
      2. Let serializedModifierData be an empty list. +
      3. If the + total member of details is present, then: +
          +
        1. Let value be + details.total.amount.value. +
          2. +
        2. If value is not a valid decimal + monetary value, then abort the update with a + TypeError. +
          3. +
        3. If the first character of value is + U+002D HYPHEN-MINUS, then abort the update with + a TypeError. +
          4. +
        4. -
      4. For each PaymentDetailsModifier - modifier in modifiers: -
          -
        1. If the total member of modifier - is present and - member.total.If the displayItems member of details + is present, then for each item in + details.displayItems: +
            +
          1. If item.amount.value is not a - valid decimal monetary value, then set - modifiers to an empty sequence and - serializedModifierData to an empty list, and - jump to the step labeled copy modifiers below. + valid decimal monetary value, then abort the + update with a TypeError. +
            2. +
          +
          2. +
        2. If the shippingOptions member of + details is present, and + target.[[\options]].requestShipping is + true, then: +
            +
          1. Set shippingOptions to + details.shippingOptions.
            2. -
          2. If the additionalDisplayItems member of - modifier is present, then for each - PaymentItem item in - modifier.additionalDisplayItems: +
          3. Let seenIDs be an empty list. +
            4. +
          4. For each option in + shippingOptions:
              -
            1. Let amountValue be - item.amount.value. +
            2. If option.amount.value + is not a valid decimal monetary value, + then abort the update with a + TypeError.
              3. -
            3. If amountValue is not a valid - decimal monetary value, then set - modifiers to an empty sequence and - serializedModifierData to an empty list, - and jump to the step labeled copy - modifiers below. +
            4. If seenIDs contains + option.id, then set + options to an empty sequence and break. +
              5. +
            5. Append option.id to + seenIDs.
            5. -
          5. Let serializedData be the result of - JSON-serializing modifier.data into a string, - if the data member of - modifier is present, or null if it is not. - If JSON-serializing throws an exception, then - set modifiers to an empty sequence and - serializedModifierData to an empty list, - and jump to the step labeled copy modifiers - below. +
          6. For each option in + shippingOptions (which may have been reset + to the empty sequence in the previous step): +
              +
            1. If option.selected is + true, then set selectedShippingOption to + option.id. +
              2. +
            7. -
          7. Add serializedData to - serializedModifierData. +
          +
          3. +
        3. If the modifiers member of details is + present, then: +
            +
          1. Let modifiers be the sequence + details.modifiers.
            2. -
          2. Remove the data member of - modifier, if it is present. +
          3. Let serializedModifierData be an empty + list. +
            4. +
          4. For each PaymentDetailsModifier + modifier in modifiers: +
              +
            1. If the total member of + modifier is present and + member.total.amount.value is not a + valid decimal monetary value, then abort + the update with a TypeError. +
              2. +
            2. If the additionalDisplayItems member of + modifier is present, then for each + PaymentItem item in + modifier.additionalDisplayItems: +
                +
              1. Let amountValue be + item.amount.value. +
                2. +
              2. If amountValue is not a valid + decimal monetary value, then abort the + update with a TypeError. +
                3. +
              +
              3. +
            3. Let serializedData be the result of + JSON-serializing + modifier.data into a + string, if the data member of + modifier is present, or null if it is + not. If JSON-serializing throws an + exception, then abort the update with that + exception. +
              4. +
            4. Add serializedData to + serializedModifierData. +
              5. +
            5. Remove the data member of + modifier, if it is present. +
              6. +
          4. -
        4. - Copy modifiers: Set - target.[[\details]].modifiers to - modifiers, and set - target.[[\serializedModifierData]] to - serializedModifierData. -
        5. -
      5. If the shippingOptions member of details - is present, and - target.[[\options]].requestShipping is true, - then: +
      6. Update the PaymentRequest using the new details:
          -
        1. Let options be - details.shippingOptions. -
          2. -
        2. Let seenIDs be an empty list. +
        3. If the + total member of details is present, then: +
            +
          1. Set + target.[[\details]].total + to details.total. +
            2. +
          4. -
        4. For each option in options: +
        5. If the displayItems member of details + is present, then:
            -
          1. If option.amount.value is not a - valid decimal monetary value, then set - options to the empty sequence and break. +
          2. Set + target.[[\details]].displayItems + to details.displayItems.
            3. -
          3. If seenIDs contains - option.id, then set - options to an empty sequence and break. +
          +
          6. +
        6. If the shippingOptions member of + details is present, and + target.[[\options]].requestShipping is + true, then: +
            +
          1. Set + target.[[\details]].shippingOptions + to shippingOptions.
            2. -
          2. Append option.id to - seenIDs. +
          3. Set the value of target's shippingOption + attribute to selectedShippingOption.
          7. -
        7. For each option in options (which - may have been reset to the empty sequence in the previous - step): +
        8. If the modifiers member of details is + present, then:
            -
          1. If option.selected is true, - then set selectedShippingOption to - option.id. +
          2. Set + target.[[\details]].modifiers + to details.modifiers. +
            3. +
          3. Set + target.[[\serializedModifierData]] to + serializedModifierData.
          9. -
        9. Copy options to the shippingOptions - member of target.[[\details]]. +
        10. If the error member of + details is present, then the user agent + should update the user interface to display the error + message contained in error.
        7. -
      7. If the error - member of details is present, then the user - agent should update the user interface to display the error - message contained in error. -
    9. In either case, run the following steps, after either the upon @@ -2274,6 +2304,40 @@

    • +

    + If any of the above steps say to abort the update with + an exception exception, then: +

    +
      +
    1. Abort the current user interaction and close down any remaining + user interface. +
      2. +
    2. Set target.[[\state]] to "closed". +
      3. +
    3. Reject the promise target.[[\acceptPromise]] + with exception. +
      4. +
    4. Abort the algorithm. +
      5. +
    +
    +

    + Aborting the update is + performed when there is a fatal error updating the payment + request, such as the supplied detailsPromise + rejecting, or its fulfillment value containing invalid data. This + would potentially leave the payment request in an inconsistent + state since the web page hasn't successfully handled the change + event. Consequently, the PaymentRequest moves to a + "closed" state. The error is signaled to the developer + through the rejection of the [[\acceptPromise]], i.e. the + promise returned by show(). +

    +

    + User agents might show an error message to the user when + this occurs. +

    +

    From 54e79f5cdda6e7fc1e0d1cc3ddc82776279c7c14 Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Fri, 10 Mar 2017 12:34:11 -0500 Subject: [PATCH 2/4] Also disallow negative modifiers in updateWith --- index.html | 5 +++++ tidyconfig.txt | 1 + 2 files changed, 6 insertions(+) diff --git a/index.html b/index.html index 52201f1f..9029eeba 100644 --- a/index.html +++ b/index.html @@ -2203,6 +2203,11 @@

    decimal monetary value, then abort the update with a TypeError. +
  • If the first character of + amountValue is U+002D HYPHEN-MINUS, + then abort the update with a + TypeError. +
  • Let serializedData be the result of diff --git a/tidyconfig.txt b/tidyconfig.txt index 07a65e6a..8de98a23 100644 --- a/tidyconfig.txt +++ b/tidyconfig.txt @@ -2,3 +2,4 @@ char-encoding: utf8 indent: yes wrap: 80 tidy-mark: no +newline: LF From d5a614b94ad17bda07c99ec69c6b23aa5b672f37 Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Fri, 10 Mar 2017 12:43:16 -0500 Subject: [PATCH 3/4] Update the model for the "error" member to match discussion --- index.html | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/index.html b/index.html index 9029eeba..48225ccb 100644 --- a/index.html +++ b/index.html @@ -1248,9 +1248,14 @@

    When the payment request is updated using updateWith(), the PaymentDetailsUpdate can contain a message in the error - member that will be displayed to the user. For example, this might - commonly be used to explain why goods cannot be shipped to the chosen - shipping address. + member that will be displayed to the user, if the + PaymentDetailsUpdate indicates that there are no valid + shippingOptions + (and the PaymentRequest was constructed with the requestShipping option set to + true). This can be used to explain why goods cannot be shipped to the + chosen shipping address, or any other reason why no shipping options + are available.
    total @@ -2283,12 +2288,22 @@

    • -
  • If the If target.[[\options]].requestShipping is + true, and + target.[[\details]].shippingOptions + is empty, then the developer has signified that there are + no valid shipping options for the currently-chosen shipping + address (given by target's shippingAddress). In + this case, the user agent SHOULD display an error + indicating this, and MAY indicate that that the + currently-chosen shipping address is invalid in some way. + The user agent SHOULD use the error member of - details is present, then the user agent - should update the user interface to display the error - message contained in error. + details, if it is present, to give more + information about why there are no valid shipping options + for that address.
    • From 943a45721f6970d3f4654ce62e5fda6efa217fe9 Mon Sep 17 00:00:00 2001 From: Domenic Denicola Date: Fri, 10 Mar 2017 13:46:03 -0500 Subject: [PATCH 4/4] Move the negative check to the correct spot --- index.html | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/index.html b/index.html index 48225ccb..d6a0fddb 100644 --- a/index.html +++ b/index.html @@ -2187,12 +2187,22 @@

    modifier in modifiers:
    1. If the total member of - modifier is present and - member.total.amount.value is not a - valid decimal monetary value, then abort - the update with a TypeError. + modifier is present, then: +
        +
      1. Let value be + modifier.total.amount.value. +
        2. +
      2. If value is not a valid + decimal monetary value, then abort the + update with a TypeError. +
        3. +
      3. If the first character of value + is U+002D HYPHEN-MINUS, then abort the + update with a TypeError. +
        4. +
    2. If the additionalDisplayItems member of modifier is present, then for each @@ -2208,11 +2218,6 @@

      decimal monetary value, then abort the update with a TypeError.

      3. -
    3. If the first character of - amountValue is U+002D HYPHEN-MINUS, - then abort the update with a - TypeError. -
  • Let serializedData be the result of