1027430 - Add EF_SPN to Navigator.mozMobileConnections and ensure it's accessible from a non-certified app

Status: VERIFIED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Wil Clouser [:clouserw]]

Please add EF_SPN access to Navigator.mozMobileConnections.  This needs to be accessible from a non-certified app.  There is some precedent for this in bug 866272 - this value should be revealed the same way.  

This should land in 2.1 - if you need help testing it the Marketplace team can help but please budget that time into when you land it.  Please let us know any questions.  Thanks.

Comment 1

[Ken Chang[:kenkai]]

Jessica, can you please take this? Thanks.

Comment 2

[Jessica Jong [:jessica] (inactive)]

(In reply to Ken Chang[:ken] from comment #1)
> Jessica, can you please take this? Thanks.

Sure!

Comment 3

[Jessica Jong [:jessica] (inactive)]

Wil, on our internal discussion, we would like to append the EF_SPN info to mozMobileConnections[i].lastKnownHomeNetwork, since this field is already used to expose SIM's mcc-mnc and can be accessed by privileged applications with 'mobilenetwork' permission.

However, apps using this field will need to deal with different versions of ril/gecko. E.g., with an old version of ril, mozMobileConnections[i].lastKnownHomeNetwork returns '466-92', but with the new ril, mozMobileConnections[i].lastKnownHomeNetwork may return '466-92.Chunghwa'.

Please let me know if this is okay for you, any comments and suggestions are welcomed. Thanks.

Comment 4

[Jessica Jong [:jessica] (inactive)]

I found that Fireplace uses |mozMobileConnections[i].lastKnownHomeNetwork.split('-')| to get network information. If we append the EF_SPN like this: <mcc>-<mnc>-<spn>, then it should be compatible with older versions, right?

[1] https://github.com/mozilla/fireplace/blob/master/yulelog/main.js#L50

Comment 5

[Chuck Harmston [:chuck]]

We do use lastKnownHomeNetwork in one other place:

https://github.com/mozilla/fireplace/blob/master/hearth/media/js/mobilenetwork.js#L338

But we use it in exactly the same way, so your proposal in comment 4 should work great. Thanks, Jessica!

Comment 6

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 1: implementation, v1.

Hi Hsinyi, we are appending spn info to .lastKnownHomeNetwork. May I have your review?

Comment 7

[Jessica Jong [:jessica] (inactive)]

Attachment: Part 2: changes to test case, v1.

Comment 8

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

Comment on attachment 8446931 [details] [diff] [review]
Part 1: implementation, v1.

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

I like this!

Comment 9

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

Comment on attachment 8446932 [details] [diff] [review]
Part 2: changes to test case, v1.

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

Thank you!

Comment 10

[Jessica Jong [:jessica] (inactive)]

Thank you, Hsinyi.

https://tbpl.mozilla.org/?tree=Try&rev=7d700d56a74f  try green!

Comment 11

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

   https://hg.mozilla.org/integration/b2g-inbound/rev/e1fcb6ea058b
   https://hg.mozilla.org/integration/b2g-inbound/rev/33c95e196040

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e1fcb6ea058b
https://hg.mozilla.org/mozilla-central/rev/33c95e196040

Comment 14

[Sandip Kamat]

updating to target 2.1

Comment 15

[Sandip Kamat]

Can QA confirm if this is successfully tested with marketplace?

Comment 16

[Wil Clouser [:clouserw]]

(In reply to Sandip Kamat from comment #15)
> Can QA confirm if this is successfully tested with marketplace?

That bug hasn't been filed yet.  We're waiting on a needinfo in bug 1035285 before we can file it (to know the values of the SPN to look for)

Comment 17

[Wil Clouser [:clouserw]]

(In reply to Wil Clouser [:clouserw] from comment #16)
> (In reply to Sandip Kamat from comment #15)
> > Can QA confirm if this is successfully tested with marketplace?
> 
> That bug hasn't been filed yet.  We're waiting on a needinfo in bug 1035285
> before we can file it (to know the values of the SPN to look for)

Bug is filed to implement: bug 1049908

Comment 18

[Eric Chang [:ericcc] [:echang]]

1. Will modify to a non-certified app  https://github.com/echangmoz/ConnectionCheck
2. RIL - Cell& Data cases https://moztrap.mozilla.org/manage/cases/?filter-suite=158

Comment 19

[Tony Chung [:tchung]]

(In reply to Eric Chang [:ericcc] [:echang] from comment #18)
> 1. Will modify to a non-certified app 
> https://github.com/echangmoz/ConnectionCheck
> 2. RIL - Cell& Data cases
> https://moztrap.mozilla.org/manage/cases/?filter-suite=158

I'm told by Caitlin from Marketplace team that this bug cannot be verified until the marketplace servers are enabled later this week.   ni? Will Clouser to add the details of location of marketplace test links, as well as keeping the tester informed when its test ready.

Comment 20

[Wil Clouser [:clouserw]]

We already tested it in bug 1049908.   We were expecting a hexadecimal identifier but this patch is giving us a service name.  Bug 1049908 is assigned to David Almstrom to clarify which we should be looking for and what we expect the values to be for the regions we can test in.

Comment 21

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

(In reply to Wil Clouser [:clouserw] from comment #20)
> We already tested it in bug 1049908.   We were expecting a hexadecimal
> identifier but this patch is giving us a service name. 

The string proposal was agreed on comment 5.

> Bug 1049908 is
> assigned to David Almstrom to clarify which we should be looking for and
> what we expect the values to be for the regions we can test in.

Looks like it's a format issue? I mean, various operator might offer different format of data, which could be a string or a hex number. API should anyway choose one format to expose. From the API's point of view, exposing a readable string sounds reasonable to me, and that's also the expectation I see from other apps. Could we handle the format transformation on marketplace app side?

Comment 22

[Eric Chang [:ericcc] [:echang]]

Thanks for Tony's info.

1. Privileged test app to access mozMobileconnection.lastKnownNetwork and .lastKnownHomeNetwork, it works
https://github.com/echangmoz/ConnectionCheck/tree/privileged-sample
2. RIL - Cell& Data cases
https://moztrap.mozilla.org/manage/cases/?filter-suite=158
3. Test using Marketplace


(In reply to Tony Chung [:tchung] from comment #19)
> (In reply to Eric Chang [:ericcc] [:echang] from comment #18)
> > 1. Will modify to a non-certified app 
> > https://github.com/echangmoz/ConnectionCheck
> > 2. RIL - Cell& Data cases
> > https://moztrap.mozilla.org/manage/cases/?filter-suite=158
> 
> I'm told by Caitlin from Marketplace team that this bug cannot be verified
> until the marketplace servers are enabled later this week.   ni? Will
> Clouser to add the details of location of marketplace test links, as well as
> keeping the tester informed when its test ready.

Comment 23

[Eric Chang [:ericcc] [:echang]]

Verified landed. 
Gaia      fbb297c39aab5f17b179533d2a9a6c5166b2c197
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/fb5e796da813
BuildID   20140902160204
Version   34.0a2
ro.build.version.incremental=110
ro.build.date=Fri Jun 27 15:57:58 CST 2014
B1TC00011230