Following bug 985090, and investigating bug 983459, I noticed that the voicemail retrieval seems broken. When you receive a voicemail notification from mozVoicemail, we send a notification. Building this one we retrieve the voicemail number from settings, and if we have none, then we try to use the voicemail number from the mozVoicemail (i.e., from the SIM card). Adding some debug to callback of this.voiceMailNumberHelper.get in apps/system/js/voicemail.js: + 58 console.debug('numbers=' + JSON.stringify(numbers)); 59 var voicemail = navigator.mozVoicemail; 60 var number = numbers && numbers[simIndex]; + 61 console.debug('setting number=' + JSON.stringify(number)); 62 ' 63 if (!number && voicemail) { 64 number = voicemail.getNumber(status.serviceId); + 65 console.debug('voicemail number=' + JSON.stringify(number)); 66 } I get this output: E/GeckoConsole( 3680): Content JS DEBUG at app://system.gaiamobile.org/js/voicemail.js:58 in gotVMNumbers: numbers=["+33695600011"] E/GeckoConsole( 3680): Content JS DEBUG at app://system.gaiamobile.org/js/voicemail.js:61 in gotVMNumbers: setting number=undefined E/GeckoConsole( 3680): Content JS DEBUG at app://system.gaiamobile.org/js/voicemail.js:65 in gotVMNumbers: voicemail number="+33695600011" Which exposes that the 'numbers' array is potentially correctly used. My device has only one SIM (Nexus S), and I had to workaround in bug 985090 to get the numbers array. The simIndex is built as |var simIndex = status.serviceId + 1;| Which, in my case, produces: E/GeckoConsole( 3680): Content JS DEBUG at app://system.gaiamobile.org/js/voicemail.js:38 in vm_updateNotification: status.serviceId=0 E/GeckoConsole( 3680): Content JS DEBUG at app://system.gaiamobile.org/js/voicemail.js:39 in vm_updateNotification: simIndex=1 I'm not sure it makes sense to query index 1 in an array on a single device SIM.

This is a bug. We should use status.serviceId instead of simIndex for retrieving the voicemail number. :(

Gregor, Please review.

Probably DSDS regression. Can we find out if we have to fix this for tarako as well?

(In reply to Gregor Wagner [:gwagner] from comment #3) > Probably DSDS regression. Can we find out if we have to fix this for tarako > as well? We need STR to be able to test this. Although to my understanding, the suspected cause of this was bug 975918, which only landed on 1.4.

(In reply to Jason Smith [:jsmith] from comment #4) > (In reply to Gregor Wagner [:gwagner] from comment #3) > > Probably DSDS regression. Can we find out if we have to fix this for tarako > > as well? > > We need STR to be able to test this. Although to my understanding, the > suspected cause of this was bug 975918, which only landed on 1.4. No, this is another issue where we do not make use of the correct index to access the array.

Please find attached a Github pull request that fixes the issue

Comment on attachment 8394199 [details] Link to Github https://github.com/mozilla-b2g/gaia/pull/17388 Updated the PR with a new test. The issue is only exposed when retrieving voicemail from the settings.

Pending travis at https://travis-ci.org/mozilla-b2g/gaia/builds/21189945

Comment on attachment 8394199 [details] Link to Github https://github.com/mozilla-b2g/gaia/pull/17388 Arthur, do you mind reviewing this? Thanks!

Comment on attachment 8394199 [details] Link to Github https://github.com/mozilla-b2g/gaia/pull/17388 Thanks for the patch. f=me. Flagging the system app owner.

(In reply to Arthur Chen [:arthurcc] from comment #10) > Comment on attachment 8394199 [details] > Link to Github https://github.com/mozilla-b2g/gaia/pull/17388 > > Thanks for the patch. f=me. Flagging the system app owner. Thanks arthur and alive. The nit has been addressed, a new travis is pending at https://travis-ci.org/mozilla-b2g/gaia/builds/2124118

Hi Alexandre, it should be "MockNavigatorSettings.mSet" instead of "MockNavigatorSettings.createLock().mSet".

(In reply to Arthur Chen [:arthurcc] from comment #12) > Hi Alexandre, it should be "MockNavigatorSettings.mSet" instead of > "MockNavigatorSettings.createLock().mSet". Should be fixed in https://travis-ci.org/mozilla-b2g/gaia/builds/21241901

(In reply to Arthur Chen [:arthurcc] from comment #12) > Hi Alexandre, it should be "MockNavigatorSettings.mSet" instead of > "MockNavigatorSettings.createLock().mSet". This does not seems to be working.

(In reply to Alexandre LISSY :gerard-majax from comment #14) > (In reply to Arthur Chen [:arthurcc] from comment #12) > > Hi Alexandre, it should be "MockNavigatorSettings.mSet" instead of > > "MockNavigatorSettings.createLock().mSet". > > This does not seems to be working. i.e., MockNavigatorSettings.mSet() but then the resulting showNotification calls are not done with the correct values.

(In reply to Alexandre LISSY :gerard-majax from comment #15) > (In reply to Alexandre LISSY :gerard-majax from comment #14) > > (In reply to Arthur Chen [:arthurcc] from comment #12) > > > Hi Alexandre, it should be "MockNavigatorSettings.mSet" instead of > > > "MockNavigatorSettings.createLock().mSet". > > > > This does not seems to be working. > > i.e., MockNavigatorSettings.mSet() but then the resulting showNotification > calls are not done with the correct values. The correct solution was to use the mock for SettingsHelper. This has made the code to change a bit. If you think it needs another review, feel free.

Travis is green: https://travis-ci.org/mozilla-b2g/gaia/builds/21255461 I'll land when the tree re-opens.

master: https://github.com/mozilla-b2g/gaia/commit/7e25bb805660b462fc3f3ae93a5f026f263b694d

v1.4: https://github.com/mozilla-b2g/gaia/commit/6d6ca750be162bf7b154c5773253b8762e172567