942355 - Add dual SIM support to the marketplace (DSDS)

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect, P1)

Description

[Wil Clouser [:clouserw]]

The APIs to get the SIM info and MCC/MNC are changing to support multiple SIM cards simultaneously in the same device.  This is actually pretty cool and pretty straight forward from our point of view - for the time being in the Marketplace we're planning on using the SIM marked for 'data' for everything (op shelf & payments).  https://wiki.mozilla.org/FirefoxOS/DSDS is an overview of the feature and has links to the API changes.  This bug is only for operator shelf, not payments (andym is taking care of that, but feel free to sync up with him).

Krupa notes that we shouldn't break old devices so detecting whether the API is returning an array or not would be good.

Let me know if there are questions, we're trying to get this shipped on device for 1.3 so we'll need to make a new package and get it onto the device.  Thanks!

Comment 1

[Wil Clouser [:clouserw]]

Hsinyi collected all the docs on that wiki page and she can answer any questions about the API.  /CCing her

Comment 2

[Christopher Van Wiemeersch [:cvan]]

Per https://wiki.mozilla.org/FirefoxOS/DSDS#WebAPI the info is surfaced in `navigator.mozMobileConnections` (which I assume is a typo; we've been using `navigator.mozMobileConnection`), but I'd like to test this with an actual device.

https://developer.mozilla.org/en-US/docs/Web/API/Navigator.mozMobileConnection

I'm fine with just serializing `navigator.mozMobileConnection` and then passing it to Fireplace and we can do the parsing there, but I want to make sure I'm looking at the correct thing.

Comment 3

[Wil Clouser [:clouserw]]

+hsinyi to answer comment 2. thanks

Comment 4

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Wil Clouser [:clouserw] from comment #3)
> +hsinyi to answer comment 2. thanks

(In reply to Christopher Van Wiemeersch [:cvan] from comment #2)
> Per https://wiki.mozilla.org/FirefoxOS/DSDS#WebAPI the info is surfaced in
> `navigator.mozMobileConnections` (which I assume is a typo; we've been using
> `navigator.mozMobileConnection`), but I'd like to test this with an actual
> device.
> 

Nop, it's not a typo. We used mozMobileConnection before. 
The naming changes for multisim support. 
See http://dxr.mozilla.org/mozilla-central/source/dom/webidl/Navigator.webidl?from=Navigator.webidl#259

> https://developer.mozilla.org/en-US/docs/Web/API/Navigator.
> mozMobileConnection
> 
> I'm fine with just serializing `navigator.mozMobileConnection` and then
> passing it to Fireplace and we can do the parsing there, but I want to make
> sure I'm looking at the correct thing.

Comment 5

[Kai-Chih Hu [:khu]]

Sorry. Just want to clarify that, this bug is planed to be landed in 1.3 and this should be done before Dec 6th, am I right? Thank you.

Comment 6

[Christopher Van Wiemeersch [:cvan]]

(In reply to Kevin Hu [:khu] from comment #5)
> Sorry. Just want to clarify that, this bug is planed to be landed in 1.3 and
> this should be done before Dec 6th, am I right? Thank you.

Correct, will be done before December 6.

Comment 7

[Jason Smith [:jsmith]]

(In reply to Kevin Hu [:khu] from comment #5)
> Sorry. Just want to clarify that, this bug is planed to be landed in 1.3 and
> this should be done before Dec 6th, am I right? Thank you.

This is server-side, which should not block the release. Clearing nom.

Comment 8

[Christopher Van Wiemeersch [:cvan]]

Updated in Fireplace:
https://github.com/mozilla/fireplace/commit/bab2d97

Comment 9

[Christopher Van Wiemeersch [:cvan]]

I've looked in Gaia, but all the code seems to be just taking `navigator.mozMobileConnections[0]` as a stopgap solution. I want to confirm that this is the expected format of `navigator.mozMobileConnections`:

    [
        {
            data: {
                network: {
                    mcc: '260',
                    mnc: '02'
                }
            }
        },
        {
            data: {
                network: {
                    mcc: '734',
                    mnc: '04'
                }
            }
        }
    ]

Comment 10

[Christopher Van Wiemeersch [:cvan]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/14285

Comment 11

[Caitlin Galimidi [:c8o]]

Enpei - 
Would you be able to help the Marketplace dev team test this DSDS modification?
We have no dual SIMs. 
Much appreciated!!

Comment 12

[Kai-Chih Hu [:khu]]

Talked with Enpei, and she said she will do the testing for sure.

Comment 13

[Enpei]

(In reply to Caitlin Galimidi from comment #11)
> Enpei - 
> Would you be able to help the Marketplace dev team test this DSDS
> modification?
> We have no dual SIMs. 
> Much appreciated!!

Hi Caitlin and Kevin,

Sorry that I didn't check with my team member first to see I can test it from my side or now. Let me consult with QA team internally first and will get back to you again.

Comment 14

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #7)
> (In reply to Kevin Hu [:khu] from comment #5)
> > Sorry. Just want to clarify that, this bug is planed to be landed in 1.3 and
> > this should be done before Dec 6th, am I right? Thank you.
> 
> This is server-side, which should not block the release. Clearing nom.

Disregard - turns out I wasn't right here. Fixing the blocking flag.

Comment 15

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Christopher Van Wiemeersch [:cvan] from comment #9)
> I've looked in Gaia, but all the code seems to be just taking
> `navigator.mozMobileConnections[0]` as a stopgap solution. 

The statement could be right 2 or more weeks ago. Since more gaia patches for multisim feature were landed last week, the case, all code taking navigator.mozMobileConnections[0], isn't true now.


> I want to confirm
> that this is the expected format of `navigator.mozMobileConnections`:
> 
>     [
>         {
>             data: {
>                 network: {
>                     mcc: '260',
>                     mnc: '02'
>                 }
>             }
>         },
>         {
>             data: {
>                 network: {
>                     mcc: '734',
>                     mnc: '04'
>                 }
>             }
>         }
>     ]

Yes, you got the idea! This is the expected structure for a dual-sim device.

Comment 16

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 8341297 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/14285

Cancelling review as I am not the right person for reviewing this. But I am happy to answer to/provide feedbacks on usage of mobileConnection API.

Comment 17

[Enpei]

(In reply to Caitlin Galimidi from comment #11)
> Enpei - 
> Would you be able to help the Marketplace dev team test this DSDS
> modification?
> We have no dual SIMs. 
> Much appreciated!!

Hi Krupa,

As I sent you another email about Marketplace testing, could you help to cover this test? 

Thank you.

Comment 18

[Wil Clouser [:clouserw]]

(In reply to Enpei from comment #17)
> (In reply to Caitlin Galimidi from comment #11)
> > Enpei - 
> > Would you be able to help the Marketplace dev team test this DSDS
> > modification?
> > We have no dual SIMs. 
> > Much appreciated!!
> 
> Hi Krupa,
> 
> As I sent you another email about Marketplace testing, could you help to
> cover this test? 
> 
> Thank you.

Krupa is out of the office this week, but I'm pretty confident she doesn't have a dual SIM phone to test with.  Who has been testing the platform side up until now?

Comment 19

[Enpei]

(In reply to Wil Clouser [:clouserw] from comment #18)
> Krupa is out of the office this week, but I'm pretty confident she doesn't
> have a dual SIM phone to test with.  Who has been testing the platform side
> up until now?

Hi Wil, US QA team has 2 Fugu devices now, one is in San Francisco. I've emailed Krupa, Naoki and Tony for this part, she should be able to get the device. I'll wait for Krupa's reply when she comes back, if there is anything I can help after discussing with her, I will assist for sure.

Comment 20

[[:fabrice] Fabrice Desré]

Comment on attachment 8341297 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/14285

LGTM but I have no way to test this to confirm.

Comment 21

[Christopher Van Wiemeersch [:cvan]]

(In reply to Fabrice Desré [:fabrice] from comment #20)
> Comment on attachment 8341297 [details] [review]
> https://github.com/mozilla-b2g/gaia/pull/14285
> 
> LGTM but I have no way to test this to confirm.

Thanks, Fabrice! Can we merge in?

Comment 22

[[:fabrice] Fabrice Desré]

Yes, go ahead.

Comment 23

[Christopher Van Wiemeersch [:cvan]]

(In reply to Fabrice Desré [:fabrice] from comment #22)
> Yes, go ahead.

I don't have commit privileges (probably for the best ;))

Comment 24

[[:fabrice] Fabrice Desré]

https://github.com/mozilla-b2g/gaia/commit/07dc0e43107d36f6f9d573cc473d72219ad24bd6

Comment 25

[Victor Carciu]

Can you please add some STRs to this bug or mark it as [qa-] ?

Comment 26

[Christopher Van Wiemeersch [:cvan]]

(In reply to Victor Carciu from comment #25)
> Can you please add some STRs to this bug or mark it as [qa-] ?

Krupa is verifying this

Comment 27

[krupa raj[:krupa]]

Yesterday, I filed https://bugzilla.mozilla.org/show_bug.cgi?id=948222. We are investigating the bug.

Comment 28

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 07dc0e43107d36f6f9d573cc473d72219ad24bd6 to:
v1.3 already had this commit