938304 - navigator.mozPay(): callbacks do not pass through result

Status: RESOLVED FIXED

(Firefox Graveyard :: Web Apps, defect)

Description

[Kumar McMillan [:kumar]]

STR:
- Install Firefox Nightly on an Android device
- Install the in-app payment tester from http://apploa.de/3b25a (manifest: http://inapp-pay-test.paas.allizom.org/manifest.webapp)
- Open the tester app from the Android home screen
- Edit the JWT textarea to make a simulation. Add {"simulate": {"result": "postback"}} to the request object so it looks something like:

{
  "iss": "323d34dc-b5cf-4822-8e47-6a4515dc74db",
  "aud": "marketplace.firefox.com",
  ...
  "request": {
    ...
    "simulate": {"result": "postback"}
  }
}

- Scroll out of the textarea and tap Pay
- When prompted to continue the simulation, tap the Cancel button

Expected: The payment page will call paymentFailed('USER_CANCELLED') and that error code is available in DOMRequest.error.name. In the above STR you should see 'error: USER_CANCELLED' in the tester app.

Actual: because DOMRequest.error.name is not set, you see 'error: undefined'

Note that this is the behavior for paymentFailed(). In paymentSuccess(), the input variable should become the DOMRequest.result: https://developer.mozilla.org/en-US/docs/Web/API/DOMRequest.result

Here is the complete callback API: https://wiki.mozilla.org/WebAPI/WebPaymentProvider#API

Android callbacks originally added in bug 924693

Comment 1

[Kumar McMillan [:kumar]]

Attachment: expected.png

expected screenshot from Firefox OS

Comment 2

[Kumar McMillan [:kumar]]

Attachment: actual.png

actual screenshot from Android

Comment 3

[Wesley Johnston (:wesj)]

Attachment: Patch

I haven't tested this, but this matches b2g. The desktop winrt has this same bug though.

Comment 4

[Wesley Johnston (:wesj)]

Attachment: desktop patch

This looked good on try:
https://tbpl.mozilla.org/?tree=Try&rev=5f84b2f62815

but I'm not sure if these tests are running there.

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 832042 [details] [diff] [review]
Patch

>diff --git a/mobile/android/components/PaymentsUI.js b/mobile/android/components/PaymentsUI.js

> function paymentSuccess(aRequestId) {
>-  return paymentCallback(aRequestId, "Payment:Success");
>+  return function(aResult) {
>+    closePaymentTab(aRequestId, function() {
>+      cpmm.sendAsyncMessage(aMsg, { result: aResult,

Where does aMsg come from?

> function paymentFailed(aRequestId) {
>-  return paymentCallback(aRequestId, "Payment:Failed");
>-}
>-
>-function paymentCallback(aRequestId, aMsg) {
>-  return function(aResult) {
>+  return function(aErrorMsg) {
>     closePaymentTab(aRequestId, function() {
>-      cpmm.sendAsyncMessage(aMsg, { result: aResult,
>+      cpmm.sendAsyncMessage(aMsg, { errorMsg: aErrorMsg,

Same

Comment 6

[Wesley Johnston (:wesj)]

Attachment: Patch

Whoops. Thanks for catching this. Tested this and seems to work fine :)

Comment 7

[Marco Castelluccio [:marco]]

Comment on attachment 832080 [details] [diff] [review]
desktop patch

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

The webapprt chrome tests work in a quite different way (see the other tests for example).
You should create an application manifest that specifies the launch path (that in this case would be "/webapprtChrome/webapprt/test/chrome/mozpay-failed.html").
There are a couple of syntax errors in browser_mozpay.js.
These tests aren't yet run on try, so you should run them manually.

You can run the tests with the webapprt-test-chrome command ("mach webapprt-test-chrome webapprt/test/chrome/browser_mozpay.js").

Comment 8

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/fx-team/rev/7f5e40e54c11

marking leave-open for the desktop + tests bit.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/7f5e40e54c11

Comment 10

[Andy McKay]

If its being left open for Desktop, moving over to that tracking bug.

Comment 11

[Myk Melez [:myk] [@mykmelez]]

I would prefer to track the desktop work in a separate bug, but in any case we should move this to the desktop product/component now that the only remaining work is for desktop.

Comment 12

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 13

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 8390001 [details] [diff] [review]
Patch

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

::: webapprt/PaymentUIGlue.js
@@ +25,5 @@
>  function paymentFailed(aRequestId) {
> +  return function(aErrorMsg) {
> +    closePaymentWindow(aRequestId, function() {
> +      cpmm.sendAsyncMessage("Payment:Failed", { requestId: aRequestId,
> +                                                 errorMsg: aErrorMsg });

nit: align errorMsg with requestId

Comment 14

[Marco Castelluccio [:marco]]

Attachment: Patch

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/9a2844261651

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9a2844261651