Closed Bug 879579 Opened 10 years ago Closed 10 years ago
.." message shown on canceling purchase in a non en-US locale
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
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.
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
Status: NEW → ASSIGNED
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?
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?
Flags: needinfo?(ferjmoreno) → needinfo?(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?
Flags: needinfo?(jparsons) → needinfo?(ferjmoreno)
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'.
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  which is handled at  where we fire the 'DOMRequest.onerror' with 'errorMsg' as 'DOMRequest.error.name' that is received and checked at . 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)'  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 . > > Also note that the fireplace code is matching the value 'cancelled', which > should be 'cancel'. 'cancelled' is set by the payment provider at .  https://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/payment.js#98  https://mxr.mozilla.org/mozilla-central/source/dom/payment/Payment.js#101  https://github.com/mozilla/fireplace/blob/master/hearth/media/js/payments/payments.js#L78  https://wiki.mozilla.org/WebAPI/WebPaymentProvider#Completion  https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/trusted_ui.js#L185  https://github.com/mozilla/webpay/blob/master/media/js/pay/cancel.js#L23
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).
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.
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.
(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.
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.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
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)
You need to log in before you can comment on or make changes to this bug.