879579 - "Payment failed.." message shown on canceling purchase in a non en-US locale

Status: RESOLVED FIXED

(Marketplace Graveyard :: Consumer Pages, defect, P3)

Description

[krupa raj[:krupa]]

Attachment: screenshot

steps to reproduce:
1. Using the Settings app on the phone, change language to es
2. Start the purchase of an app on dev
3. Mid-purchase, cancel the purchase

expected behavior:
We show the user message "Payment cancelled"

observed behavior:
We still show the old user message-"Payment failed. Try again later" for non en-US locale.

Works fine on en-US

Comment 1

[Stuart Colville [:muffinresearch]]

This is because the only info passed into the callback is the error message itself and this is localised.

Currently we are keying off the english string to determine cancelled versus some other reason and when localised this means there's no match so we fall through to the default which is 'Payment failed. Try again later'.

What we really need is some kind of universal identifier in the error object so we can implement our own messages. Otherwise we could just show the message that we're given and that would need to be changed to be the text we require.

See here for more info - https://wiki.mozilla.org/WebAPI/WebPaymentProvider#Completion

A quick fix would be to have 'Payment cancelled' always be the default however this would mean that's shown regardless of reason.

Comment 2

[Stuart Colville [:muffinresearch]]

To see what's available I've used this code: http://www.pastebin.mozilla.org/2491348 which pprints errorMsg here: https://github.com/mozilla/fireplace/blob/master/hearth/media/js/payments/payments.js#L75

And this is the output: http://www.pastebin.mozilla.org/2491553

Comment 3

[Kumar McMillan [:kumar]]

Fernando, when a user presses cancel in the Trusted UI, we receive an error object with this property:

this.error.name = 'Dialog closed by the user'

This is done in gaia: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/trusted_ui.js#L185

As you can see, it is localized so the value will change based on locale and thus it is not useful as a key. We need a reliable key value so we can handle the cancel scenario gracefully. Do you know of a reliable key in the error object? Should we raise a gaia bug for this? If so, any suggestions on what change to propose for gaia?

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

I'm ok with adding an error-like string (like DIALOG_CLOSED) instead of a localized message in that case.

I filed bug 880626.

Jed, is that also ok for the identity flow?

Comment 5

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Thanks for checking, Fernando.  We don't actually use the errorMsg value in the identity flow, so this change would be ok for us.

But a question - why not just key off the 'type' field of the event, which is 'cancel'?  Does that not solve this problem already?

Comment 6

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Yes, looking at this with Austin now, we see that the fireplace code is looking for the 'name' property on the error:

https://github.com/mozilla/fireplace/blob/master/hearth/media/js/payments/payments.js#L78

But there is no name property, so the switch statement seems always to be falling through to the default case, which is the error message mentioned in the description above.

Also note that the fireplace code is matching the value 'cancelled', which should be 'cancel'.

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Jed, I am not sure that I am properly understanding you.

(In reply to Jed Parsons [:jparsons] from comment #5)
 > But a question - why not just key off the 'type' field of the event, which
> is 'cancel'?  Does that not solve this problem already?

The 'type' field is not exposed to content, the 'errorMsg' is. Take a look at the message sent at [1] which is handled at [2] where we fire the 'DOMRequest.onerror' with 'errorMsg' as 'DOMRequest.error.name' that is received and checked at [3].

At that point the value of 'DOMRequest.error.name' can be:

1. The string passed by the payment provider when completing the payment flow via 'paymentFailed(errorMsg)' [4]

or

2. The string (or now the key) set by the UI if the dialog is closed from outside of the payment provider's scope, in this case, by user interaction. At [5].

> 
> Also note that the fireplace code is matching the value 'cancelled', which
> should be 'cancel'.

'cancelled' is set by the payment provider at [6].

[1] https://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/payment.js#98
[2] https://mxr.mozilla.org/mozilla-central/source/dom/payment/Payment.js#101
[3] https://github.com/mozilla/fireplace/blob/master/hearth/media/js/payments/payments.js#L78
[4] https://wiki.mozilla.org/WebAPI/WebPaymentProvider#Completion
[5] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/trusted_ui.js#L185
[6] https://github.com/mozilla/webpay/blob/master/media/js/pay/cancel.js#L23

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

bug 880626 just landed. Note that it won't be in v1.1 though (unless you nominate it for leo+, but I don't think this would be a blocker).

Comment 9

[Kumar McMillan [:kumar]]

Thanks Fernando. Stuart, we can fall back to the current en string (for 1.1) and detect the new DIALOG_CLOSED_BY_USER string for the future.

Comment 10

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

Fernando, thanks for the explanation.  My head was more in chromeland than contentland. I understand that using the DOMRequest.error you only get one choice for the message, and it's got to be a string.  This seems good to me.  Again, thanks for the notice of the change; if we need to deal with that string, we'll know what to do.

Comment 11

[Stuart Colville [:muffinresearch]]

(In reply to Kumar McMillan [:kumar] from comment #9)
> Thanks Fernando. Stuart, we can fall back to the current en string (for 1.1)
> and detect the new DIALOG_CLOSED_BY_USER string for the future.

r? https://github.com/mozilla/fireplace/pull/169

Let me know if that's what you had in mind for falling back to the current en string. Clearly that will only work for en locales. The only alternative I can see would be to have "Payment cancelled" be the default message shown.

Comment 12

[Stuart Colville [:muffinresearch]]

https://github.com/mozilla/fireplace/commit/8f3d8dbaa7a3843ce42266e1679734fa4bf965d1

This won't actually resolve the bug but as soon as truste-ui.js sends a non-localised string we will do the right thing for locales other than en. That's all we can do for now.

Comment 13

[Kumar McMillan [:kumar]]

For QA, please note that you'll have to flash a b2g-18 trunk build to test this (or we can qa- it if that's not feasible)