Closed Bug 905889 Opened 7 years ago Closed 6 years ago

[Buri][PLMN]Network name will be shown twice.

Categories

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

defect

Tracking

(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed, b2g-v1.2 affected)

RESOLVED FIXED
blocking-b2g leo+
Tracking Status
b2g18 --- fixed
b2g-v1.1hd --- fixed
b2g-v1.2 --- affected

People

(Reporter: sync-1, Assigned: gweng)

References

Details

(Keywords: regression)

Attachments

(4 files)

AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.184
 Firefox os  v1.1
 Mozilla build ID:20130806071254
 
 
 Created an attachment (id=479528)
 when rgister on 724 02. TIM is right but 724 02 should not be shown.
 
 DEFECT DESCRIPTION:
 Network name will be shown twice.
 
  REPRODUCING PROCEDURES:
 When registers on some network, network name will be shown twice and the network name is not right.
 Attachment Pic.
 
  EXPECTED BEHAVIOUR:
 The netwrok name should be right.
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE:
 4/5
 
  For FT PR, Please list reference mobile's behavior:
Clone from brother
Clone from brother
Attached file QXDM log
Clone from brother
Clone from brother
Attached image register on 724 24
blocking-b2g: --- → leo?
Hi, 
In gaia/shared/js/mobile_operator.js file, why there are carrier and region info for Brazil? Is this a requirement? Can we remove it?
Tim - is this a regression from v1.0.1?  Is there a potential low risk backout of changes to mobile_operator.js that would fix this?
Flags: needinfo?(timdream)
(In reply to lsblakk@mozilla.com [:lsblakk] from comment #9)
> Tim - is this a regression from v1.0.1?  Is there a potential low risk
> backout of changes to mobile_operator.js that would fix this?

Yes, this is a regression. I would say there should be a low risk fix (rather than a protential backout)
Flags: needinfo?(timdream)
By the way, TIM is a mobile carrier in Brazil, not me :)
Component: Gaia::Homescreen → Gaia::System
blocking-b2g: leo? → leo+
Keywords: regression
QA,

Please check if this issue is seen in latest build.
Keywords: qawanted
QA Contact: sarsenyev
This issue doesn't reproduce on the latest Buri build with AT&T network operator. Seems like the issue reproduces with Roaming network, unable to test it with roaming network or Brazil SIM, the network name isn't shown twice

Environmental  Variables:
Build ID: 20130903040204
Gecko: http://hg.mozilla.org/mozilla-central/rev/d54e0cce6c17
Gaia: c6d58088f207829d96e7bb33ddacf990e90752fb
Platform Version: 26.0a1
Keywords: qawanted
Tim,

Can you please have someone look at this? The inconsistency needs to be resolved. Please check on roaming env.
Flags: needinfo?(timdream)
Greg, do you have bandwidth to help out?
Flags: needinfo?(timdream) → needinfo?(gweng)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #15)
> Greg, do you have bandwidth to help out?

Ok, I will start to research this bug after my last reviewing patch (Bug 893560) got granted. It's nearly done.
Flags: needinfo?(gweng)
I can reproduce this bug by adding some debugging code in lockscreen.js (for convenience).
It seems that showing both the connstateLine1 and connstateLine2's textContent at the same time, would result in the similar appearance.
The next step is to find out why will this happen according to the code.
I've fed the displaying function with mock mobile operator as below to let the lockscreen display it as in real connection:

      var mockConnOperator = {
        'operator': 'TIM',
        'carrier': '724',
        'region': '02' 
      }; 

It'll show 2 lines which is similar to the attached appearance except that the bottom borders were missing. So it looks like the original function just show the second line *on purpose*. I can hide it easily according to the code, but I'm not sure which is right (is there any spec?).

Another notable issue is I now test this on simulator, and can install it on device later. However, I don't know how to make the testing environment more close to the reporter (different region, operator, etc), so my patch may be broken.
Flags: needinfo?(timdream)
Greg, thank you for investigating! I think we need to pad 0 to signal digit to mnc here. Could you carry on and add test to that too? Please refer to bug 845629.

diff --git a/shared/js/mobile_operator.js b/shared/js/mobile_operator.js
index 153f8df..8ca1262 100644
--- a/shared/js/mobile_operator.js
+++ b/shared/js/mobile_operator.js
@@ -49,9 +49,9 @@ var MobileOperator = {
 var MobileInfo = {
   brazil: {
     carriers: {
-      '0': 'NEXTEL',
-      '2': 'TIM', '3': 'TIM', '4': 'TIM',
-      '5': 'CLARO', '6': 'VIVO', '7': 'CTBC', '8': 'TIM',
+      '00': 'NEXTEL',
+      '02': 'TIM', '03': 'TIM', '04': 'TIM',
+      '05': 'CLARO', '06': 'VIVO', '07': 'CTBC', '08': 'TIM',
       '10': 'VIVO', '11': 'VIVO', '15': 'SERCOMTEL',
       '16': 'OI', '23': 'VIVO', '24': 'OI', '31': 'OI',
       '32': 'CTBC', '33': 'CTBC', '34': 'CTBC', '37': 'AEIOU'

The above should fix comment 2.

For comment 7, I believe LAC is not provided so it's invalid. Please file another bug if MCC/MNC/LAC are all set and the phone still behavior incorrectly.
Flags: needinfo?(timdream) → needinfo?(gweng)
Ok, I'll take a look and continue to solve this issue.
Flags: needinfo?(gweng)
Greg,

Any findings on the bug? Please note your investigation in the bug, partner will need this fix soon.
Flags: needinfo?(gweng)
I'm finishing this bug, and Tim has given me some useful suggestions. The main difficult is that I don't know what it should exactly looks like as I mentioned.
Flags: needinfo?(gweng)
Maybe Tim or someone else know how to test this without the same mobile network and the SIM card? I can't find how to reproduce it in such situation in the comments above.
Assignee: nobody → gweng
Flags: needinfo?(timdream)
Flags: needinfo?(timdream)
Ok I've asked Tim offline about how to test this issue.
Attached file Patch
Followed the Tim's suggestion and fixed & added the test.
Attachment #808364 - Flags: review?(timdream)
Attachment #808364 - Flags: review?(timdream) → review+
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #26)
> Comment on attachment 808364 [details]
> Patch
> 
> Removing r+ because of test failure:
> 
> https://travis-ci.org/mozilla-b2g/gaia/jobs/11672546#L2534

It's so strange. The code just passed tests on my machine... I'll check it and set the review flag again.
Comment on attachment 808364 [details]
Patch

I've clean and update the code. This time the related tests are passed.
Attachment #808364 - Flags: review?(timdream)
Tim

Please review the patch ASAP so it can land.
Flags: needinfo?(timdream)
Attachment #808364 - Flags: review?(timdream) → review+
Flags: needinfo?(timdream)
Greg, could you commit a v1-train patch and set the flag? I cannot merge the patch with git automatically.

Thanks.
Flags: needinfo?(gweng)
v1.2: ff5f7fa1a0fc849d9b2e5c325963c6ebd51ccd12
Tim: I've sent the pull request but again, I still lack the permission to merge it to v1-train, so please merge it for me, thanks.

https://github.com/mozilla-b2g/gaia/pull/12385
Flags: needinfo?(gweng)
v1.1.0hd: 6d9eec501a209bea945c6f841400ec0a75fac11d
v1.1.0hd: a7d949961c5d17a8648f59a0ffd719d418944b5c
Reverted v1.2: https://github.com/mozilla-b2g/gaia/commit/4e0125e47bb109cc7e7a91559d120b78bf65ace1 due to test failure.

Greg, when you back from Summit, please investigate why the tests failed and submit a patch that is green. We might have another patch applied on master only to fix the breakage, or there are other difference between the two inc. testing framework. Thanks!
Flags: needinfo?(gweng)
BTW the last bug touches the same area was bug 915038.
I now can't run the test-agent-test either on the lastest master or v1.2 branch, no matter if I revert my reverted patch or not. It reports:

[system] system/AppInstallManager > duringInstall > hosted app with cache > on first progress > on uninstall > "before each" hook:
     Error: TypeError: app.manifest is undefined (http://system.gaiamobile.org:8080/js/notifications.js?time=1381283756697:23)

I'll try it later to see what happened.
Flags: needinfo?(gweng)
You need to log in before you can comment on or make changes to this bug.