981989 - [DSDS] Fixed dialing numbers page is not separated for 2 SIM card, which means that if set SIM 1 to enabled, same page of SIM 2 will become enabled as well even it's disabled in Call settings.

Status: VERIFIED FIXED

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

Description

[Enpei]

Attachment: fugu_FDN.txt

Fixed dialing numbers page displaying after select FDN in Call settings is actually only a global page, which means that when user enables FDN on SIM 1, then go to SIM 2 Call settings, FDN is disabled but if select FDN, it shows FDN is enabled on Fixed dialing number page.  

* Build Number  
Gaia      b193de26878dae53a4b20b78e453ed07bfbf4d09
Gecko     85f09a609c5d96f23988df41c58932e57d1e398e
BuildID   20140311053055
Version   30.0a1

* Reproduce Steps
1. Both SIM card FDN is off.
2. Go to SIM 1 call settings > fixed dialing numbers, enable FDN.
3. Go back to SIM picker, select SIM 2, FDN is diabled in Call settings page.
4. Select Fixed dialing numbers item.

* Expected Result
FDN is disabled. 

* Actual Result
FDN is enabled. But actually FDN is disabled from functionality perspective, it's just Fixed dialing numbers page is following latest update by any SIM, not the real FDN status of each SIM.

* Occurrence rate
100%

Comment 1

[Enpei]

Attachment: VIDEO0041.mp4

FDN is disabled while it shows enabled in Fixed dialing numbers.

Comment 2

[Enpei]

One reason nominate this bug to 1.4 is that if user enable SIM 1 FDN while SIM 2 is disabled, then go to SIM 2 try to enable FDN as well, user needs to disable FDN in Fixed dialing numbers page first then enable it again, which is no reasonable and really troublesome.

Comment 3

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

hi Sergi, do you mind taking a look at this as well? Thanks

Comment 4

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Attachment: Github PR

The FDN status was not refreshed in the FDN settings page, so it always would adopt the status of the last card checked for it. This patch adds a check whenever the FDN Settings panel is ready.

Comment 5

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

Comment on attachment 8389181 [details] [review]
Github PR

Hi Sergi, please check my github comment. And could you also add unit tests for it? Thanks!

Comment 6

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Hi Arthur,

I updated the pull request with your suggestion.

About adding unit tests, there is really not much functionality added and it is rather visual. Do you mean UI tests or were you meaning to mock the whole Settings environment and unit test the init of this panel? In that case, it would help if you can point me to an example of a similar test.

Thanks!

Comment 7

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

I think the unit tests could be like dispatching a 'panelready' event and then ensuring 'renderAuthorizedNumbers' and 'updateFdnStatus' are called correspondingly. :)

Comment 8

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Comment on attachment 8389181 [details] [review]
Github PR

Fixed your remaining concerns and added unit tests.

Comment 9

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

1.4+, blocks a user story

Comment 10

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

Comment on attachment 8389181 [details] [review]
Github PR

We are almost there, please check my github comments, thanks!

Comment 11

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Hi Arthur,

Fixed some of the issues. Please see my comment about the remaining one.

Sergi

Comment 12

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

Sorry for the late response. I was able to get the test work with the loadBodyHTML function. Detail please check my github comment.

Comment 13

[Fabien Cazenave [:kaze]]

Comment on attachment 8389181 [details] [review]
Github PR

Waiting for Arthur’s comments to be addressed, please r?-me again when you’re ready.

Comment 14

[Fabien Cazenave [:kaze]]

We have a linter error:

> apps/settings/test/unit/simcard_fdn_test.js: line 2, col 56,
> 'appendToBodyHTML' is defined but never used. (ERROR)

Comment 15

[Fabien Cazenave [:kaze]]

Comment on attachment 8389181 [details] [review]
Github PR

LGTM, please remove `appendToBodyHTML' from the `global' jshint list and make sure the linters are happy before merging.

Comment 16

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Sergi, what stops this bug from landing?

Comment 17

[Sergi Mansilla [:sergi] (Telenor) [PTO]]

Landed:

https://github.com/mozilla-b2g/gaia/commit/8a59a400f98cacad5b3e0415a5099a5353cb45f0

Comment 18

[(PTO June 19-July 7) Ryan VanderMeulen]

v1.4: https://github.com/mozilla-b2g/gaia/commit/216cc2e5c8692ba71aa78a9f27e84e9da27952b8

Comment 19

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

Verified okay with v1.4 on Fugu.

GAIA_REV=441c4bcd8ac4f8c01a9bc5a2f8d64eaa87844803
GECKO_REV=0c18beacb37a7e1e80bcf6d46cc8ff9a18c0c653
GAIA_BRANCH=mozillaorg/v1.4
GECKO_BRANCH=mozillaorg/v1.4
BUILD_TAG=jenkins-B2G.v1.4.0.fugu-25
BuildID=20140416041552

Comment 20

[Lina.Guo]

Dear Sergi:
 
   v1.3 also has the same issue.I have merge the code to V1.3,but it does not work.
   I checked the code found that the differents between them is :

   updateFdnStatus: function spl_updateSimStatus() {
    var self = this;
    var req = IccHelper.getCardLock('fdn');/////v1.3 has not get the current Icc


   updateFdnStatus: function spl_updateSimStatus() {///v1.4
    var self = this;
    var iccObj = getIccByIndex();
    if (!iccObj) {
      return console.error('Could not retrieve ICC object');
    }
    var req = iccObj.getCardLock('fdn');
  
Please help tell me how to modify it, thanks a lot.

Comment 21

[Lina.Guo]

Attachment: bug336906_FDN_of_dual_SIM_synchronously.patch V1.3

Hi Sergi:

   I have modified the code,and test ok.

   Please kindly help to review my patch,thanks a lot.