- From: Marijn Kruisselbrink <notifications@github.com>
- Date: Tue, 25 Jul 2017 23:38:23 +0000 (UTC)
- To: w3c/payment-handler <payment-handler@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3c/payment-handler/pull/194/review/52222864@github.com>
mkruisselbrink commented on this pull request. This definitely looks a lot better than what was there before. There still is the weirdness of saying that the user agent MUST run the "payment app failure" algorithm, without any requirement as to what that algorithm does. It's not really a MUST requirement if there aren't any actual requirements. > </p> - <ol> - <li>Set <var>handlerResponse</var> to the - <a>PaymentHandlerResponse</a> instance used to resolve the - <a>PaymentRequestEvent</a>.<a data-lt= - "PaymentRequestEvent.respondWith">respondWith()</a> Promise. + <ol class="algorithm"> + <li>If <var>event</var>'s <a>dispatch flag</a> is unset, then + <a>throw</a> an "<a>InvalidStateError</a>" <a>DOMException</a> and + abort these steps. </li> Should there be a check here, or somewhere else, to make sure respondWith is only called once (i.e. similar to FetchEvent's "respond-with entered flag")? > </h2> <p> - If the Promise is successfully resolved, the user agent MUST run the - <a>user accepts the payment request algorithm</a> as defined in - [[!payment-request]], replacing steps 6 and 7 with these steps or - their equivalent: + When this algorithm with <var>event</var> and + <var>handlerResponsePromise</var> parameters are invoked, the user This should at least be "When this algorithm [...] is invoked", and maybe even re-order it to "When this algorithm is invoked with `event` and `handlerResponsePromise` parameters, " > + "PaymentHandlerResponse.details">details</a> is not present + or not <a>JSON-serializable</a>, run the <a>payment app + failure algorithm</a> and terminate these steps. + </li> + <li>The user agent MUST run the <a>user accepts the payment + request algorithm</a> as defined in [[!payment-request]], + replacing steps 7 and 8 with these steps or their equivalent. + <ol> + <li>Create a <a>structured clone</a> of + <var>handlerResponse</var>.<a data-lt= + "PaymentHandlerResponse.methodName">methodName</a> and + assign it to associated PaymentRequest's <a data-lt= + "PaymentResponse"><var>response</var></a>.<a data-cite= + "!payment-request#dom-paymentresponse-methodname">methodName</a>. + </li> + <li>Create a <a>structured clone</a> of "structured clone" does not exist anymore in the latest HTML spec (because the way it was defined was problematic with which realms things are done in). Instead you'll need to call [StructuredSerialize](http://w3c.github.io/html/infrastructure.html#section-structuredserialize) in the context of the service worker (i.e. before calling the "user accepts the payment request algorithm"), and then [StructuredDeserialize](http://w3c.github.io/html/infrastructure.html#section-structureddeserialize) here, to create javascript objects in the correct realm. One reason for that is because serialization can have observable side effects in the source context, so it needs to be well specified when serialization happens. > </li> - <li>Create a <a>structured clone</a> of - <var>handlerResponse</var>.<var>details</var> and assign it to <var> - response</var>.<var>details</var>. + <li>Perform the remaining steps in parallel: I don't think you need "perform the remaining steps in parallel", or the "wait until promis settles" below. Instead you can just directly have 1. Upon rejection of handlerResponsePromise: 1. Do stuff 1. Upon fulfillment of handlerResponsePromise: 1. Do stuff 1. Upon fulfillment or rejection of handlerResponsePromise queue a microtask to perform the following steps: 1. Do stuf -- 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-handler/pull/194#pullrequestreview-52222864
Received on Tuesday, 25 July 2017 23:38:46 UTC