859712 - [Gaia][STK] Use new WebAPI to access STK functionality

Status: RESOLVED FIXED

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

Description

[Edgar Chen [:edgar]]

In bug 859220, we are going to separate mozIccManager from mozMobileConnection. The mozIccManager will not be inside mozMobileConnection but in navigator. This bug is filed in Gaia side to make corresponding changes.

Comment 1

[Edgar Chen [:edgar]]

Bug 859220 is already r+ and sr+.
I can help to provide the patch for corresponding Gaia changes.

Comment 2

[Edgar Chen [:edgar]]

Attachment: gaia pull request #9665

Comment 3

[Edgar Chen [:edgar]]

Comment on attachment 747815 [details]
gaia pull request #9665

Hi Fernando:

In this patch, I add a checking for IccManager.
If we can not get IccManger from navigator, try to get it from MobileConnection.
In this way, we can make sure it won't break STK function no matter bug 859220 is landed or not. Otherwise, STK function may fail until both bugs are laned (bug 859220 and this bug).

Could you review this patch for me?

Thanks

Comment 4

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Comment on attachment 747815 [details]
gaia pull request #9665

fix nits please :)

also, how can I test to force one or another api?

Is this only a provisional hack while 859220 is landed?

Is expected to be dropped after that? in that case a comment refering this bug could be cool

Comment 5

[Edgar Chen [:edgar]]

(In reply to Fernando R. Sela [:frsela] from comment #4)
> Comment on attachment 747815 [details]
> gaia pull request #9665
> 
> fix nits please :)
> 
> also, how can I test to force one or another api?
I built two gecko, one applied patches of bug 859220 and another one didn't.

> 
> Is this only a provisional hack while 859220 is landed?
> 
> Is expected to be dropped after that? in that case a comment refering this
> bug could be cool
Yes, we could drop the 'navigator.mozMobileConnection.icc' after bug 859220 is landed.
I will add more comment about this.
Thanks

Comment 6

[Edgar Chen [:edgar]]

@frsela, could you help to merge pull request #9665?
After bug 859220 is landed, I will provide another patch to remove 'navigator.mozMobileConnection.icc' here.
Thanks

Comment 7

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Edgar Chen [:edgar][:echen] from comment #6)
> @frsela, could you help to merge pull request #9665?
> After bug 859220 is landed, I will provide another patch to remove
> 'navigator.mozMobileConnection.icc' here.
> Thanks

sure

Comment 8

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Edgar Chen [:edgar][:echen] from comment #6)
> @frsela, could you help to merge pull request #9665?
> After bug 859220 is landed, I will provide another patch to remove
> 'navigator.mozMobileConnection.icc' here.
> Thanks

Hi Edgar,

One point:

Github informs about automatic merge is not possible... Can you rebase the patch? (thanks in advance)

Second point:

Did you tested this with the patch from 859220 in your device?

Comment 9

[Edgar Chen [:edgar]]

(In reply to Fernando R. Sela [:frsela] from comment #8)
> (In reply to Edgar Chen [:edgar][:echen] from comment #6)
> > @frsela, could you help to merge pull request #9665?
> > After bug 859220 is landed, I will provide another patch to remove
> > 'navigator.mozMobileConnection.icc' here.
> > Thanks
> 
> Hi Edgar,
> 
> One point:
> 
> Github informs about automatic merge is not possible... Can you rebase the
> patch? (thanks in advance)
sure

> 
> Second point:
> 
> Did you tested this with the patch from 859220 in your device?

Yes, I have tested with two gecko, one applied patches of bug 859220 and another one didn't.
Both gecko works well with this patch in my unagi device.

Here are my test steps:
1. Launch Setting
2. Go into STK menu
3. Click some sub-menu

Thanks

Comment 10

[Edgar Chen [:edgar]]

Fernando, I have rebased the patch. Thanks.

Comment 11

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Comment on attachment 747815 [details]
gaia pull request #9665

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed: 
Risk to taking this patch (and alternatives if risky):
String or UUID changes made by this patch:

Comment 12

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

(In reply to Edgar Chen [:edgar][:echen] from comment #10)
> Fernando, I have rebased the patch. Thanks.

cool, now is 'green' ;)

I asked for a? since it's not a blocking issue.

Comment 13

[Edgar Chen [:edgar]]

(In reply to Fernando R. Sela [:frsela] from comment #12)
> (In reply to Edgar Chen [:edgar][:echen] from comment #10)
> > Fernando, I have rebased the patch. Thanks.
> 
> cool, now is 'green' ;)
> 
> I asked for a? since it's not a blocking issue.

Actually bug 859220 is only affect m-c.
We only need this patch for master branch.
This doesn't need to uplift to v1.
Sorry for the unclear.
Thanks

Comment 14

[Alex Keybl [:akeybl]]

Comment on attachment 747815 [details]
gaia pull request #9665

No risk/reward provided

Comment 15

[Fernando R. Sela (no CC, needinfo please) [:frsela]]

Merged in master: a5f660b207f515597a654958c071e71d32fa97aa

Path not to be uplifted !

Edgar, please, create a tracking bug to change this patch as soon as m-c we should start using only the new API.
Thanks in advance.

Comment 16

[Julien Wajsberg [:julienw]]

Hey, could you please fix the linter today ?

  ----- FILE : /home/travis/build/mozilla-b2g/gaia/apps/settings/js/icc.js -----
  Line 69, E:0110: Line too long (92 characters).
  Line 70, E:0110: Line too long (85 characters).

  ----- FILE : /home/travis/build/mozilla-b2g/gaia/apps/system/js/icc_cache.js -----
  Line 33, E:0110: Line too long (90 characters).
  Line 34, E:0110: Line too long (87 characters).

  Found 4 errors, including 0 new errors, in 2 files (703 files OK).

Otherwise I'll sadly have to back out the change.

Thanks !

Comment 17

[Edgar Chen [:edgar]]

Attachment: gaia pull request #9785

Hi Fernando/Julien,

I send this pull request to fix the linter.
Please help to review.

Sorry for any inconvient.

Thanks

Comment 18

[Julien Wajsberg [:julienw]]

Comment on attachment 749778 [details]
gaia pull request #9785

stealing review for such simple patch
r=me

thanks !

Comment 19

[Julien Wajsberg [:julienw]]

master: 40f8307b2ab53689fa563de9ce65483beee610a1