search

w3c / secure-payment-confirmation

Secure Payment Confirmation (SPC)
https://w3c.github.io/secure-payment-confirmation/
Other
106 stars 48 forks source link

Add isSecurePaymentConfirmationAvailable API to spec #233

Closed nickburris closed 1 year ago

nickburris commented 1 year ago

This adds spec text for an API for developers to more easily check whether SPC is available. Fixes #81. One question is whether the spec should further clarify what it means for SPC to be available, but I'm not sure how we can say that without being too implementation specific.

Preview | Diff

ianbjacobs commented 1 year ago

Thanks @nickburris and @stephenmcgruer,

I don't have a strong view; let's try to get input from users of the API.

Having said that, I see some small advantages to isSecurePaymentConfirmationAvailable:

1) isSecurePaymentConfirmationAvailable appears to be more straightforward in meaning than canMakePayment. 2) This new approach might ease a transition away from SPC-via-Payment-Request if we choose that path.

stephenmcgruer commented 1 year ago

I don't have a strong view; let's try to get input from users of the API.

How/when do you want to gather this input?

Having said that, I see some small advantages to isSecurePaymentConfirmationAvailable: [...]

Yes, I think we definitely want to make isSecurePaymentConfirmationAvailable the primary way to do availability-checking for the API. The question is more "what about canMakePayment" in such a world - should the SPC spec try to address it (either by defining it to do 'something' useful, TBD what, or by explicitly saying not to use it for SPC)?

ianbjacobs commented 1 year ago

I don't have a strong view; let's try to get input from users of the API.

How/when do you want to gather this input?

I have reached out to Stripe and Adyen colleagues for input. I'll also add this to our next meeting agenda.

Having said that, I see some small advantages to isSecurePaymentConfirmationAvailable: [...]

Yes, I think we definitely want to make isSecurePaymentConfirmationAvailable the primary way to do availability-checking for the API. The question is more "what about canMakePayment" in such a world - should the SPC spec try to address it (either by defining it to do 'something' useful, TBD what, or by explicitly saying not to use it for SPC)?

While SPC is grounded in PR API I don't see a reason to try to get rid of canMakePayment. So would this make sense language for users of the API:

stephenmcgruer commented 1 year ago

I have reached out to Stripe and Adyen colleagues for input. I'll also add this to our next meeting agenda.

Ack. Note that we would like to ship this in M113 (branches March 23rd, for stable release in early May), so we are likely to pursue that anyway as we don't foresee any objections from the WG. If for some reason there is significant push-back at the remote meeting (week of March 27th), we should be fine to halt the brakes that close post-branch.

While SPC is grounded in PR API I don't see a reason to try to get rid of canMakePayment. So would this make sense language for users of the API:

Yep, sgtm. Practically, what I believe this amounts to is:

  1. Spec isSecurePaymentConfirmationAvailable
  2. Add a spec note saying "canMakePayment also exists, but we recommend using isSecurePaymentConfirmationAvailable instead"

Does that SGTY @ianbjacobs ?

omertoast commented 1 year ago
  1. Spec isSecurePaymentConfirmationAvailable
  2. Add a spec note saying "canMakePayment also exists, but we recommend using isSecurePaymentConfirmationAvailable instead"

SGTM. As someone who didn't know this issue exist, it seems straightforward and simple as it gets.

adrianhopebailie commented 1 year ago

until the mythical day when SPC is no longer grounded in PR API

< sigh />

ianbjacobs commented 1 year ago

until the mythical day when SPC is no longer grounded in PR API

< sigh />

It's an open issue; sorry I was a bit blithe in my comment.

nickburris commented 1 year ago

Can you update the example in section 1.2.2 to: (1) use your const spcAvailable = ..., and to (2) remove or update the comment about canMakePayment ?

Done.

stephenmcgruer commented 1 year ago

@ianbjacobs - PTAL as well. Let us know also if you want this to wait for a WPWG meeting to let anyone chime in, or if you're happy to see it merged after your review.

ianbjacobs commented 1 year ago

Hi @nickburris,

I've approved the request, but this should not be part of the document that we advance to Candidate Recommendation. I do expect us to fold in other pull requests that will be part of the CR (namely, #230, #234, #236, and #238). Should we block merging this for now?

stephenmcgruer commented 1 year ago

Should we block merging this for now?

Yep; let's just leave this PR sitting until the first version of SPC gets to CR (assuming no significant bumps along the way). On the Chrome side, an open approved PR is sufficient for our launch processes :)

RByers commented 1 year ago

[Chromium API owner hat on] Do y'all have an idea of how long you expect SPC to take to get to CR? While we can ship based on a PR if necessary, it's not ideal as it can contribute to confusion about what's ready to be implemented in other engines. If you expect it to take more than a month or so, could you consider creating a separate 'v2' branch or something which we can point to as the landed spec for this API?

ianbjacobs commented 1 year ago

Hi @RByers, We are poised to request to advance to CR. I am hopeful for a mid-May publication.

RByers commented 1 year ago

Thanks @ianbjacobs. Leaving this as an approved but unlanded PR for a few weeks seems like a reasonable pragmatic tradeoff to me. If it ends up taking more than a month or two, would you be open to the future/v2 branch idea?

ianbjacobs commented 1 year ago

@RByers, yes, very open to a versioning / branch discussion.

stephenmcgruer commented 1 year ago

@ianbjacobs - now that we have a CR version branched (though not approved), what do you think of landing this PR? I don't expect us to have many more changes to bring into the CR branch, and if we do we can cherry-pick them individually from main as needed.

ianbjacobs commented 1 year ago

@stephenmcgruer, I'm ok to land the pull request, but would prefer to do so after the publication of the CR (ideally, therefore, in a few days). Would it be ok to wait for that? If you want to delegate landing this to me (for just after the publication), I'm happy to merge it.

stephenmcgruer commented 1 year ago

Sure, sounds good to me - if you can take the task to land this once the CR is published, SGTM. Thanks! :)