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

VERIFIED FIXED

Status

defect
P2
normal
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: jsmith, Assigned: ferjm)

Tracking

unspecified
ARM
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:+, firefox18 fixed, firefox19 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
Blocks: 767818
Assignee: nobody → ferjmoreno
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
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → WONTFIX
At this point, we're now doing feature tracking now for v1 in bugzilla, so i'm moving this to boottogecko --> gaia.
Status: RESOLVED → UNCONFIRMED
Ever confirmed: false
Resolution: WONTFIX → ---
Status: UNCONFIRMED → NEW
Ever confirmed: true
Component: DOM: Device Interfaces → Gaia
Product: Core → Boot2Gecko
Version: Trunk → unspecified
Whiteboard: [system/trusted_dialog]
blocking-basecamp: --- → ?
Blocks: marketplace-payments
No longer blocks: basecamp-payments
I was wrong, we need platform work for this bug.
Component: Gaia → General
Whiteboard: [system/trusted_dialog]
blocking-basecamp: ? → ---
Spoke with ferjm about this - there's no work-around if a developer to know if the trusted UI is closed. Noming to block.
blocking-basecamp: --- → ?
blocking-basecamp: ? → +
Posted patch v1 (obsolete) — Splinter Review
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.
Attachment #669920 - Flags: review?(fabrice)
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?
Attachment #669920 - Flags: review?(fabrice) → feedback+
Posted patch v2Splinter Review
I've added a 'cleanUp' function to PaymentUIGlue so we can remove the listeners once the payment flow is closed in any case.
Attachment #669920 - Attachment is obsolete: true
Attachment #670346 - Flags: review?(fabrice)
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()| ?
Attachment #670346 - Flags: review?(fabrice) → review+
Blocks: TrustedUI
Priority: -- → P2
https://hg.mozilla.org/mozilla-central/rev/ef672af86f8f
Status: NEW → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Keywords: verifyme
QA Contact: jsmith
Verified on 10/23 build.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Blocks: basecamp-payments
No longer blocks: marketplace-payments
No longer blocks: TrustedUI
You need to log in before you can comment on or make changes to this bug.