w3c / payment-request

Payment Request API
https://www.w3.org/TR/payment-request/
Other
486 stars 133 forks source link

Rethink "payment request is showing" boolean #809

Closed marcoscaceres closed 5 years ago

marcoscaceres commented 5 years ago

I think we should reconsider how "payment request is showing" boolean works and perhaps leave it to the user agent (or the payment handler, rather).

For example, in Firefox, for "basic-card", there is no real reason why one shouldn't be able to have multiple payment sheets up for different tabs/windows at the same time.

However, for payment handlers like Apple Pay, where authentication is required either via an Apple Watch or an iPhone, it makes sense to only allow one payment sheet at a time (because there is a strong tie between the authentication device and the payment sheet being shown).

Thus, I think what the spec currently states is incorrect: right now, if Sita A is showing a payment sheet, and Site B tries to show a payment sheet, Site B's payment request gets rejected.

What I think we want is: if required by the Payment Handler, if Sita A is showing a payment sheet, and Site B tries to show a payment sheet, then Site A's payment gets aborted.

cc @aestes, @rsolomakhin, @zkoch, @mnoorenberghe...

ianbjacobs commented 5 years ago

Hi Marcos,

I don't think this should be marked as CR-Exit. Can we revisit this after v1 ships?

Ian

marcoscaceres commented 5 years ago

We can, but I recall this being a thing that we were going to explore explicitly during CR.

If we make the change, I don’t think it should affect our ability to pass CR... but need to check what browsers do.

Firefox probably won’t conform to the spec as currently specified.

rsolomakhin commented 5 years ago

I'm OK with this change.

adrianhopebailie commented 5 years ago

@marcoscaceres I would suggest we don't make this a CR-Exit issue as it is adding complexity that we haven't really explored and could be bigger than we think.

Example:

mnoorenberghe commented 5 years ago

The currently specified behaviour leads to a really bad/confusing experience when comparison shopping so I agree the spec should change. Firefox doesn't plan to require only one active payment request because of the bad UX it would provide so we are wilfully violating the spec currently and because of this I think it should be a CR-exit issue.

What I think we want is: if required by the Payment Handler, if Site A is showing a payment sheet, and Site B tries to show a payment sheet, then Site A's payment gets aborted.

That is also bad because it causes dataloss for anything the user had typed into the Payment Request dialog that wasn't saved into persistent storage. I think that's better than the current state since basic-card wouldn't have this requirement but I think it wouldn't be a good behaviour for any payment method that could have dataloss if it gets randomly aborted by another tab.

For v.1 it may make sense to just remove the single "payment request is showing" requirement from the spec altogether.

ianbjacobs commented 5 years ago

My sense is that we should keep it simple for now. The user opens a sheet and pays. There may be use cases (e.g., comparative shopping) for multiple sheets open, but that seems like something we would learn once the simple version has been more broadly deployed.

Ian

marcoscaceres commented 5 years ago

We still need a boolean that represents “a sheet is already showing on this page or an iframe”, so the change is to just change it from a global check to a local check.

ianbjacobs commented 5 years ago

@marcoscaceres, the spec currently says:

"Because the simultaneous display of multiple PaymentRequest user interfaces might confuse the user, this specification limits the user agent to displaying one at a time via the show() method. This is ensured by a payment request is showing boolean."

Would this change work for you:

Ian

marcoscaceres commented 5 years ago

@mnoorenberghe wrote:

That is also bad because it causes dataloss for anything the user had typed into the Payment Request dialog that wasn't saved into persistent storage.

Potentially, yes. But it's Handler dependent and there are a few guards in place: consider, a web page can't arbitrarily cause another sheet to be closed down without the user explicitly clicking on another pay button.

Also, there is no reason that the payment sheet that is being shut down couldn't keep the data in the sheet in a temporary state (and restore the state if the user hits pay again... so its mostly a UX problem, I feel).

Thus, I think we should leave it to handlers to deal with it (e.g., Apple Pay deals with it in it's own way, for valid reasons). Firefox will deal with it differently for "basic-card". And so on...

@adrianhopebailie wrote:

I would suggest we don't make this a CR-Exit issue as it is adding complexity that we haven't really explored and could be bigger than we think... Examples:...

These are great questions, but I think we need to address them as part of Payment Handler.

The problem is that we have a spec requirement that implementations are not following and/or willfully violating - so we won't be able to move out of CR unless we address this somehow.

domenic commented 5 years ago

The problem is that we have a spec requirement that implementations are not following and/or willfully violating - so we won't be able to move out of CR unless we address this somehow.

Strong +1; we should not rush through process steps while ignoring implementer feedback.

adrianhopebailie commented 5 years ago

Sounds like the consensus is to at least drop the restriction as a CR-Exit requirement, is that correct?

I think we can do that in a way that ensures the complexity of parallel requests is dealt with in PH API spec not PR API. (I.e. the flag is simply per context not global)

@marcoscaceres you said:

consider, a web page can't arbitrarily cause another sheet to be closed down without the user explicitly clicking on another pay button.

This seems like a very surprising user experience, performing an action on one tab having an impact on another, no?

In any case, I'm not too concerned about Payment Handlers needing to deal with parallel request, I think it will be trivial for them to do this. I think we can put that aside in deciding what to do with PR API for CR.

Are there any other reasons, besides "user confusion", to limit the browser to only a single global payment sheet?

marcoscaceres commented 5 years ago

@adrianhopebailie wrote:

Sounds like the consensus is to at least drop the restriction as a CR-Exit requirement, is that correct? I think we can do that in a way that ensures the complexity of parallel requests is dealt with in PH API spec not PR API. (I.e. the flag is simply per context not global).

Yes. So a neat thing about the current design is that it handles not showing multiple payments sheets in a single browser window or tab (I know, that would be really silly and no one would do that... but it's a happy coincidence we have that protection in the spec!).

So, by simply changing "user agent's payment request is showing boolean" to "top-level browsing context's payment request is showing boolean" we get the behavior we want 🏆. Then, we just need to add a note that payment handlers might do different things when handling multiple simultaneous requests for payment.

I'll draft something up.

This seems like a very surprising user experience, performing an action on one tab having an impact on another, no?

Depends... again, see what I wrote about Apple Pay. It's not surprising in that context because of how Apple Pay hooks up to a phone or watch. But I can imagine a situation where a user is verifying a payment manually (without a phone or watch), in which case, it could be theoretically possible to show multiple payment sheets in separate browser windows/tabs.

Are there any other reasons, besides "user confusion", to limit the browser to only a single global payment sheet?

No. I can't think of any.

marcoscaceres commented 5 years ago

Ok, PR #811 ready. I've also amended #810 to say that if the Payment Handler itself does the aborting, then the user agent should treat that as if the user has aborted.