sync-1 Reporter
	Description • 10 years ago

A.- Overview Description (technical background, concise explanation of the bug):
     
     Received settings on an OTA CP message must not delete previously created settings after installing, but must be activated as default (services must use this new settings with no user interaction, only installation)
 
     ________________________________________________________________________________
     B.- Steps to Reproduce (initial conditions, required resources, step by step instructions to reproduce):
     
     Send throught OTA CP a CP configuration message to the DUT.
 
     ________________________________________________________________________________
     C.- Actual Result (current bad behaviour that is reported as a bug):
     
     The new OTA CP settings don't delete previour settings. This is Ok. 
     But the new  settings are not activated, this is wrong. 
     ________________________________________________________________________________
     D.- Expected Result (correct behaviour wished):
     
     Received setting must always  be activated as default after being installed.

	田旻
	Comment 1 • 10 years ago

Firefox OS v1.3
Mozilla build ID:20140422024003

	José Antonio Olivera Ortega [:jaoo]
	Comment 2 • 10 years ago

This is not a bug but a new feature. If the APN being installed must be selected as the active one for data calls then the ril.data.apnSettings setting must be updated from the WAP PUSH app. Right now the WAP PUSH app stores the APN into the settings database and that's it.

	Vance Chen [:vchen][skype:vance.lucida][vchen@mozilla.com]
	Comment 3 • 10 years ago

Per comment#2, de-nom since this is a new feature request

	田旻
	Comment 4 • 10 years ago

(In reply to Vance Chen [:vchen][vchen@mozilla.com] from comment #3)
> Per comment#2, de-nom since this is a new feature request

But it's really a block for us.

	田旻
	Comment 5 • 10 years ago

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #2)
> This is not a bug but a new feature. If the APN being installed must be
> selected as the active one for data calls then the ril.data.apnSettings
> setting must be updated from the WAP PUSH app. Right now the WAP PUSH app
> stores the APN into the settings database and that's it.

Wap push store apn in "ril.data.cp.apns" and active apn store in "ril.data.apnSettings". What's more, they have a different data structure and a little difficult to merge as a new one.

	José Antonio Olivera Ortega [:jaoo]
	Comment 6 • 10 years ago

(In reply to 田旻 from comment #5)
> (In reply to José Antonio Olivera Ortega [:jaoo] from comment #2)
> Wap push store apn in "ril.data.cp.apns" and active apn store in
> "ril.data.apnSettings". What's more, they have a different data structure
> and a little difficult to merge as a new one.

Leaving some other details apart, the APN being installed could be set as the default (internet browsing) one by storing it in the "ril.data.apnSettings".

	田旻
	Comment 7 • 10 years ago

Please check this patch.

	José Antonio Olivera Ortega [:jaoo]
	Comment 8 • 10 years ago

Comment on attachment 8431470 [details] [diff] [review]
0005-BUG-685386_modified-for-the-oma-cp-apn-should-be-act.patch

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

Thanks for your patch, please see my comments below. Note: I'm not a peer for the WAP APP, anyway, I'll be happy to provide feedback every time you need it.

First of all we don't need any IAC communication between the WAP PUSH app and the Settings one. Every time the user opens any APN panel in the settings app the list gets updated. Please continue reading bellow.

::: apps/settings/js/carrier.js
@@ +179,5 @@
> +      if (message === 'newApnReceive') {
> +        cs_queryApns(cs_updateApnList, 'data');
> +      }
> +    });
> +    //tcl_tianmin add for bug#685386 end

Do not need this IAC stuff. Moreover what about if the APN being installed is MMS type? You are updating the default one (data type which is internet browsing).

::: apps/settings/manifest.webapp
@@ +54,5 @@
> +    },
> +    "cpm-comms": {
> +           "description": "Communication with wap push when a cp message is received",
> +           "rules": {}
> +    }

Do not need this IAC stuff.

::: apps/wappush/js/store.js
@@ +108,5 @@
>            data[CP_APN_KEY] = existingApns;
>          }
>          transaction.set(data);
> +        //tcl_tianmin add for bug#685386 begin
> +        var iccCardIndex = 0;

For the time being we do not support OMA CP stuff for DSDS devices. Please leave the bug number 947198 in the comment please.

@@ +115,5 @@
> +          var result1 = request1.result['ril.data.apnSettings'];
> +          if (!result1 || !Array.isArray(result1)) {
> +            result1 = [[], []];
> +          }
> +          var apns = result1[iccCardIndex];

Once you have the APNs for the ICC card just find the one in use for the APN type being installed and overwrite it with the new APN.

@@ +146,5 @@
> +              }
> +            }
> +          }
> +          result1[iccCardIndex] = apns;
> +          transaction.set({'ril.data.apnSettings': result1});

What about if the data call is up by the time you set the new APN? You must disconnect the data call and connect it again. There are some bits for it in the setting app (carrier.js) file for your reference.

@@ +158,5 @@
> +            });
> +          }, function onConnRejected(reason) {
> +          });
> +        };
> +        //tcl_tianmin add for bug#685386 end

Do not need this IAC stuff.

	Gabriele Svelto [:gsvelto]
	Comment 9 • 10 years ago

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #8)
> Thanks for your patch, please see my comments below. Note: I'm not a peer
> for the WAP APP, anyway, I'll be happy to provide feedback every time you
> need it.

Feel free to review this José, you know this code much better than me. In fact if you'd like to become a peer it would be welcome as you've written almost half of the code in the app yourself.

	田旻
	Comment 10 • 10 years ago

IAC is used to update the UI when user has enter to the setting app and see the settings about the apn setting. The UI can't be update automatically because user has enter into the right UI, thus there need a update my us.

	田旻
	Comment 11 • 10 years ago

(In reply to 田旻 from comment #10)
> IAC is used to update the UI when user has enter to the setting app and see
> the settings about the apn setting. The UI can't be update automatically
> because user has enter into the right UI, thus there need a update my us.

I know the apn setting can ben update every time when you enter into the settings app. 
I mean that When you are in the settings app and see the apn you select, then you install a new apn via a cpm, the new one can't be update automatically.

	José Antonio Olivera Ortega [:jaoo]
	Comment 12 • 10 years ago

(In reply to 田旻 from comment #11)
> (In reply to 田旻 from comment #10)
> I know the apn setting can ben update every time when you enter into the
> settings app.

Even more, the APN lists in the panels get updated every time the user open any panel.

> I mean that When you are in the settings app and see the apn you select,
> then you install a new apn via a cpm, the new one can't be update
> automatically.

If the panel with the APN list where the APN being installed will be added is already open then the user don't see the new APN added until she/he opens the panel again. In that case we use 'visibilitychange' events. IMHO this is a minor issue but if it needs to be solved as well the IAC approach doesn't seem the easiest way.

	José Antonio Olivera Ortega [:jaoo]
	Comment 13 • 10 years ago

(In reply to Gabriele Svelto [:gsvelto] from comment #9)
> (In reply to José Antonio Olivera Ortega [:jaoo] from comment #8)
> > Thanks for your patch, please see my comments below. Note: I'm not a peer
> > for the WAP APP, anyway, I'll be happy to provide feedback every time you
> > need it.
> 
> Feel free to review this José, you know this code much better than me.

Thanks, will do.

> In
> fact if you'd like to become a peer it would be welcome as you've written
> almost half of the code in the app yourself.

Oh, that would be an honor.

	田旻
	Comment 14 • 10 years ago

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #13)
> (In reply to Gabriele Svelto [:gsvelto] from comment #9)
> > (In reply to José Antonio Olivera Ortega [:jaoo] from comment #8)
> > > Thanks for your patch, please see my comments below. Note: I'm not a peer
> > > for the WAP APP, anyway, I'll be happy to provide feedback every time you
> > > need it.
> > 
> > Feel free to review this José, you know this code much better than me.
> 
> Thanks, will do.
> 
> > In
> > fact if you'd like to become a peer it would be welcome as you've written
> > almost half of the code in the app yourself.
> 
> Oh, that would be an honor.

Thanks for you patient and I will try to modified as your suggestion.

	Albert [:albert] Assignee
	Comment 15 • 10 years ago

Filter duplicated APNs coming from message and add automatic selection.

	José Antonio Olivera Ortega [:jaoo]
	Comment 16 • 10 years ago

Comment on attachment 8435300 [details]
Patch

Looking good so far. We need to take care of updating the APN list panel in the setting app and add some test cases. Thanks!

	Albert [:albert] Assignee
	Comment 17 • 10 years ago

Comment on attachment 8435300 [details]
Patch

Removed filter for duplicate APNs.
Added tests.
Fixed settings panel refresh.

	José Antonio Olivera Ortega [:jaoo]
	Comment 18 • 10 years ago

Comment on attachment 8435300 [details]
Patch

LGTM r=me

Tested on a real device with a real OMA CP server. Everything seems to work fine and the test pass.

Thanks for taking care of it.

	Albert [:albert] Assignee
	Comment 19 • 10 years ago

Master: https://github.com/mozilla-b2g/gaia/commit/b3c544e2cf766e7ff161a3e2a3bfa5fed1484332

	田旻
	Comment 20 • 10 years ago

Three problems:
1.Sometimes， there are some apns which have the same type in 'ril.data.apnSettings', if you just replace the comment type , there may exist duplicate ones.
2.The data struction in 'ril.data.apnSettings' and  'ril.data.cp.apns' have a lot of different, especially in mms. Do you check how to transform? Can all of the three types can work well.
3. The order of different apns can't be mixed, you can check it in system initial and carrier.js.

	José Antonio Olivera Ortega [:jaoo]
	Comment 21 • 10 years ago

(In reply to 田旻 from comment #20)
> Three problems:
> 1.Sometimes， there are some apns which have the same type in
> 'ril.data.apnSettings', if you just replace the comment type , there may
> exist duplicate ones.

In theory there is only one APN per type in the 'ril.data.apnSettings' setting. The logic added in this patch replaces the existing one (for a given type) in 'ril.data.apnSettings' for the APN in the provisioning document.

> 2.The data struction in 'ril.data.apnSettings' and  'ril.data.cp.apns' have
> a lot of different, especially in mms.

Yes, the 'ril.data.apnSettings' setting stores the APNs in use by the RIL plumbing. That setting is just an array (actually a couple of arrays, one per ICC card) of APNs. The 'ril.data.cp.apns' settings it's a (JSON) object containing the APN the device received through OMA CP messages. It's an APN database quite similar to the one in the apn.json file. It might contain several APNs of the same type as the carrier might send several different APNs for a type.

> Do you check how to transform? Can
> all of the three types can work well.

I don't understand what you mean, could you elaborate a bit more your question please? Thanks!

> 3. The order of different apns can't be mixed, you can check it in system
> initial and carrier.js.

The order of APN objects in the 'ril.data.apnSettings' setting is not a concern. The RIL plumbing is able to handle them correctly. What do you mean with 'APNs cannot be mixed'?, is that a (new) requirement?

	田旻
	Comment 22 • 10 years ago

> Do you check how to transform? Can
> all of the three types can work well.

I don't understand what you mean, could you elaborate a bit more your question please? Thanks!

You can see the logs I got from the system:
This is what I got from "ril.data.apnSettings" in operator_variant.js.
[[{"carrier":"沃3G连接互联网 (China Unicom)","apn":"3gnet","types":["default"]},{"carrier":"联通彩信 (China Unicom)","apn":"3gwap","mmsc":"http://mmsc.myuni.com.cn","mmsproxy":"10.0.0.172","mmsport":"80","types":["mms"]},{"carrier":"沃3G连接互联网 (China Unicom)","apn":"3gnet","types":["supl"]}],[]]

This is what I got from "ril.data.cp.apns" in store.js. 
Ｎote: I make several parameters as "tianmin" which will be easy to recognize.

{"NAPID":"tianmin1_NAPID","carrier":"tianmin1","apn":"tianmin1","type":["default"],"proxy":"tianmin","TO-NAPID":["tianmin1_NAPID"],"port":"9201"}




> 3. The order of different apns can't be mixed, you can check it in system
> initial and carrier.js.

The order of APN objects in the 'ril.data.apnSettings' setting is not a concern. The RIL plumbing is able to handle them correctly. What do you mean with 'APNs cannot be mixed'?, is that a (new) requirement?


Ｉmean the first type should be "data“ which is also named "default", the second type should be "mms", the third type should be "supl". The order can't be disordered.  You can mix the order and have a try once more, it will be failed.

	田旻
	Comment 23 • 10 years ago

I once got several same type vpn from "ril.data.apnSettings" operator_variant.js during system initialization. But I can't get it this time any more.

	José Antonio Olivera Ortega [:jaoo]
	Comment 24 • 10 years ago

(In reply to 田旻 from comment #22)
> > I don't understand what you mean, could you elaborate a bit more your
> > question please? Thanks!
> 
> You can see the logs I got from the system:
> This is what I got from "ril.data.apnSettings" in operator_variant.js.
> [[{"carrier":"沃3G连接互联网 (China
> Unicom)","apn":"3gnet","types":["default"]},{"carrier":"联通彩信 (China
> Unicom)","apn":"3gwap","mmsc":"http://mmsc.myuni.com.cn","mmsproxy":"10.0.0.
> 172","mmsport":"80","types":["mms"]},{"carrier":"沃3G连接互联网 (China
> Unicom)","apn":"3gnet","types":["supl"]}],[]]

The APNs in use by the RIL plumbing are stored under the 'ril.data.apnSettings' settings, this is correct. The logic in operator_variant.js is in charge of populating that setting on boot.

> This is what I got from "ril.data.cp.apns" in store.js. 
> Ｎote: I make several parameters as "tianmin" which will be easy to recognize.
> 
> {"NAPID":"tianmin1_NAPID","carrier":"tianmin1","apn":"tianmin1","type":
> ["default"],"proxy":"tianmin","TO-NAPID":["tianmin1_NAPID"],"port":"9201"}

Yes, this is an APN you received from the provisioning document.

To sum up, 'ril.data.apnSettings' and 'ril.data.cp.apns' settings are used for different things.

> > > 3. The order of different apns can't be mixed, you can check it in system
> > > initial and carrier.js.
> 
> >The order of APN objects in the 'ril.data.apnSettings' setting is not a
> >concern. The RIL plumbing is able to handle them correctly. What do you mean
> >with 'APNs cannot be mixed'?, is that a (new) requirement?
> 
> 
> Ｉmean the first type should be "data“ which is also named "default", the
> second type should be "mms", the third type should be "supl". The order
> can't be disordered.

Why? I don't agree with that.

	田旻
	Comment 25 • 10 years ago

Then you can disorder the type  of 'ril.data.apnSetting' in store.js and see whether it could be active in settings again. I have tried lots of times once, but it faild.

	José Antonio Olivera Ortega [:jaoo]
	Comment 26 • 10 years ago

(In reply to 田旻 from comment #25)
> Then you can disorder the type  of 'ril.data.apnSetting' in store.js and see
> whether it could be active in settings again. I have tried lots of times
> once, but it faild.

I'm sorry but I don't understand what you mean. It would be great if we discuss about it on IRC or you propose a fix for the issue you see. That would help me to figure out what's wrong. Thanks.

	田旻
	Comment 27 • 10 years ago

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #26)
> (In reply to 田旻 from comment #25)
> > Then you can disorder the type  of 'ril.data.apnSetting' in store.js and see
> > whether it could be active in settings again. I have tried lots of times
> > once, but it faild.
> 
> I'm sorry but I don't understand what you mean. It would be great if we
> discuss about it on IRC or you propose a fix for the issue you see. That
> would help me to figure out what's wrong. Thanks.

You just push the 'mms' type first, 'default' type second, and 'supl' third and set 'ril.data.apnSetting' again. Then you open the settings and check whether the right apn is selected. You will find the problem.

	田旻
	Comment 29 • 10 years ago

(In reply to 田旻 from comment #28)
> Created attachment 8439674 [details]
> oma-cp default setting order relative problems.doc

Dear José Antonio Olivera Ortega,
This file can tell you what the problem is.