793329 - If the trusted UI is closed on device, fire an onerror callback in mozPay indicating as such

Status: VERIFIED FIXED

Description

[Jason Smith [:jsmith]]

If a user cancels the trusted UI dialog during a payment through mozPay, we don't fire any callback at all. We should satisfy the use case of that when the trusted UI is canceled by the user, we provide context in JS through an onerror callback that the trusted UI was canceled (and by caveat, the payment was canceled.

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

This is a Gaia issue, not a platform issue. I've filed a bug for it in https://github.com/mozilla-b2g/gaia/issues/5085

Comment 2

[Jason Smith [:jsmith]]

At this point, we're now doing feature tracking now for v1 in bugzilla, so i'm moving this to boottogecko --> gaia.

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

I was wrong, we need platform work for this bug.

Comment 4

[Jason Smith [:jsmith]]

Spoke with ferjm about this - there's no work-around if a developer to know if the trusted UI is closed. Noming to block.

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

Created attachment 669920 [details] [diff] [review]
v1

Basically, the important part of the patch is the addition of a new event listener for a mozContentEvent containing an 'errorMsg' parameter after starting the payment flow (PaymentGlue.js:134).

The rest is just some clean-up to avoid checking for the error callback each time that we want to notify about errors and the deletion of some "removeEventListener" calls that IMO shouldn't be there.

Comment 6

[[:fabrice] Fabrice Desré]

Comment on attachment 669920 [details] [diff] [review]
v1

Review of attachment 669920 [details] [diff] [review]:
-----------------------------------------------------------------

::: b2g/components/PaymentGlue.js
@@ +142,5 @@
> +      if (evt.detail.errorMsg) {
> +        _error(evt.detail.errorMsg);
> +        content.removeEventListener("mozContentEvent", closePayFlow);
> +        return;
> +      }

So if the payment flow is not canceled, we never remove this event listener... Is there a wa we could swnd a "cleanup" call to the glue?

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Created attachment 670346 [details] [diff] [review]
v2

I've added a 'cleanUp' function to PaymentUIGlue so we can remove the listeners once the payment flow is closed in any case.

Comment 8

[[:fabrice] Fabrice Desré]

Comment on attachment 670346 [details] [diff] [review]
v2

Review of attachment 670346 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with nits addressed

::: b2g/chrome/content/payment.js
@@ +68,5 @@
>                                  closePaymentFlowReturn);
> +
> +    let glue = Cc["@mozilla.org/payment/ui-glue;1"]
> +               .createInstance(Ci.nsIPaymentUIGlue);
> +     glue.cleanUp();

nit: align .createInstance with [
Also, the indentation looks wrong on the |glue.| line

::: dom/payment/interfaces/nsIPaymentUIGlue.idl
@@ +16,4 @@
>  interface nsIPaymentUIGlue : nsISupports
>  {
>      // The 'paymentRequestsInfo' contains the payment request information
>      // for each JWT provided via navigator.mozPay call.    

Nit: trailing whitespace.

@@ +23,5 @@
>  
>      void showPaymentFlow(in nsIPaymentFlowInfo paymentFlowInfo,
>                           in nsIPaymentUIGlueCallback errorCb);
> +
> +    void cleanUp();

can you rename this to |cleanup()| ?

Comment 9

[Fernando Jiménez Moreno [:ferjm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ef672af86f8f

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ef672af86f8f

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/16d70275df8f

Comment 12

[Jason Smith [:jsmith]]

Verified on 10/23 build.