- From: Marcos Cáceres <notifications@github.com>
- Date: Mon, 28 Oct 2019 01:14:24 -0700
- To: w3c/payment-request <payment-request@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3c/payment-request/pull/873/review/307665796@github.com>
marcoscaceres requested changes on this pull request. Thanks for continuing to work on this... found a couple of issues. The main think is around the state machine of the event that needs a few clarifications. Having multiple ongoing asynchronous operations (.updateWidth() + .requestBillingAddress()) is going to be quite challenging to implement... but I guess as long as the order is clear (i.e., await `requestBillingAddress()` first, then do `updateWidth()`, then it might be ok 😅). I need to chew on this a bit more... the example makes me wonder if it could be simplified. > @@ -1137,7 +1139,9 @@ <h2> <li> <p> Pass the <a data-lt="converted to an IDL value">converted</a> - second element in the |paymentMethod| tuple and |modifiers|. + second element in the |paymentMethod| tuple, |modifiers|, + |request|.<a>[[\options]]</a>.<a data-lt="PaymentOptions.requestBillingAddress">requestBillingAddress</a> and ```suggestion |request|.<a>[[\options]]</a>.{{PaymentOptions/requestBillingAddress}} and ``` > @@ -1137,7 +1139,9 @@ <h2> <li> <p> Pass the <a data-lt="converted to an IDL value">converted</a> - second element in the |paymentMethod| tuple and |modifiers|. + second element in the |paymentMethod| tuple, |modifiers|, + |request|.<a>[[\options]]</a>.<a data-lt="PaymentOptions.requestBillingAddress">requestBillingAddress</a> and + |request|.<a>[[\options]]</a>.<a data-lt="PaymentOptions.requestBillingAddressParts">requestBillingAddressParts</a>. ```suggestion |request|.<a>[[\options]]</a>.{{PaymentOptions/requestBillingAddressParts}}. ``` > @@ -1490,6 +1494,20 @@ <h2> The <a>PaymentOptions</a> supplied to the constructor. </td> </tr> + <tr> + <td> + <dfn>[[\requestedShippingAddressParts]]</dfn> + </td> + <td> + The current <a>sequence</a><<a>AddressParts</a>> specifying ```suggestion The current <a>sequence</a><{{AddressParts}}> specifying ``` > @@ -1490,6 +1494,20 @@ <h2> The <a>PaymentOptions</a> supplied to the constructor. </td> </tr> + <tr> + <td> + <dfn>[[\requestedShippingAddressParts]]</dfn> + </td> + <td> + The current <a>sequence</a><<a>AddressParts</a>> specifying + the payee-requested shipping address parts, initially supplied to + the constructor via {{PaymentOptions}}'s + <a data-lt="PaymentOptions.requestShippingAddressParts"> ```suggestion ``` > @@ -1490,6 +1494,20 @@ <h2> The <a>PaymentOptions</a> supplied to the constructor. </td> </tr> + <tr> + <td> + <dfn>[[\requestedShippingAddressParts]]</dfn> + </td> + <td> + The current <a>sequence</a><<a>AddressParts</a>> specifying + the payee-requested shipping address parts, initially supplied to + the constructor via {{PaymentOptions}}'s + <a data-lt="PaymentOptions.requestShippingAddressParts"> + requestShippingAddressParts</a> and then updated with calls to ```suggestion {{PaymentOptions/requestShippingAddressParts}} and then updated with calls to ``` > @@ -1490,6 +1494,20 @@ <h2> The <a>PaymentOptions</a> supplied to the constructor. </td> </tr> + <tr> + <td> + <dfn>[[\requestedShippingAddressParts]]</dfn> + </td> + <td> + The current <a>sequence</a><<a>AddressParts</a>> specifying + the payee-requested shipping address parts, initially supplied to + the constructor via {{PaymentOptions}}'s + <a data-lt="PaymentOptions.requestShippingAddressParts"> + requestShippingAddressParts</a> and then updated with calls to + <a data-lt="PaymentRequestUpdateEvent.requestShippingAddress"> ```suggestion ``` > @@ -1490,6 +1494,20 @@ <h2> The <a>PaymentOptions</a> supplied to the constructor. </td> </tr> + <tr> + <td> + <dfn>[[\requestedShippingAddressParts]]</dfn> + </td> + <td> + The current <a>sequence</a><<a>AddressParts</a>> specifying + the payee-requested shipping address parts, initially supplied to + the constructor via {{PaymentOptions}}'s + <a data-lt="PaymentOptions.requestShippingAddressParts"> + requestShippingAddressParts</a> and then updated with calls to + <a data-lt="PaymentRequestUpdateEvent.requestShippingAddress"> + requestShippingAddress()</a>. ```suggestion {{PaymentRequestUpdateEvent/requestShippingAddress()}}. ``` > + "city", + "country", + "dependentLocality", + "organization", + "phone", + "postalCode", + "recipient", + "region", + "sortingCode" + }; + </pre> + <p> + The {{AddressParts}} enum is used to represent individual parts of a + <a>physical address</a>. + </p> + <dl data-dfn-for="AddressFields"> ```suggestion <dl> ``` > @@ -2094,12 +2199,33 @@ <h2> A boolean that indicates whether the <a>user agent</a> SHOULD collect and return the billing address associated with a <a>payment method</a> (e.g., the billing address associated with a credit card). - Typically, the user agent will return the billing address as part of - the {{PaymentMethodChangeEvent}}'s <a>methodDetails</a>. A merchant - can use this information to, for example, calculate tax in certain - jurisdictions and update the displayed total. See below for privacy - considerations regarding <a href="#user-info">exposing user - information</a>. + The <a>user agent</a> returns the billing address as part of the + {{PaymentMethodChangeEvent}}'s <a>methodDetails</a> attribute and the ```suggestion {{PaymentMethodChangeEvent}}'s {{PaymentMethodChangeEvent/methodDetails}} attribute and the ``` > @@ -2094,12 +2199,33 @@ <h2> A boolean that indicates whether the <a>user agent</a> SHOULD collect and return the billing address associated with a <a>payment method</a> (e.g., the billing address associated with a credit card). - Typically, the user agent will return the billing address as part of - the {{PaymentMethodChangeEvent}}'s <a>methodDetails</a>. A merchant - can use this information to, for example, calculate tax in certain - jurisdictions and update the displayed total. See below for privacy - considerations regarding <a href="#user-info">exposing user - information</a>. + The <a>user agent</a> returns the billing address as part of the + {{PaymentMethodChangeEvent}}'s <a>methodDetails</a> attribute and the + {{PaymentResponse}}'s <a>details</a> attribute. A payee can use this ```suggestion {{PaymentResponse}}'s {{PaymentResponse/details}} attribute. A payee can use this ``` > @@ -2094,12 +2199,33 @@ <h2> A boolean that indicates whether the <a>user agent</a> SHOULD collect and return the billing address associated with a <a>payment method</a> (e.g., the billing address associated with a credit card). - Typically, the user agent will return the billing address as part of - the {{PaymentMethodChangeEvent}}'s <a>methodDetails</a>. A merchant - can use this information to, for example, calculate tax in certain - jurisdictions and update the displayed total. See below for privacy - considerations regarding <a href="#user-info">exposing user - information</a>. + The <a>user agent</a> returns the billing address as part of the + {{PaymentMethodChangeEvent}}'s <a>methodDetails</a> attribute and the + {{PaymentResponse}}'s <a>details</a> attribute. A payee can use this + information to, for example, calculate tax in certain jurisdictions + and update the displayed total. See below for privacy considerations + regarding <a href="#user-info">exposing user information</a>. ```suggestion regarding [[[#user-info]]]. ``` You might need to reword this one after it expands out. However, avoid `<a href=>` and let ReSpec deal with linking :) > - can use this information to, for example, calculate tax in certain - jurisdictions and update the displayed total. See below for privacy - considerations regarding <a href="#user-info">exposing user - information</a>. + The <a>user agent</a> returns the billing address as part of the + {{PaymentMethodChangeEvent}}'s <a>methodDetails</a> attribute and the + {{PaymentResponse}}'s <a>details</a> attribute. A payee can use this + information to, for example, calculate tax in certain jurisdictions + and update the displayed total. See below for privacy considerations + regarding <a href="#user-info">exposing user information</a>. + </dd> + <dt> + <dfn>requestBillingAddressParts</dfn> member + </dt> + <dd> + A sequence of <a>AddressParts</a> that specifies which fine-grained ```suggestion A sequence of {{AddressParts}} that specifies which fine-grained ``` > - considerations regarding <a href="#user-info">exposing user - information</a>. + The <a>user agent</a> returns the billing address as part of the + {{PaymentMethodChangeEvent}}'s <a>methodDetails</a> attribute and the + {{PaymentResponse}}'s <a>details</a> attribute. A payee can use this + information to, for example, calculate tax in certain jurisdictions + and update the displayed total. See below for privacy considerations + regarding <a href="#user-info">exposing user information</a>. + </dd> + <dt> + <dfn>requestBillingAddressParts</dfn> member + </dt> + <dd> + A sequence of <a>AddressParts</a> that specifies which fine-grained + parts of the billing address that the payee wishes to receive when the + <a>requestBillingAddress</a> boolean is true. An empty list means no ```suggestion <a>requestBillingAddress</a> member is true. An empty list means no ``` > + The <a>user agent</a> returns the billing address as part of the + {{PaymentMethodChangeEvent}}'s <a>methodDetails</a> attribute and the + {{PaymentResponse}}'s <a>details</a> attribute. A payee can use this + information to, for example, calculate tax in certain jurisdictions + and update the displayed total. See below for privacy considerations + regarding <a href="#user-info">exposing user information</a>. + </dd> + <dt> + <dfn>requestBillingAddressParts</dfn> member + </dt> + <dd> + A sequence of <a>AddressParts</a> that specifies which fine-grained + parts of the billing address that the payee wishes to receive when the + <a>requestBillingAddress</a> boolean is true. An empty list means no + restriction, i.e. the payee requests the full billing address. This + field has no effect if the <a>requestBillingAddress</a> boolean is ```suggestion field has no effect if the <a>requestBillingAddress</a> member is ``` > + <dfn>requestBillingAddressParts</dfn> member + </dt> + <dd> + A sequence of <a>AddressParts</a> that specifies which fine-grained + parts of the billing address that the payee wishes to receive when the + <a>requestBillingAddress</a> boolean is true. An empty list means no + restriction, i.e. the payee requests the full billing address. This + field has no effect if the <a>requestBillingAddress</a> boolean is + false. + <aside class="note"> + The billing address is part of the payment-method specific response. + The <a>user agent</a> simply passes the payee-specified + <a>requestBillingAddressParts</a> to the payment handler. It is the + responsibility of the payment handler to ensure no more than the + requested pieces is returned. A payment handler may choose to not + return some requested pieces (e.g., as a result of applying a redact Nit: `<a>` around "redact list" > + <dd> + A sequence of <a>AddressParts</a> that specifies which fine-grained + parts of the billing address that the payee wishes to receive when the + <a>requestBillingAddress</a> boolean is true. An empty list means no + restriction, i.e. the payee requests the full billing address. This + field has no effect if the <a>requestBillingAddress</a> boolean is + false. + <aside class="note"> + The billing address is part of the payment-method specific response. + The <a>user agent</a> simply passes the payee-specified + <a>requestBillingAddressParts</a> to the payment handler. It is the + responsibility of the payment handler to ensure no more than the + requested pieces is returned. A payment handler may choose to not + return some requested pieces (e.g., as a result of applying a redact + list). The payee can request additional pieces incrementally using + <a>PaymentMethodChangeEvent.requestBillingAddress()</a>. ```suggestion {{PaymentMethodChangeEvent}}'s {{PaymentMethodChangeEvent/requestBillingAddres()}}. ``` > @@ -2133,10 +2259,37 @@ <h2> </dt> <dd> A boolean that indicates whether the <a>user agent</a> SHOULD collect - and return a shipping address as part of the payment request. For - example, this would be set to true when physical goods need to be - shipped by the merchant to the user. This would be set to false for - the purchase of digital goods. + and return a shipping address as part of the payment request (e.g., + when physical goods need to be shipped by the payee to the user). The + <a>user agent</a> returns the shipping address in the + {{PaymentRequest}}'s <a data-lt="PaymentRequest.shippingAddress"> ```suggestion {{PaymentRequest}}'s {{PaymentRequest/shippingAddress}} ``` > @@ -2133,10 +2259,37 @@ <h2> </dt> <dd> A boolean that indicates whether the <a>user agent</a> SHOULD collect - and return a shipping address as part of the payment request. For - example, this would be set to true when physical goods need to be - shipped by the merchant to the user. This would be set to false for - the purchase of digital goods. + and return a shipping address as part of the payment request (e.g., + when physical goods need to be shipped by the payee to the user). The + <a>user agent</a> returns the shipping address in the + {{PaymentRequest}}'s <a data-lt="PaymentRequest.shippingAddress"> + shippingAddress</a> attribute and the {{PaymentResponse}}'s ```suggestion attribute and the {{PaymentResponse}}'s ``` > @@ -2133,10 +2259,37 @@ <h2> </dt> <dd> A boolean that indicates whether the <a>user agent</a> SHOULD collect - and return a shipping address as part of the payment request. For - example, this would be set to true when physical goods need to be - shipped by the merchant to the user. This would be set to false for - the purchase of digital goods. + and return a shipping address as part of the payment request (e.g., + when physical goods need to be shipped by the payee to the user). The + <a>user agent</a> returns the shipping address in the + {{PaymentRequest}}'s <a data-lt="PaymentRequest.shippingAddress"> + shippingAddress</a> attribute and the {{PaymentResponse}}'s + <a data-lt="PaymentResponse.shippingAddress">shippingAddress</a> ```suggestion {{PaymentResponse/shippingAddress}} ``` > + A payee may not need the full shipping address of a user until the + payment request is completed. For example, just having "country" + may be sufficient to calculate shipping cost. A payee sensitive to + the user's privacy can use <a>requestShippingAddressParts</a> to + restrict the initial set of address parts returned by the + <a>user agent</a>. + </p> + </aside> + </dd> + <dt> + <dfn>requestShippingAddressParts</dfn> member + </dt> + <dd> + A sequence of <a>AddressParts</a> that specifies which fine-grained + parts of the shipping address that the payee wishes to receive when + the <a>requestShipping</a> boolean is true. An empty list means no ```suggestion the <a>requestShipping</a> member is true. An empty list means no ``` > + the user's privacy can use <a>requestShippingAddressParts</a> to + restrict the initial set of address parts returned by the + <a>user agent</a>. + </p> + </aside> + </dd> + <dt> + <dfn>requestShippingAddressParts</dfn> member + </dt> + <dd> + A sequence of <a>AddressParts</a> that specifies which fine-grained + parts of the shipping address that the payee wishes to receive when + the <a>requestShipping</a> boolean is true. An empty list means no + restriction, i.e. the <a>user agent</a> will return the full shipping + address, subject to any applicable redact list. This field has no + effect if the <a>requestShipping</a> boolean is false. The payee can ```suggestion effect if the <a>requestShipping</a> member is false. The payee can ``` > + <a>user agent</a>. + </p> + </aside> + </dd> + <dt> + <dfn>requestShippingAddressParts</dfn> member + </dt> + <dd> + A sequence of <a>AddressParts</a> that specifies which fine-grained + parts of the shipping address that the payee wishes to receive when + the <a>requestShipping</a> boolean is true. An empty list means no + restriction, i.e. the <a>user agent</a> will return the full shipping + address, subject to any applicable redact list. This field has no + effect if the <a>requestShipping</a> boolean is false. The payee can + request additional pieces incrementally using + <a>PaymentRequestUpdateEvent.requestShippingAddress()</a>. ```suggestion {{PaymentRequestUpdateEvent}}'s {{PaymentRequestUpdateEvent/requestShippingAddress()}}. ``` > @@ -2937,9 +3098,9 @@ <h2> </li> </ol> </li> - <li>If "sortingCode" is not in |redactList|, set - |details|["<a>sortingCode</a>"] to the user-provided sorting code, or - to the empty string if none was provided. + <li>If "sortingCode" is n |requestedParts| and is not in |redactList|, ```suggestion <li>If "sortingCode" is in |requestedParts| and is not in |redactList|, ``` > @@ -3915,6 +4077,72 @@ <h2> more information. </p> </section> + <section> + <h2> + <dfn data-lt="requestBillingAddress(addressParts)">requestBillingAddress()</dfn> method + </h2> + <aside class="note"> + <p> + <a>PaymentMethodChangeEvent.requestBillingAddress()</a> allows the ```suggestion The {{PaymentMethodChangeEvent/requestBillingAddress()}} method allows the ``` > @@ -3915,6 +4077,72 @@ <h2> more information. </p> </section> + <section> + <h2> + <dfn data-lt="requestBillingAddress(addressParts)">requestBillingAddress()</dfn> method + </h2> + <aside class="note"> + <p> + <a>PaymentMethodChangeEvent.requestBillingAddress()</a> allows the + developer to incrementally request additional pieces of the + billing address that were not initially requested in + {{ PaymentOptions.requestBillingAddressParts }}. + </p> + <pre class="example js" title="how to use `requestBillingAddress()` correct."> ```suggestion <pre class="example js" title="how to use `requestBillingAddress()`."> ``` > + const promiseForUpdate = getNewTotal(methodDetails); + return promiseForUpdate; + }); + ev.updateWith(promiseForUpdatingTax); + }; + </pre> + </aside> + <p> + The <a>requestBillingAddress(|addressParts|)</a> method MUST act as + follows: + </p> + <ol class="algorithm"> + <li>Let |event:PaymentMethodChangeEvent| be this + {{PaymentMethodChangeEvent}} instance. + </li> + <li>If |event|'s {{ Event.isTrusted }} attribute is false, then ```suggestion <li>If |event|'s {{ Event/isTrusted }} attribute is false, then ``` > + follows: + </p> + <ol class="algorithm"> + <li>Let |event:PaymentMethodChangeEvent| be this + {{PaymentMethodChangeEvent}} instance. + </li> + <li>If |event|'s {{ Event.isTrusted }} attribute is false, then + return <a>a promise rejected with</a> an {{"InvalidStateError"}} + {{DOMException}}. + </li> + <li>Let |request:PaymentRequest| be the value of |event|'s + [=Event/target=]. + </li> + <li>Assert: |request| is an instance of {{PaymentRequest}}. + </li> + <li>If |request|.<a>[[\state]]</a> is not "<a>interactive</a>", It's not clear what happens if it's called multiple times: ```JS ev.requestBillingAddress(["country"]); ev.requestBillingAddress(["postcode"]); ``` Or called in a different order: ```JS ev.updateWith(somePromise); // [[waitForUpdate]] is true now... ev.requestBillingAddress(["country"]); ``` And there might also be: ```JS await ev.updateWith(somePromise); ev.requestBillingAddress(["country"]); // invalid state I guess ``` So we might need some kind of internal state to prevent some of the above... requestBillingAddress() can only happen before updateWith() and requestBillingAddress() can only be called once. > + // Discard the returned promise because the full address is not + // needed until the final PaymentResponse. Empty addressParts + // means all parts are requested. + ev.requestShippingAddress(/* addressParts= */[]); + } + </pre> + </aside> + <p> + The <a>requestShippingAddress(|addressParts|)</a> method MUST act as + follows: + </p> + <ol class="algorithm"> + <li>Let |event:PaymentRequestUpdateEvent| be this + {{PaymentRequestUpdateEvent}} instance. + </li> + <li>If |event|'s {{ Event.type }} attribute is not <a> ```suggestion <li>If |event|'s {{ Event/type }} attribute is not <a> ``` > + The <a>requestShippingAddress(|addressParts|)</a> method MUST act as + follows: + </p> + <ol class="algorithm"> + <li>Let |event:PaymentRequestUpdateEvent| be this + {{PaymentRequestUpdateEvent}} instance. + </li> + <li>If |event|'s {{ Event.type }} attribute is not <a> + shippingaddresschange</a>, then return <a>a promise rejected with + </a> a {{TypeError}}. + <li>Let |request:PaymentRequest| be the value of |event|'s + [=Event/target=]. + </li> + <li>Assert: |request| is an instance of {{PaymentRequest}}. + </li> + <li>If |event|'s {{ Event.isTrusted }} attribute is false, then ```suggestion <li>If |event|'s {{ Event/isTrusted }} attribute is false, then ``` -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/w3c/payment-request/pull/873#pullrequestreview-307665796
Received on Monday, 28 October 2019 08:14:27 UTC