- From: Marcos Cáceres <notifications@github.com>
- Date: Tue, 28 Nov 2017 09:33:32 +0000 (UTC)
- To: w3c/payment-handler <payment-handler@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <w3c/payment-handler/pull/207/review/79429615@github.com>
marcoscaceres requested changes on this pull request.
This is a good start. I think we can get Payment Request to do some of the work tho.
> @@ -517,7 +527,11 @@ <h2>
partial interface ServiceWorkerRegistration {
readonly attribute PaymentManager paymentManager;
};
- </pre>
+ </pre>
+ <p>
+ The <a>paymentManager</a> attribute exposes payment handler
Shouldn't this be a `dfn`?
> @@ -1641,6 +1646,137 @@ <h2>
</ol>
</section>
</section>
+ <section id="abort">
+ <h2>
+ Aborting an invocation
+ </h2>
+ <p>
This paragraph is redundant. This should be in an algorithm somewhere (probably is, just haven't gotten to it).
> @@ -1641,6 +1646,137 @@ <h2>
</ol>
</section>
</section>
+ <section id="abort">
+ <h2>
+ Aborting an invocation
+ </h2>
+ <p>
+ If the <a>payee</a> uses [[!payment-request]] method <a>abort()</a>,
+ the <a>user agent</a> fires <a>AbortPaymentEvent</a> in the <a>payment
+ handler</a> that is currently handling <a>PaymentRequestEvent</a>.
+ There is no expectation that the <a>payment handler</a> would roll back
+ a transaction or issue a refund. (This is out of scope.) The <a>payment
Note, this whole talk of "refunds" and other "money talk" is not ideal. We should only talk about how we manipulate things in the API - but not go into this kind of detail. We can describe "here is how you do a refund" in tutorial material.
> + <h2>
+ Aborting an invocation
+ </h2>
+ <p>
+ If the <a>payee</a> uses [[!payment-request]] method <a>abort()</a>,
+ the <a>user agent</a> fires <a>AbortPaymentEvent</a> in the <a>payment
+ handler</a> that is currently handling <a>PaymentRequestEvent</a>.
+ There is no expectation that the <a>payment handler</a> would roll back
+ a transaction or issue a refund. (This is out of scope.) The <a>payment
+ handler</a> should respond with a boolean that indicates whether the
+ abort was successful.
+ </p>
+ <p>
+ Implementations may impose a timeout for developers to respond to the
+ <a>AbortPaymentEvent</a>. If the timeout expires, then the
+ implementation will behave as if <a data-lt=
Nit: there is no need to use `data-lt` :) Just add `data-link-for="AbortPaymentEvent"` in a parent element (either the paragraph or the parent `<section>`).
> + </h2>
+ <p>
+ This specification extends the <a>ServiceWorkerGlobalScope</a>
+ interface.
+ </p>
+ <pre class="idl">
+ partial interface ServiceWorkerGlobalScope {
+ attribute EventHandler onabortpayment;
+ };
+ </pre>
+ <section>
+ <h2>
+ <dfn>onabortpayment</dfn> attribute
+ </h2>
+ <p>
+ The <a>onabortpayment</a> attribute is an <a>event handler</a>
Nit: `<a>event handler</a>` should be `<a>event handler attribute</a>` and linked to HTML correctly.
> + This specification extends the <a>ServiceWorkerGlobalScope</a>
+ interface.
+ </p>
+ <pre class="idl">
+ partial interface ServiceWorkerGlobalScope {
+ attribute EventHandler onabortpayment;
+ };
+ </pre>
+ <section>
+ <h2>
+ <dfn>onabortpayment</dfn> attribute
+ </h2>
+ <p>
+ The <a>onabortpayment</a> attribute is an <a>event handler</a>
+ whose corresponding <a>event handler event type</a> is
+ abortpayment.
`abortpayment` should be in quotes
> + <section>
+ <h2>
+ <dfn>respondWith()</dfn> method
+ </h2>
+ <p>
+ This method is used by the payment handler to indicate whether it
+ was able to abort the payment.
+ </p>
+ </section>
+ </section>
+ <section>
+ <h2>
+ Handling an AbortPaymentEvent
+ </h2>
+ <p>
+ Upon <a data-cite=
Nit: we should try to be consistent with how we reference PaymentRequest.show/abort/etc. See the fix I made above, which gives you the nice linking both the PaymentRequest interface and the method itself..
> + </p>
+ </section>
+ </section>
+ <section>
+ <h2>
+ Handling an AbortPaymentEvent
+ </h2>
+ <p>
+ Upon <a data-cite=
+ "!payment-request#abort-method">PaymentRequest.abort()</a>, the
+ <a>user agent</a> MUST run the following steps:
+ </p>
+ <ol>
+ <li>Let <var>registration</var> be the
+ <a>ServiceWorkerRegistration</a> corresponding to the
+ <a>PaymentRequestEvent</a> that is being processed.
Q: How are these linked together? Is there some more explicit means to get at the registration from the event?
> + </section>
+ <section>
+ <h2>
+ Handling an AbortPaymentEvent
+ </h2>
+ <p>
+ Upon <a data-cite=
+ "!payment-request#abort-method">PaymentRequest.abort()</a>, the
+ <a>user agent</a> MUST run the following steps:
+ </p>
+ <ol>
+ <li>Let <var>registration</var> be the
+ <a>ServiceWorkerRegistration</a> corresponding to the
+ <a>PaymentRequestEvent</a> that is being processed.
+ </li>
+ <li>If <var>registration</var> is not found, reject the
"not found" makes is sounds like some kind of lookup took place. Maybe we want:
1. Let registration be (... steps to get the registration....)
1. If registration is null, then...
> + <section>
+ <h2>
+ Handling an AbortPaymentEvent
+ </h2>
+ <p>
+ Upon <a data-cite=
+ "!payment-request#abort-method">PaymentRequest.abort()</a>, the
+ <a>user agent</a> MUST run the following steps:
+ </p>
+ <ol>
+ <li>Let <var>registration</var> be the
+ <a>ServiceWorkerRegistration</a> corresponding to the
+ <a>PaymentRequestEvent</a> that is being processed.
+ </li>
+ <li>If <var>registration</var> is not found, reject the
+ <a>Promise</a> that was created by <a data-cite=
So, I don't think we can do this... we need to hook back into Payment Request here - but we can't reach across threads and reject the promise?
> + "!payment-request#abort-method">PaymentRequest.abort()</a>, the
+ <a>user agent</a> MUST run the following steps:
+ </p>
+ <ol>
+ <li>Let <var>registration</var> be the
+ <a>ServiceWorkerRegistration</a> corresponding to the
+ <a>PaymentRequestEvent</a> that is being processed.
+ </li>
+ <li>If <var>registration</var> is not found, reject the
+ <a>Promise</a> that was created by <a data-cite=
+ "!payment-request#abort-method">PaymentRequest.abort()</a> with false
+ and terminate these steps.
+ </li>
+ <li>Invoke the <a>handle functional event</a> algorithm with a
+ <a>ServiceWorkerRegistration</a> of <var>registration</var> and <var>
+ callbackSteps</var> set to the following steps:
Maybe we want some kinda of task here? This looks like a task.
> + <a>user agent</a> MUST run the following steps:
+ </p>
+ <ol>
+ <li>Let <var>registration</var> be the
+ <a>ServiceWorkerRegistration</a> corresponding to the
+ <a>PaymentRequestEvent</a> that is being processed.
+ </li>
+ <li>If <var>registration</var> is not found, reject the
+ <a>Promise</a> that was created by <a data-cite=
+ "!payment-request#abort-method">PaymentRequest.abort()</a> with false
+ and terminate these steps.
+ </li>
+ <li>Invoke the <a>handle functional event</a> algorithm with a
+ <a>ServiceWorkerRegistration</a> of <var>registration</var> and <var>
+ callbackSteps</var> set to the following steps:
+ <ol>
Yeah, we shouldn't need to do any of this... Payment Request should do this for us. We just need to tell Payment Request that we have aborted, and it should do all this, no?
> + and terminate these steps.
+ </li>
+ <li>Invoke the <a>handle functional event</a> algorithm with a
+ <a>ServiceWorkerRegistration</a> of <var>registration</var> and <var>
+ callbackSteps</var> set to the following steps:
+ <ol>
+ <li>Set <var>global</var> to the global object that was provided
+ as an argument.
+ </li>
+ <li>Create a <a>trusted event</a>, <var>e</var>, that uses the
+ <a>AbortPaymentEvent</a> interface, which does not bubble, cannot
+ be canceled, and has no default action.
+ </li>
+ <li>Dispatch <var>e</var> to <var>global</var>.
+ </li>
+ <li>Wait for all of the promises in the <a>extend lifetime
This feels out of place to me. Shouldn't we get this for free from .waitUntil() without needing to say anything? Hopefully this whole task can go away.
> + as an argument.
+ </li>
+ <li>Create a <a>trusted event</a>, <var>e</var>, that uses the
+ <a>AbortPaymentEvent</a> interface, which does not bubble, cannot
+ be canceled, and has no default action.
+ </li>
+ <li>Dispatch <var>e</var> to <var>global</var>.
+ </li>
+ <li>Wait for all of the promises in the <a>extend lifetime
+ promises</a> of <var>e</var> to resolve.
+ </li>
+ <li>If the <a>payment handler</a> has not provided a boolean
+ response, reject the <a>Promise</a> that was created by
+ <a data-cite=
+ "!payment-request#abort-method">PaymentRequest.abort()</a> with a
+ <a>DOMException</a> whose value "<a>OperationError</a>".
You can remove this whole thing. Payment Request doesn't know anything about "OperationError"s and WebIDL will just coerce anything you send into `.respondeWith()` into a boolean.
> + as an argument.
+ </li>
+ <li>Create a <a>trusted event</a>, <var>e</var>, that uses the
+ <a>AbortPaymentEvent</a> interface, which does not bubble, cannot
+ be canceled, and has no default action.
+ </li>
+ <li>Dispatch <var>e</var> to <var>global</var>.
+ </li>
+ <li>Wait for all of the promises in the <a>extend lifetime
+ promises</a> of <var>e</var> to resolve.
+ </li>
+ <li>If the <a>payment handler</a> has not provided a boolean
+ response, reject the <a>Promise</a> that was created by
+ <a data-cite=
+ "!payment-request#abort-method">PaymentRequest.abort()</a> with a
+ <a>DOMException</a> whose value "<a>OperationError</a>".
...anyway, I'm kinda sure this whole thing needs to go away. Fingers crossed!
--
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/207#pullrequestreview-79429615
Received on Tuesday, 28 November 2017 09:34:00 UTC