Closed Bug 936471 Opened 7 years ago Closed 7 years ago

Payments fail on Inari 1.3 with TypeError: iccProvider.iccInfo is undefined

Categories

(Core :: DOM: Device Interfaces, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28
blocking-b2g 1.3+
Tracking Status
firefox28 --- fixed

People

(Reporter: Bebe, Assigned: allstars.chh)

References

Details

(Keywords: regression)

Attachments

(1 file, 1 obsolete file)

Flash inari with mozilla-central (1.3) build
Gaia:     42bbe26a72e030faf07a6fc297f61a3a8ccda25b
Gecko:    http://hg.mozilla.org/mozilla-central/rev/70de5e24d79b
BuildID   20131107040200
Version   28.0a1

Login and try to buy a payed app:

The Bango screen opens and it is blank:
http://imageshack.com/a/img194/8820/owtj.png

In the logcat we can see:
https://pastebin.mozilla.org/3455005

E/GeckoConsole( 1279): [JavaScript Error: "TypeError: iccProvider.iccInfo is undefined" {file: "chrome://browser/content/payment.js" line: 195}]
E/GeckoConsole( 1279): [JavaScript Warning: "Unknown property 'box-sizing'.  Declaration dropped." {file: "https://marketplace.allizom.org/mozpay/?req=eyJhbGciOiAiSFMyNTYiLCAidHlwIjogIkpXVCJ9.eyJh
Component: Payments/Refunds → DOM: Device Interfaces
Product: Marketplace → Core
Version: Avenir → Trunk
ferjm, any ideas on what might have changed here?
(In reply to Kumar McMillan [:kumar] from comment #1)
> ferjm, any ideas on what might have changed here?

I'm pretty sure this is likely a regression from the DSDS features landing. Let me hunt around for what caused this.
blocking-b2g: --- → 1.3?
Keywords: regression
Might have been caused by bug 930394.
Blocks: 930394
Hi,
Can you provide more detail STR ?
And what's Bango?

Also why did Jason say this is caused by Bug 930394?

Or what you met is actually Bug 933524?

Thanks
Flags: needinfo?(florin.strugariu)
Flags: needinfo?(jsmith)
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #4)
> Hi,
> Can you provide more detail STR ?

The STR involved here is buying a paid app using the payments API with marketplace as the payment provider.

Bebe - Can you include the Gaia UI Test that triggers this workflow?

> And what's Bango?

That's the payment processor marketplace uses to process payments when buying an app.

> 
> Also why did Jason say this is caused by Bug 930394?

The regression involved here has to be due to a change in the iccProvider's API likely that we didn't update in the payments API. Originally thought the regression range pointed to that bug, but I think I need to double check with Bebe to understand the regression range.

Bebe - Can you clarify regression range here?

> 
> Or what you met is actually Bug 933524?

Don't know. We need a regression range to confirm this.
No longer blocks: 930394
Flags: needinfo?(jsmith)
Bebe - Can you include the Gaia UI Test that triggers this workflow?
https://github.com/mozilla/marketplace-tests-gaia/blob/master/marketplacetests/tests/test_marketplace_purchase_app_on_dev_with_mocked_bango.py
Flags: needinfo?(florin.strugariu)
I tested this on unagi 1.3 too and got the same issue.

I will try to get a regression range for this but what I can say now 1.2 has the same issue so it might be a old issue
Thanks for the info, Bebe and Jason.

I'll try to run the script in Comment 6 to see if I can reproduce it.

Thanks
Sorry, I am not sure how to run the test correctly,
paste the error here. http://pastebin.com/G0Mk0TZW

Can you help to check what went wrong here?
Zac - Can you help give input to Yoshi in comment 9 on what's not working?
Flags: needinfo?(zcampbell)
Hey Yoshi, it looks like you have not installed marionette client.
"pip install marionette_client" will do it.

btw if you need help more local to you then Askeing and Walter can help you out.
Flags: needinfo?(zcampbell)
Thanks, Jason and Zac.
I'll try again.
It's interesting that market-place test branched out
https://github.com/mozilla/marketplace-tests-gaia

It seems like old gaia-ui-tests structure. I don't know what was missed.
In that case, I force it to install most up-to-date gaia-ui-tests and switch to install marketplace-tests-gaia again, and it works now.

However, does anyone know about this?
(In reply to Florin Strugariu [:Bebe] from comment #0)
> Flash inari with mozilla-central (1.3) build
> Gaia:     42bbe26a72e030faf07a6fc297f61a3a8ccda25b
> Gecko:    http://hg.mozilla.org/mozilla-central/rev/70de5e24d79b
> BuildID   20131107040200
> Version   28.0a1
> 
> E/GeckoConsole( 1279): [JavaScript Error: "TypeError: iccProvider.iccInfo is
> undefined" {file: "chrome://browser/content/payment.js" line: 195}]

I think it's a regression from Bug 926343, which changes the interface for nsIIccProvider.
Attached patch Patch (obsolete) — Splinter Review
Assignee: nobody → allstars.chh
Status: NEW → ASSIGNED
Attachment #832729 - Flags: review?(fabrice)
Comment on attachment 832729 [details] [diff] [review]
Patch

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

I think the initial ideas was to return an array with all the sim information available, not just the first one when several sims are available.
Fernando, is that correct?
Attachment #832729 - Flags: review?(fabrice) → review-
Flags: needinfo?(ferjmoreno)
(In reply to Fabrice Desré [:fabrice] from comment #16)
> Comment on attachment 832729 [details] [diff] [review]
> Patch
> 
> Review of attachment 832729 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I think the initial ideas was to return an array with all the sim
> information available, not just the first one when several sims are
> available.
> Fernando, is that correct?

We should return the information of the active SIM. Returning an array was not correct as it was a wrong assumption done before the DSDS design.

Yoshi, you filed bug 938993 which seems to be the same as this one. Can I close that one as a dup?
Flags: needinfo?(ferjmoreno)
(In reply to Kumar McMillan [:kumar] from comment #1)
> ferjm, any ideas on what might have changed here?

Use needinfo instead of CC please :)
(In reply to Fernando Jiménez Moreno [:ferjm] (use needinfo instead of CC, please) from comment #17)
> > 
> > I think the initial ideas was to return an array with all the sim
> > information available, not just the first one when several sims are
> > available.
> > Fernando, is that correct?
> 
> We should return the information of the active SIM. Returning an array was
> not correct as it was a wrong assumption done before the DSDS design.
> 
> Yoshi, you filed bug 938993 which seems to be the same as this one. Can I
> close that one as a dup?

Hi Fernando
Currently DSDS is still ongoing, and 'active SIM' feature is, too, Bug 936325.
Also DSDS is not stable yet.
So I file Bug 938993 to track the Multi-SIM for payments, and fix that until DSDS feature in more stable, also we can test that.
(Right now RIL team is still struggling on the device porting.)

I'd suggest we could fix the regression first, so RIL won't block Marketplace team.
Even if I fix the multi-SIM case here, I worry we might not have a device or some way to test.

What do you think??
Flags: needinfo?(ferjmoreno)
Ok, fixing the regression first works for me. Thanks Yoshi!
Flags: needinfo?(ferjmoreno)
Thanks, Fernando.

Hi Fabrice,
Do you think we could move the payments for multi-SIM to Bug 938993? And fix the regression first?

Or if you still think we should do something for multi-SIM in this bug,
I will discuss this with Fernando again, or let Fernando to take this bug to continue,
since I've already found out the regression, but not familiar with payments for multi-SIM, (we already have a lot of discussions for Multi-SIM in RIL team now). 

Thanks
(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #21)
> Thanks, Fernando.
> 
> Hi Fabrice,
> Do you think we could move the payments for multi-SIM to Bug 938993? And fix
> the regression first?

From the QA perspective - I'd prefer that we would get the regression fixed first, so that we could our payments automation back online. I'd take care of multi-SIM support in a separate bug.
(In reply to Jason Smith [:jsmith] from comment #22)
> (In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #21)
> > Thanks, Fernando.
> > 
> > Hi Fabrice,
> > Do you think we could move the payments for multi-SIM to Bug 938993? And fix
> > the regression first?
> 
> From the QA perspective - I'd prefer that we would get the regression fixed
> first, so that we could our payments automation back online. I'd take care
> of multi-SIM support in a separate bug.

That's fine for me too.
Comment on attachment 832729 [details] [diff] [review]
Patch

Request for review again for Fabrice and Fernando both agree to move the multi-SIM to another bug, and fix the regression in this bug.
Attachment #832729 - Flags: review- → review?(fabrice)
Comment on attachment 832729 [details] [diff] [review]
Patch

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

Thanks Yoshi!
Attachment #832729 - Flags: review?(fabrice) → review+
Attached patch Patch.Splinter Review
Update patch title and add r=fabrice.

Thanks
Attachment #832729 - Attachment is obsolete: true
Attachment #8334313 - Flags: review+
try https://tbpl.mozilla.org/?tree=Try&rev=ad0dd22941e4

When it's all green I'll push the patch.

Thanks
https://hg.mozilla.org/mozilla-central/rev/8eebd3c21b74
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Setting to blocking cause payments need to work.
blocking-b2g: 1.3? → 1.3+
You need to log in before you can comment on or make changes to this bug.