985090 - Unable to change the Voicemail number in Settings after a data migration from 1.3 to 1.4

Status: RESOLVED FIXED

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

Description

[:gerard-majax]

While debugging bug 983459 I noticed that I cannot change the voicemail number in Settings: its value is '+' and it cannot be changed.

STR:
 0. Open Settings, Call Settings, tap on Voicemail
 1. Type the new number ([phone] in my case)
 2. Validate

Expected:
 New number changed

Actual:
 Nothing happens, voicemail is still '+'

In logcat, I can see:
E/GeckoConsole( 3000): [JavaScript Error: "TypeError: 0 is read-only" {file: "app://settings.gaiamobile.org/js/call_voicemail_settings.js" line: 36}]

This is 100% reproductibility on master from yesterday, on Nexus S.

Comment 1

[:gerard-majax]

As for bug 983459, this code has been touched by bug 975918.

Arthur, can you help me there?

Comment 2

[:gerard-majax]

Looks like queyring ril.iccInfo.mbdn I don't get an array but rather a string.

Hacking Settings app to force creating an array does the trick:

> diff --git a/apps/settings/js/call_voicemail_settings.js b/apps/settings/js/call_voicemail_settings.js
> index 7740b60..131d152 100644
> --- a/apps/settings/js/call_voicemail_settings.js
> +++ b/apps/settings/js/call_voicemail_settings.js
> @@ -33,6 +33,9 @@
>      var targetIndex = DsdsSettings.getIccCardIndexForCallSettings();
>      Settings.getSettings(function(results) {
>        var numbers = results['ril.iccInfo.mbdn'] || [];
> +      if (typeof numbers === 'string') {
> +        numbers = [ numbers ];
> +      }
>        numbers[targetIndex] = input.value;
>        navigator.mozSettings.createLock().set({
>          'ril.iccInfo.mbdn': number

I fear we have a regression due to DSDS.

Comment 3

[:gerard-majax]

Asking QA to check whether this may reproduce on 1.3 also.

Comment 4

[sarsenyev]

The issue doesn't reproduce on Buri device 1.4 or 1.3
User is able to change a voicemail number

Comment 5

[:gerard-majax]

(In reply to sarsenyev from comment #4)
> The issue doesn't reproduce on Buri device 1.4 or 1.3
> User is able to change a voicemail number

You should be able to reproduce at least on 1.4. There is clearly some context in action then. 
Can you make sure you try with a profile from before 1.3 ?

And then update to 1.3 and then 1.4.

On my device there was clearly a string stored in ril.iccInfo.mbdn rather than an array.

Comment 6

[Jason Smith [:jsmith]]

(In reply to Alexandre LISSY :gerard-majax from comment #5)
> (In reply to sarsenyev from comment #4)
> > The issue doesn't reproduce on Buri device 1.4 or 1.3
> > User is able to change a voicemail number
> 
> You should be able to reproduce at least on 1.4. There is clearly some
> context in action then. 
> Can you make sure you try with a profile from before 1.3 ?
> 
> And then update to 1.3 and then 1.4.
> 
> On my device there was clearly a string stored in ril.iccInfo.mbdn rather
> than an array.

The problem is that's not really the same bug as filed. The bug as filed indicates that the user is unable to change a voicemail number without any data migration STR included.

Comment 7

[:gerard-majax]

(In reply to Jason Smith [:jsmith] from comment #6)
> (In reply to Alexandre LISSY :gerard-majax from comment #5)
> > (In reply to sarsenyev from comment #4)
> > > The issue doesn't reproduce on Buri device 1.4 or 1.3
> > > User is able to change a voicemail number
> > 
> > You should be able to reproduce at least on 1.4. There is clearly some
> > context in action then. 
> > Can you make sure you try with a profile from before 1.3 ?
> > 
> > And then update to 1.3 and then 1.4.
> > 
> > On my device there was clearly a string stored in ril.iccInfo.mbdn rather
> > than an array.
> 
> The problem is that's not really the same bug as filed. The bug as filed
> indicates that the user is unable to change a voicemail number without any
> data migration STR included.

Well, I had no way to know prior to asking QA. All my devices are running 1.4 newer than the DSDS commit. So now at least we know on a fresh build it seems to be okay.

Comment 8

[Jason Smith [:jsmith]]

So what do you want checked now? Do you want to see if this reproduces if we do a data migration from 1.3 to 1.4?

Comment 9

[:gerard-majax]

(In reply to Jason Smith [:jsmith] from comment #8)
> So what do you want checked now? Do you want to see if this reproduces if we
> do a data migration from 1.3 to 1.4?

Upgrading from anything prior to commit 7c3f9ce222c5fafbd338e7eef777e29d8b5e6f5a would be good, I think. This has been merged on March, 11th.

I'm doing all my updates using MAR packages, if it may help.

Maybe we should wait for Arthur's feedback on this, since it looks not being that reproductible.

Comment 10

[:gerard-majax]

I just upgraded my Keon from a build of around march 7th, to yesterday's build, and I'm reproducing the issue also.

This is with a different profile, and a different SIM card.

Comment 11

[:gerard-majax]

I'm also noticing the issue on my Peak, and my Desire Z. All with different SIM cards.

Comment 12

[Jason Smith [:jsmith]]

Okay - leaving QA Wanted to test this by doing an OTA update from 1.3 --> 1.4 on a Leo device and then executing the STR in comment 0.

Comment 13

[:gerard-majax]

(In reply to Jason Smith [:jsmith] from comment #12)
> Okay - leaving QA Wanted to test this by doing an OTA update from 1.3 -->
> 1.4 on a Leo device and then executing the STR in comment 0.

If it confirms the issue (I would be surprised if not, given the number of devices on which I reproduce).

Bug 975918 was not 1.3+, so we don't care I think :)

Comment 14

[sarsenyev]

The issue reproduces on Leo device after OTA from 1.3 to 1.4
The bar shows "+" sign and cannot be updated

1.4 Environmental Variables:
Device: Leo 1.4 MOZ
BuildID: 20140318000203
Gaia: c03a6af9028c4b74a84b5a98085bbb0c07261175
Gecko: 3776f72f1967
Version: 30.0a2
Firmware Version: v1.2-device.cfg

Comment 15

[Jason Smith [:jsmith]]

Okay - so this is a confirmed data migration bug then.

Comment 16

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

Could you share the STR you used for migration? Did you use make reset-gaia or make install-gaia?

Comment 17

[Jason Smith [:jsmith]]

(In reply to Arthur Chen [:arthurcc] from comment #16)
> Could you share the STR you used for migration? Did you use make reset-gaia
> or make install-gaia?

QA tested this via doing an OTA from 1.3 --> 1.4 using a Leo device & then running the STR in comment 0.

Alexandre I think did this differently though.

Comment 18

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

Root cause found. https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/operator_variant/operator_variant.js#L409

We should not set the result back to result[0] as it has already been set to ['', ''].
Writing a patch and adding unit tests for it.

Comment 19

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

Attachment: Link to https://github.com/mozilla-b2g/gaia/pull/17373

The root cause is as my previous comment suggested. I re-written part of the  settings update logic to make it more readable and easier to test.

Jose, could you help review the patch? Thanks!

Comment 20

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8394030 [details]
Link to https://github.com/mozilla-b2g/gaia/pull/17373

LGTM, r=me

Travis is green on the PR, tests pass, I tested this on hamachi on a fresh build and after performing a system update and everything seems to work correctly.

Thanks Arthur!

Comment 21

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

Thanks, Jose!

master: 06cfe08cd72f32404f304d34fb902b544bdfeb9e

Comment 22

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/0ef43a4a1fa72837fa7c62b60fb8a0dab816e020