1008125 - [tarako]Aircel ,Firefox shows blank space instead of operator name in manual plmn in roaming area

Status: RESOLVED FIXED

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

Description

[Xiaohong.Fan]

a) Go to Settings/Cellular&Data/SIM 1 or SIM 2/Network operator,turn off the Automatic selection to search the network
b) Firefox shows blank space instead of operator name in manual plmn in roaming area .

Comment 1

[Xiaohong.Fan]

If there is no name about the operator from the network we've gotten from gecko,there should not show the name in the itemlist.

Comment 2

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

I think if we're not connected to a network operator, then we won't show any information.  I can't reproduce the issue.

Was a network selected after turning off the Automatic selection?
If not, did you try making a call following?
If you are able to make a call following, could you turn ril debugging on ( https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#RIL_Debugging ) and capture a : adb logcat -b radio
and attach it to this bug please?

Comment 3

[Xiaohong.Fan]

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #2)
> I think if we're not connected to a network operator, then we won't show any
> information.  I can't reproduce the issue.
> 
> Was a network selected after turning off the Automatic selection?
> If not, did you try making a call following?
> If you are able to make a call following, could you turn ril debugging on (
> https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#RIL_Debugging ) and capture
> a : adb logcat -b radio
> and attach it to this bug please?

The key issue is that the information about the operators we've gotten from internet sometimes has no name but has a code,such as 40522,40566 below,for this issue,we should filter out these information that hasn't name，please see bug http://bugzilla.spreadtrum.com/bugzilla/show_bug.cgi?id=310119

05-07 14:19:22.564    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:19:22.634    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:20:58.514    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:21:03.604    86   322 I Gecko   : RIL Worker: [1] Operator: DOLPHIN,DOLPHIN,40468
05-07 14:21:56.464    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:23:15.504    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:24:42.534    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:26:53.494    86   322 I Gecko   : RIL Worker: [1] Operator: DOLPHIN,DOLPHIN,40468
05-07 14:26:53.614    86   322 I Gecko   : RIL Worker: [1] Operator: DOLPHIN,DOLPHIN,40468
05-07 14:26:55.684    86   322 I Gecko   : RIL Worker: [1] Operator: ,,40522
05-07 14:26:55.754    86   322 I Gecko   : RIL Worker: [1] Operator: ,,40522
05-07 14:27:22.064    86   322 I Gecko   : RIL Worker: [1] Operator: ,,40522
05-07 14:27:23.754    86   322 I Gecko   : RIL Worker: [1] Operator: ,,40566
05-07 14:27:25.614    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:27:26.324    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:27:26.384    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:27:36.694    86   322 I Gecko   : RIL Worker: [1] Operator: ,,40456
05-07 14:27:38.804    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:27:39.504    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:27:39.574    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:27:58.234    86   322 I Gecko   : RIL Worker: [1] Operator: ,,40497
05-07 14:28:12.854    86   322 I Gecko   : RIL Worker: [1] Operator: ,,405046
05-07 14:28:29.054    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:28:29.144    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:28:50.494    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:28:50.564    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:30:18.374    86   322 I Gecko   : RIL Worker: [1] Operator: ,,
05-07 14:30:18.454    86   322 I Gecko   : RIL Worker: [1] Operator: ,,
05-07 14:30:20.864    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:30:20.954    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:30:23.404    86   322 I Gecko   : RIL Worker: [0] Operator: ,,
05-07 14:30:23.704    86   322 I Gecko   : RIL Worker: [0] Operator: ,,
05-07 14:30:25.274    86   322 I Gecko   : RIL Worker: [0] Operator: AIRCEL,AIRCEL,405800
05-07 14:31:03.529    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:31:03.599    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:32:13.639    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:32:13.759    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:32:18.119    86   322 I Gecko   : RIL Worker: [0] Operator: IDEA,IDEA,40456
05-07 14:32:14.672    86   322 I Gecko   : RIL Worker: [0] Operator: IDEA,IDEA,40456
05-07 14:32:37.282    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:32:37.492    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:33:29.182    86   322 I Gecko   : RIL Worker: [0] Operator: IDEA,IDEA,40456
05-07 14:33:41.632    86   322 I Gecko   : RIL Worker: [0] Operator: IDEA,IDEA,40456
05-07 14:33:48.912    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454
05-07 14:33:48.972    86   322 I Gecko   : RIL Worker: [1] Operator: CellOne,CellOne,40454

Comment 4

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

ok.  Thanks.

Comment 5

[Xiaohong.Fan]

Attachment: To avoid confusing users,we should remove the detected operators without names

To avoid confusing users,we should remove the detected operators without names.

Comment 6

[Steven Yang [:styang]]

Hi Omega, could you also review the concept from UX perspectives? thanks

Comment 7

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8421453 [details] [diff] [review]
To avoid confusing users,we should remove the detected operators without names

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

The patch looks good except for a few nits. I'll grant the patch after Omega approves the design.

::: apps/settings/js/carrier.js
@@ +607,4 @@
>            var networks = scanRequest.result;
>            for (var i = 0; i < networks.length; i++) {
>              var network = networks[i];
> +            //To avoid confusing users,we should remove the detected operators without names

nit: Please leave a space between '/' and 'T'.

@@ +607,5 @@
>            var networks = scanRequest.result;
>            for (var i = 0; i < networks.length; i++) {
>              var network = networks[i];
> +            //To avoid confusing users,we should remove the detected operators without names
> +            if (!(network.shortName || network.longName)) {              

Please remove the trailing spaces.

@@ +608,5 @@
>            for (var i = 0; i < networks.length; i++) {
>              var network = networks[i];
> +            //To avoid confusing users,we should remove the detected operators without names
> +            if (!(network.shortName || network.longName)) {              
> +                  continue;

nit: indention error. Please use an indention with two spaces.

@@ +609,5 @@
>              var network = networks[i];
> +            //To avoid confusing users,we should remove the detected operators without names
> +            if (!(network.shortName || network.longName)) {              
> +                  continue;
> +              }

nit: Please align '}' to 'if'.

Comment 8

[Omega Feng [:Omega] [:馮於懋]]

I think we shouldn't remove the items without Operator names from the list. We should try to get the Operator name from something like apn.json. If it still cannot get a name, just show "Unknown". User can still connect to an Unknown if its status is Available.

Comment 9

[James Zhang (Spreadtrum)]

Xiaohong, please update your patch.

Comment 10

[Xiaohong.Fan]

Attachment: 1008125.patch

update patch

Comment 11

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8421453 [details] [diff] [review]
To avoid confusing users,we should remove the detected operators without names

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

Per comment 8, there is still a chance to get the carrier name from apn.json if mcc and mnc are available. And we should display unknown if there is no valid information at all.

Comment 12

[Joe Cheng [:jcheng] (please needinfo)]

can we please have an updated patch? thanks

Comment 13

[Xiaohong.Fan]

apn.json is used for connections,and there are some difference between the operator names in it and the names we get from networks.

Comment 14

[Evelyn Hung]

(In reply to Xiaohong.Fan from comment #13)
> apn.json is used for connections,and there are some difference between the
> operator names in it and the names we get from networks.

So we should display 'unknown' if we are not sure.

Comment 15

[helloarvin]

xiaohong, 

  could you please update the patch referred to Evelyn's advice on comment 14?

  Thanks.

Comment 16

[Xiaohong.Fan]

Attachment: 1008125-update.patch

I have updated the patch according to Evelyn's advice on comment 14,please help to review,if have any problems,contact me freely.

Comment 17

[Xiaohong.Fan]

In localize files,translation about the name 'unknow' is only in the file of locales/settings.ar.properties,
I think we should file a new bug about this problem.

Comment 18

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8427489 [details] [diff] [review]
1008125-update.patch

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

::: apps/settings/js/carrier.js
@@ +415,5 @@
>  
>        // name
>        var name = document.createElement('a');
> +
> +      //if the operator has no name,unknown is shown

nit: please add a space after the comma.

@@ +419,5 @@
> +      //if the operator has no name,unknown is shown
> +      if (network.shortName || network.longName){
> +        name.textContent = network.shortName || network.longName;
> +      } else {
> +        name.textContent = navigator.mozL10n.get('unknown');

Please use `localize(name, <l10nId>);` to assign the l10n id at the same time.

And because the strings are already freeze, please use `unknown-phoneNumber` as the l10n id (it's value is also "unknown").

Comment 19

[Xiaohong.Fan]

Attachment: update1008125.patch

I have updated the patch according to the advice in comment 18 ,
please help review,thanks.

Comment 20

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8428200 [details] [diff] [review]
update1008125.patch

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

The code looks good to me except for a few nits. Please address the nits and create a PR on github to ensure it won't break any tests. Thanks!

::: apps/settings/js/carrier.js
@@ +415,5 @@
>  
>        // name
>        var name = document.createElement('a');
> +
> +      // if the operator has no name,unknown is shown

nit: please leave a space after the comma.

@@ +419,5 @@
> +      // if the operator has no name,unknown is shown
> +      if (network.shortName || network.longName){
> +        name.textContent = network.shortName || network.longName;
> +      } else {
> +        localize(name,'unknown-phoneNumber');

nit: please leave a space between "," and "'".

Comment 21

[Xiaohong.Fan]

Attachment: update.patch

I have updated the patch according to the advice in comment 20 ,
please help review,thanks.

Comment 22

[Arthur Chen [:arthurcc][inactive after 6/30]]

Comment on attachment 8428478 [details] [diff] [review]
update.patch

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

r=me, thanks. I'll create a PR on the basis of this patch.

Comment 23

[Arthur Chen [:arthurcc][inactive after 6/30]]

Attachment: link to https://github.com/mozilla-b2g/gaia/pull/19625

PR of attachment 8428478 [details] [diff] [review].

Comment 24

[Arthur Chen [:arthurcc][inactive after 6/30]]

v1.3t: d355042bd2a715162f8d568345d9e0ed7804c3ae

Comment 25

[Xiaohong.Fan]

Indian colleague suggests to show mcc/mnc instead of unknown in list item 
because it can show more useful information to users in sprd bug 311230 ,the same problem .

Comment 26

[Arthur Chen [:arthurcc][inactive after 6/30]]

As this bug is closed, please create another one if you think this is still an issue.