Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - very rough sketch of requestShippingAddress() method #873

Open
wants to merge 5 commits into
base: gh-pages
from

Conversation

@danyao
Copy link
Collaborator

commented Jun 28, 2019

closes #842

This is a very rough proposal based on @adrianhopebailie's idea from w3c/payment-method-basic-card#72 (comment). It's applied to shipping address instead of billing address.

The following tasks have been completed:

  • Confirmed there are no ReSpec errors/warnings.
  • Modified Web platform tests (link)
  • Modified MDN Docs (link)
  • Has undergone security/privacy review (link)

Implementation commitment:

  • Safari (link to issue)
  • Chrome (link to issue)
  • Firefox (link to issue)
  • Edge (public signal)

Optional, impact on Payment Handler spec?


Preview | Diff

@ianbjacobs

This comment has been minimized.

Copy link
Collaborator

commented Jun 28, 2019

Thank you, @danyao!
cc @samuelweiler

To summarize the proposal:

  • A new requestShippingAddress() method enables the merchant to request partial shipping address information.
  • To incrementally request address data, repeated calls to requestShippingAddress() can be chained with a final promise to compute new details using the address information. A call to updateWith() with the promise chain can be used to update the payment details.
  • For full shipping address information, the merchant can still use the requestShipping boolean. This also helps with backwards compatibility.

We have begun discussions [1] about the delegation of the request for shipping addresses to payment handlers. If the current proposal is adopted, that discussion will need to include a mechanism for the browser to request shipping address information from the payment handler in order to respond to the merchant's incremental request.

As @danyao notes, this proposal does not address billing addresses. Note that:

  • By default, merchants do not receive billing address information; they must explicitly request it (via requestBillingAddress).
  • The Basic Card specification defines a redact list that intends to provide enough information for payment authorization.
  • This proposal does not require changes to Basic Card.

[1] w3c/payment-handler#337

index.html Outdated Show resolved Hide resolved
Copy link
Collaborator

left a comment

Technically this should work.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

I'm not following why we need the incremental increase in granularity? Is that really necessary? It seems like it would make implementing an interface much more difficult, because you'd need to be constantly updating it to add, remove, different address parts... that might be really confusing for users.

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

to be clear, I don't follow why we need this:

To incrementally request address data, repeated calls to requestShippingAddress() can be chained with a final promise to compute new details using the address information. A call to updateWith() with the promise chain can be used to update the payment details.

Why not just ask up front?

@aestes

This comment has been minimized.

Copy link
Collaborator

commented Jul 2, 2019

@marcoscaceres asking for everything up front requires user agents to over-share in some cases. For instance, maybe you offer flat-rate shipping in countryA but variable-per-region shipping in countryB. To cover both cases up front you'd have to ask for country and region, even though if you had only asked for country and gotten back countryA then you didn't need to know the user's region.

So with incremental requests you can start at least-specific and drill down one level at a time (until you reach the redacted levels) until you have just enough to compute a details update and resolve your updateWith() promise.

@marcoscaceres

This comment has been minimized.

Copy link
Member

commented Jul 2, 2019

Ah, I see. Thanks for clarifying @aestes.

@samuelweiler

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

This architecture seems like a good approach for both shipping and billing address.

I would prefer if the "just ask for everything" option were removed - if only as a way to encourage the (admittedly harder to implement) incremental requests. Failing that, I want to see some specific text discouraging the use of the "just ask for everything". Perhaps as a sidebar in section 10? Point out that if potential customer disables it - assuming a UI has such a feature - the merchant will need to fall back to incremental requests anyway or lose the sale. And if they have to implement the incremental anyway, then why keep the "all"?

Whether that boolean is removed or not, there should be some specific text encouraging the use of the incremental approach. That could be in 18.4.2, perhaps as a sidebar. It should also be mentioned in the security considerations, in 20.6.

@aestes Thank you for reiterating the need for this. @joshkaroly, thank you for keeping the pressure on to do this with billing addresses, as well.

@danyao danyao force-pushed the danyao:danyao-requestShippingAddress branch from 4f5fc3e to 6884656 Sep 13, 2019
@danyao danyao force-pushed the danyao:danyao-requestShippingAddress branch from 6884656 to 28f206c Sep 13, 2019
@danyao

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 13, 2019

@samuelweiler I incorporated your comments:

  • Added a note in Section 10 to discourage the use of the boolean and encourage the use of the new requestShippingAddress() method. Also noting potential future deprecation of the boolean.
  • Added similar note to Section 18.4.2
  • Added the detailed algorithm to Section 18.4.2 to highlight now the incremental request interact with the concept of redactList.

I didn't add a note to the Security Considerations in Section 20.7 because that section currently focuses on non-normative recommendations to payment handler implementer to not over share user information (especially billing address) via PaymentMethodChangeEvent. I considered expanding it to highlight the user agent's responsibility to not over share shipping address in PaymentRequestUpdateEvent, but I felt it's redundant with the normative requirements encoded in the algorithm in 18.4.2. I also considered adding a non-normative note to compel the payee to use requestShippingAddress(), but that also seems redundant with the other notes in Section 10 and 18.4.2.

Would you mind taking another look? @marcoscaceres @ianbjacobs @aestes PTAL as well.

Also, as I was writing up the algorithm, I realize that the UX implementation may be tricky and would like to hear what others think. Say a payee initially requests "country", then request "region", should the user agent prompt the user for permission each time? This could be annoying. Should the user agent ask for a blanket permission upfront, e.g. "your shipping information will be shared"? But this is a bit misleading because user perceives more information is being shared than what the payee is actually asking for.

Currently this is only a problem for user agents with built-in payment methods (e.g. ApplePay in Safari and basic-card in Chrome). Soon, payment handlers will face this problem as well when they start to support delegated shipping and contact information (w3c/payment-handler#337).

@adrianhopebailie

This comment has been minimized.

Copy link
Collaborator

commented Oct 7, 2019

@danyao thank you for this work, sorry you couldn't be at TPAC where we discussed the issue at length.

My read of the room was that there is wide support for this but that the group does not want to hold up the process by making this a normative requirement in v1.0 nor remove the existing APIs which are already widely used.

@marcoscaceres @aestes @rsolomakhin @ianbjacobs is there a way this can be marked as an "optional" feature for v1.0 of the spec?

If so, I propose we make this change and I will do a call for consensus from the WG to proceed with v1.0 including this PR and that amendment.

A future version of this specification may remove the all-in-one
{{PaymentOptions.requestShipping}} boolean.
</p>
Comment for lines +4099  – +4101

This comment has been minimized.

Copy link
@ianbjacobs

ianbjacobs Oct 8, 2019

Collaborator

@danyao, I think we should delete this paragraph. It is too unknown at this point.

<p>
A future version of this specification may remove the all-in-one
{{PaymentOptions.requestShipping}} boolean.
</p>

This comment has been minimized.

Copy link
@ianbjacobs

ianbjacobs Oct 8, 2019

Collaborator

See other comment; I think we can remove this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.