1026381 - [Single Variant] if NFC value was changed, it is not preserved when the user inserts a configured SIM

Status: RESOLVED FIXED

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

Description

[rafael.marquez]

*Pre-requisites :
Having a variant.json configured for our sim card with a line similar to: "nfc": true,
Having a configured sim card


*Procedure
1. Completing the FTE without sim inserted
2. Open Settings app
3. Enable NFC
4. Disable NFC
5. Enable NFC
6. Disable NFC
7. Turn off the device
8. Insert a configured sim card
9. Turn on the device

*Expected result
Customization process is launched. NFC value changed by the user before insert a configured sim card is preserved when the user inserts a configured SIM. In this case NFC continues configured as deactivated


*Actual Result
NFC value changed by the user before insert a configured sim card is NOT preserved when the user inserts a configured SIM. In this case NFC is configured as activated.

Comment 1

[Preeti Raghunath(:Preeti)]

Peter,

Please weigh in with expected behavior. A product call would be good here.

Comment 2

[Peter Dolanjski [:pdol]]

Blocking.  This is blowing away a user's selected setting.

Comment 3

[:gerard-majax]

Let's try to do something.

Comment 4

[:gerard-majax]

I'm not even able to get any operator variant to work at all on master:
$ ADB="adb -s 94fdcea6" make MOZILLA_OFFICIAL=1 PRODUCTION=1 REMOTE_DEBUGGER=1 NOFTU=0 GAIA_DEV_PIXELS_PER_PX=1.5 GAIA_OPTIMIZE=1 GAIA_DISTRIBUTION_DIR=$PWD/customization reset-gaia

Comment 5

[:gerard-majax]

I got variant partly working, but the process to build and flash the profile and get variant to be applied seems to be totally unreliable and not always reproductible, i.e., same commands several times won't produce the same result.

Comment 6

[:gerard-majax]

The only documentation I have been able to find was: https://developer.mozilla.org/en-US/Firefox_OS/Developing_Firefox_OS/Market_customizations_guide#Steps_to_apply_customization

I'm unable to consistently produce a working gaia profile with operatir variant enabled, running several times this command result in sometimes operatorvariant being properly ran, and sometimes not:
> make clean && make really-clean && make MOZILLA_OFFICIAL=1 PRODUCTION=1 REMOTE_DEBUGGER=1 NOFTU=0 GAIA_DEV_PIXELS_PER_PX=1.5 GAIA_OPTIMIZE=1 GAIA_DISTRIBUTION_DIR=$PWD/customization/ production

I can't tell whether variant customization is just completely broken or badly documented, but I have already lost much more than acceptable time on this.

Comment 7

[:gerard-majax]

Albert, I've been told you know this code quite well. Can you either help me getting customization to work properly and consistently or assign yourself this bug and fix it? Thanks!

Comment 8

[Francisco Jordano [:arcturus] [:francisco]]

I've seen Alexandre getting crazy this afternoon try to trigger the OV, Carmen is something else we are missing?

Thanks a lot!

Comment 9

[Albert [:albert]]

(In reply to Alexandre LISSY :gerard-majax from comment #6)
> The only documentation I have been able to find was:
> https://developer.mozilla.org/en-US/Firefox_OS/Developing_Firefox_OS/
> Market_customizations_guide#Steps_to_apply_customization
> 
> I'm unable to consistently produce a working gaia profile with operatir
> variant enabled, running several times this command result in sometimes
> operatorvariant being properly ran, and sometimes not:
> > make clean && make really-clean && make MOZILLA_OFFICIAL=1 PRODUCTION=1 REMOTE_DEBUGGER=1 NOFTU=0 GAIA_DEV_PIXELS_PER_PX=1.5 GAIA_OPTIMIZE=1 GAIA_DISTRIBUTION_DIR=$PWD/customization/ production
> 
> I can't tell whether variant customization is just completely broken or
> badly documented, but I have already lost much more than acceptable time on
> this.

I tried your command several times and it worked fine. How do you know it is not working as expected?

In order to test it, you can just use the command: GAIA_DISTRIBUTION_DIR=customization/ make reset-gaia

To check if single variant is configured you have to look $GAIA_HOME/build_stage/operatorvariant folder. If all goes fine you should see a resources folder, if not it means no single variant is applied.

Another think you should check is if there is some configuration for the mcc/mnc of your SIM card in the variant.json at $GAIA_HOM/customization/variant.json. If you didn't edit the variant.json, only SIMs with mcc/mnc 310/260 and 311/261 are configured with single variant.

Comment 10

[:gerard-majax]

(In reply to Albert [:albert] from comment #9)

[...]

> 
> I tried your command several times and it worked fine. How do you know it is
> not working as expected?

Because, for example, I added console.debug() calls in the operatorvariant init() code that installs the system message handler, and it was only triggered randomly and not consistently.

> 
> In order to test it, you can just use the command:
> GAIA_DISTRIBUTION_DIR=customization/ make reset-gaia

Complete failure also. Moreover, my testing suggests that the build system cannot cope with relative paths as I was constantly getting errors about missing variant.json when using them. Using absolute paths, no error about the file.

> 
> To check if single variant is configured you have to look
> $GAIA_HOME/build_stage/operatorvariant folder. If all goes fine you should
> see a resources folder, if not it means no single variant is applied.

It seems to be there:
$ ls build_stage/operatorvariant/resources/
total 944K
drwxr-xr-x 2 alex alex 4,0K juin  25 17:22 .
drwxr-xr-x 6 alex alex 4,0K juin  25 17:22 ..
-rw-r--r-- 1 alex alex  845 juin  25 17:22 customization.json
-rw-r--r-- 1 alex alex  216 juin  25 17:22 keyboard-88cf36fbc274369ce1c2bea24dffce3017cc6f69.json
-rwxr-xr-x 1 alex alex  197 juin  25 17:22 mobizilla_contacts.json
-rw-r--r-- 1 alex alex  235 juin  25 17:22 mobizilla_default_search.json
-rw-r--r-- 1 alex alex  947 juin  25 17:22 mobizilla_known_networks.json
-rw-r--r-- 1 alex alex  439 juin  25 17:22 mobizilla_network_type.json
-rw-r--r-- 1 alex alex  30K juin  25 17:22 mobizilla_poweroff.png
-rw-r--r-- 1 alex alex  56K juin  25 17:22 mobizilla_poweron.mp4
-rw-r--r-- 1 alex alex 370K juin  25 17:22 mobizilla_ringtone.ogg
-rw-r--r-- 1 alex alex 5,4K juin  25 17:22 mobizilla_search1.ico
-rw-r--r-- 1 alex alex 5,4K juin  25 17:22 mobizilla_search2.ico
-rw-r--r-- 1 alex alex 1,2K juin  25 17:22 mobizilla_search3.ico
-rw-r--r-- 1 alex alex  505 juin  25 17:22 mobizilla_search.json
-rw-r--r-- 1 alex alex   52 juin  25 17:22 mobizilla_sms.json
-rwxr-xr-x 1 alex alex  254 juin  25 17:22 mobizilla_support_contacts.json
-rw-r--r-- 1 alex alex 5,3K juin  25 17:22 mobizilla_topsites.json
-rwxr-xr-x 1 alex alex  82K juin  25 17:22 mobizilla_wallpaper.png
-rw-r--r-- 1 alex alex   34 juin  25 17:22 nfc-93c047a6d0389e755cacdbd6bb0986fff0576aee.json
-rw-r--r-- 1 alex alex  185 juin  25 17:22 power-0ccc24f04b44aaadc8962735b5f86eabc5bb71e6.json
-rw-r--r-- 1 alex alex  114 juin  25 17:22 ringtone-8261e854cb494bf7f3a2a25510e595931244292a.json
-rw-r--r-- 1 alex alex 315K juin  25 17:22 wallpaper-7b8d66705b283f474c5892e70ece5890df7f9be2.json


> 
> Another think you should check is if there is some configuration for the
> mcc/mnc of your SIM card in the variant.json at
> $GAIA_HOM/customization/variant.json. If you didn't edit the variant.json,
> only SIMs with mcc/mnc 310/260 and 311/261 are configured with single
> variant.

That's the first thing I made sure.

Comment 11

[:gerard-majax]

Oh, and of course, another sign of not working customization was that there was ... no customization: invalid wallpaper, invalid preloaded stuff, etc.

Comment 12

[Albert [:albert]]

(In reply to Alexandre LISSY :gerard-majax from comment #11)
> Oh, and of course, another sign of not working customization was that there
> was ... no customization: invalid wallpaper, invalid preloaded stuff, etc.

Sometimes I had problems with the /persist partition, because depending on the device and the single variant apps size there is no enough space and the gecko part responsible of throwing the message is not executed. What device are you using?

Can you attach the output of your make and the adb logcat to see if there is something wrong?

Comment 13

[:gerard-majax]

(In reply to Albert [:albert] from comment #12)
> (In reply to Alexandre LISSY :gerard-majax from comment #11)
> > Oh, and of course, another sign of not working customization was that there
> > was ... no customization: invalid wallpaper, invalid preloaded stuff, etc.
> 
> Sometimes I had problems with the /persist partition, because depending on
> the device and the single variant apps size there is no enough space and the
> gecko part responsible of throwing the message is not executed. What device
> are you using?

That's something I can investigate. Flame, custom build.

> 
> Can you attach the output of your make and the adb logcat to see if there is
> something wrong?

It's bad if we do not stop on an error. I'll generate this output as soon as possible.

Comment 14

[:gerard-majax]

Meanwhile,

> make_ext4fs -s -l 5242880 -a persist out/target/product/flame/persist.img out/target/product/flame/persist
> Creating filesystem with parameters:
>     Size: 5242880
>     Block size: 4096
>     Blocks per group: 32768
>     Inodes per group: 320
>     Inode size: 256
>     Journal blocks: 1024
>     Label: 
>     Blocks: 1280
>     Block groups: 1
>     Reserved block group size: 7
> Created filesystem with 12/320 inodes and 1066/1280 blocks
> out/target/product/flame/persist.img maxsize=5136384 blocksize=135168 total=4366496 reserve=270336

Comment 15

[Albert [:albert]]

If you can also enable gecko's debug it would be helpful.

https://mxr.mozilla.org/mozilla-central/source/dom/apps/src/OperatorApps.jsm#30

Comment 16

[:gerard-majax]

Output of the debug.

Doing a first reset-gaia:
> $ adb logcat | egrep -i operator
> I/Gecko   ( 1153): -*-*- OperatorApps.jsm : init
> I/Gecko   ( 1153): -*-*- OperatorApps.jsm : No First Run with SIM
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : init
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : First Run with SIM
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : copying /data/local/svoperapps to /persist/svoperapps
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : copying /data/local/svoperapps/Twitter to /persist/svoperapps/Twitter
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : removing directory:/data/local/svoperapps
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : appsDir SET: /persist/svoperapps
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : ******* iccListener cardIccInfo MCC-MNC: 208-01
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : Install operator apps ---> mcc:208, mnc:01
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : Broadcast message first-run-with-sim
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : First Run with configured SIM 
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : installOperatorApps --> aIdsApp:["Twitter"]
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : metadata:{"id":"Twitter","installOrigin":"https://marketplace.firefox.com","manifestURL":"https://mobile.twitter.com/cache/twitter.webapp","origin":"https://mobile.twitter.com"}
> I/Gecko   ( 1440): -*-*- OperatorApps.jsm : aId:Twitter. Installing as hosted app.
> E/GeckoConsole( 1643): Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/operator_variant.js:32 in ovm_init: OperatorVariantManager: init()
> E/GeckoConsole( 1643): Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/operator_variant.js:34 in ovm_init/<: OperatorVariantManager: first-run-with-sim: E
> E/GeckoConsole( 1643): Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/operator_variant.js:35 in ovm_init/<: OperatorVariantManager: first-run-with-sim: E: msg={"mcc":"208","mnc":"01"}
> E/GeckoConsole( 1643): Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/operator_variant.js:37 in ovm_init/<: OperatorVariantManager: first-run-with-sim: mcc_mnc=208-001
> E/GeckoConsole( 1643): Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/operator_variant.js:39 in ovm_init/<: OperatorVariantManager: first-run-with-sim: DO
> E/GeckoConsole( 1643): Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/operator_variant.js:46 in ovm_init/<: OperatorVariantManager: first-run-with-sim: X

So triggered properly once.

Then doing another reset-gaia:
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : init
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : First Run with SIM
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : copying /data/local/svoperapps to /persist/svoperapps
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : copying /data/local/svoperapps/Twitter to /persist/svoperapps/Twitter
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : removing directory:/data/local/svoperapps
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : appsDir SET: /persist/svoperapps
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : ******* iccListener cardIccInfo MCC-MNC: 208-01
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : Install operator apps ---> mcc:208, mnc:01
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : Broadcast message first-run-with-sim
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : First Run with configured SIM 
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : installOperatorApps --> aIdsApp:["Twitter"]
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : metadata:{"id":"Twitter","installOrigin":"https://marketplace.firefox.com","manifestURL":"https://mobile.twitter.com/cache/twitter.webapp","origin":"https://mobile.twitter.com"}
> I/Gecko   ( 1827): -*-*- OperatorApps.jsm : aId:Twitter. Installing as hosted app.

And Gaia counterpart not ran.

In both case, I used the same command line as documented earlier.

Comment 17

[:gerard-majax]

> root@flame:/ # df /persist/svoperapps
> Filesystem               Size     Used     Free   Blksize
> /persist/svoperapps      4.9M     4.1M   832.0K   4096

Comment 18

[Albert [:albert]]

So it doesn't seem a problem with the persist partition, the broadcast message is being lost for some reason.

I only have an hamachi now, I'll try to reproduce it tomorrow with Flame.

Comment 19

[:gerard-majax]

Attachment: operator.log

And another reset-gaia.

I'm not a big fan of this line:
> 06-25 22:49:57.245  2310  2310 E GeckoConsole: [JavaScript Error: "types.forEach is not a function" {file: "resource://gre/modules/ActivitiesService.jsm" line: 428}]

Comment 20

[Albert [:albert]]

(In reply to Alexandre LISSY :gerard-majax from comment #19)
> Created attachment 8446116 [details]
> operator.log
> 
> And another reset-gaia.
> 
> I'm not a big fan of this line:
> > 06-25 22:49:57.245  2310  2310 E GeckoConsole: [JavaScript Error: "types.forEach is not a function" {file: "resource://gre/modules/ActivitiesService.jsm" line: 428}]

I have same error in hamachi but single variant works fine.

Comment 21

[Albert [:albert]]

(In reply to Albert [:albert] from comment #20)
> (In reply to Alexandre LISSY :gerard-majax from comment #19)
> > Created attachment 8446116 [details]
> > operator.log
> > 
> > And another reset-gaia.
> > 
> > I'm not a big fan of this line:
> > > 06-25 22:49:57.245  2310  2310 E GeckoConsole: [JavaScript Error: "types.forEach is not a function" {file: "resource://gre/modules/ActivitiesService.jsm" line: 428}]
> 
> I have same error in hamachi but single variant works fine.

I tried to reproduce it with open C because is similar to flame, but all is fine. I haven't a Flame, and I can't have one in one or two weeks, so Carmen is going to help you because she can get one flame.

Comment 22

[:gerard-majax]

Thanks albert, I'll wait for Carmen's feedback. And since I now have something I can investigate, taking the bug back and hacking to find why the operatorvariant app is not ran properly.

Comment 23

[Carmen Jimenez Cabezas]

There are several issues here: 

* The activities error is unrelated, I think, and it seems to happen when a view activity is invoked with a type filter that it's not a string or an array. Antonio, can you take a look at that in case it's related with the DRM changes you made?

* The GAIA_DISTRIBUTION_DIR has to be an absolute path, as you said.

* And I don't know why the system message is being lost. On my own tests that worked reliably always. I'll take a look at this on the Flame

Comment 24

[Albert [:albert]]

(In reply to Carmen Jimenez Cabezas from comment #23)
> There are several issues here: 
> 
> * The activities error is unrelated, I think, and it seems to happen when a
> view activity is invoked with a type filter that it's not a string or an
> array. Antonio, can you take a look at that in case it's related with the
> DRM changes you made?
> 
> * The GAIA_DISTRIBUTION_DIR has to be an absolute path, as you said.

That's not true, I always use  'GAIA_DISTRIBUTION_DIR=../single-variant/firefox-gaia-testsbuild make reset-gaia' and never had problems.

Comment 25

[:gerard-majax]

Attachment: operator.nok.log

Gecko/Gaia log when operatorvariant does not start.

Comment 26

[:gerard-majax]

Attachment: operator.ok.log

Gecko/Gaia logs when operatorvariant properly starts.

Comment 27

[:gerard-majax]

Attachment: operator.nok2.log

Gecko/Gaia logs when not working

Comment 28

[:gerard-majax]

Attachment: operator.ok2.log

Gecko/Gaia logs when working

Comment 29

[:gerard-majax]

Attachment 8446563 [details] and attachment 8446564 [details] exposes the gecko and gaia log with some debug added in both case: working and non working.

From those logs, we can see that in all cases, the open-app event is properly sent from Gecko upon detecting a first run with a SIM card.

In attachement 8446564, this is done at line 394. Later, line 399 shows that the Gaia window management code handles this and opens the app. OperatorVariantManager processing is done, as proved by line 440.

In attachment 8446563 [details], open-app event is sent at line 272. But we cannot find any trace of its handling by the window management code.

A closer look shows that the AppWindowFactory addEventListener("open-app") seems to performed AFTER the open-app event is sent by the SystemAppProxy in the bogus case.

Comment 30

[Alexandre Poirot [:ochameau]]

It looks like the is a pretty significant chance of races around mozChromeEvent during startup!

Thing is that we try to avoid that from gecko side, by waiting for load event of the system app before dispatching any event:
http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#650

But, on the gaia side:
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/bootstrap.js#L41
we also wait for load event before registering event listener (like here appWindowFactory.start(), that listen for app-open events).
I would imagine that we end up calling this gaia load listener before the gecko load listener.
But what is worse is that under some conditions, we may delay event more the registration of event listener by waiting for applicationready event. In such scenario, I can easily see this code race a lot!

Anyway, I think we should do something here and either never delay event listener registration in gaia or figure out a way to delay a bit more mozChromeEvent dispatch from gecko side.

Comment 31

[:gerard-majax]

Comment 30 makes totally sense. I've moved the loading of OperatorApps.jsm from before we wait for the 'load' event to when we receive it, and then wait another 5 seconds to leave time for AppWindowFactory to register its event handler. Once this is done, I get operatorvariant properly started with 100% chances.

Comment 32

[:gerard-majax]

Now that the customization issue has been documented, I do reproduce the issue documented in comment 1. Let's file another bug for the race condition.

Comment 33

[:gerard-majax]

So the logic for operator variant customization of NFC is the following: if the setting is undefined OR is equal to the default value, then we consider the user has NOT changed the setting value.

This is handled at https://github.com/mozilla-b2g/gaia/blob/b8f36518696f3191a56e4f33447ee9d6ec820da1/apps/operatorvariant/js/customizers/nfc_customizer.js#L25, by |if (value === undefined || value === nfcParams.default) {|

However, the way the nfcParams.default value is built makes it a non-sense: in https://github.com/mozilla-b2g/gaia/blob/b8f36518696f3191a56e4f33447ee9d6ec820da1/apps/operatorvariant/build/build.js#L306, we |default: this.settings['nfc.enabled']|. So we reuse the default Gaia NFC setting, defined to false at https://github.com/mozilla-b2g/gaia/blob/b8f36518696f3191a56e4f33447ee9d6ec820da1/build/config/common-settings.json#L112.

This results in the following during execution:

> 6-26 11:20:19.420  1013  1013 E GeckoConsole: Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/customizers/nfc_customizer.js:28 in nc_onsuccess: NfcCustomizer: onsuccess: typeof value=boolean
> 06-26 11:20:19.430  1013  1013 E GeckoConsole: Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/customizers/nfc_customizer.js:29 in nc_onsuccess: NfcCustomizer: onsuccess: value=false
> 06-26 11:20:19.440  1013  1013 E GeckoConsole: Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/customizers/nfc_customizer.js:30 in nc_onsuccess: NfcCustomizer: onsuccess: typeof nfcParams.default=boolean
> 06-26 11:20:19.440  1013  1013 E GeckoConsole: Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/customizers/nfc_customizer.js:31 in nc_onsuccess: NfcCustomizer: onsuccess: nfcParams.default=false
> 06-26 11:20:19.450  1013  1013 E GeckoConsole: Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/customizers/nfc_customizer.js:32 in nc_onsuccess: NfcCustomizer: onsuccess: (value === undefined)=false
> 06-26 11:20:19.460  1013  1013 E GeckoConsole: Content JS DEBUG at app://operatorvariant.gaiamobile.org/js/customizers/nfc_customizer.js:33 in nc_onsuccess: NfcCustomizer: onsuccess: (value === nfcParams.default)=true

So the NfcCustomizer thinks the user has not changed the value because the user has set it to false.

Comment 34

[:gerard-majax]

Attachment: Link to the github pull request https://github.com/mozilla-b2g/gaia/pull/21099

Comment 35

[:gerard-majax]

Comment on attachment 8447052 [details] [review]
Link to the github pull request https://github.com/mozilla-b2g/gaia/pull/21099

Sending a first PR with the testcase that exposes the bug.

Comment 36

[Carmen Jimenez Cabezas]

The change proposed to the test do no expose the failure.

On the test:
-testCases[x].inputValue: represents the values that operator variant customizer will receive.
   where isEnabled will be the single variant desired value and default is the value configured on the phone at first boot
-currentValue: represents the value of the setting before the test starts
-expectedValue: is the value that the setting should have at the end of the test

The original requirement said that SingleVariant should change the value if the current one haven't changed respect to the start value. And there is a test case that reflects this circunstance (the first one)
      title: 'set true value. It has not changed previously > ',
      inputValue: {
        'isEnabled': true,
        'default': false
      },
      currentValue: false,
      expectedValue: true
    },

Now we need to recognize if the user has ever changed the value, even if the current value is the default (default.pref). For that we should keep that information (the user has changed the value) in some place.

Actually, Gecko already knows this (since it stores a defaultValue and a userValue for the settings) but it isn't exposed to content.

Comment 37

[:gerard-majax]

(In reply to Carmen Jimenez Cabezas from comment #36)

[...]

> The original requirement said that SingleVariant should change the value if
> the current one haven't changed respect to the start value. And there is a
> test case that reflects this circunstance (the first one)

Let me confirm that I properly understood the semantic there:

>       title: 'set true value. It has not changed previously > ',
>       inputValue: {
>         'isEnabled': true,
>         'default': false
>       },

This matches the build config where NFC is set to true in variant.json, and gaia settings defaults to false for NFC.

>       currentValue: false,

This matches that the user has set NFC to disabled.

>       expectedValue: true
>     },

This matches that the nfc.enabled setting has been turned to true. But still, this bug is about keeping the nfc.enabled value to false. So I'm lost.

> 
> Now we need to recognize if the user has ever changed the value, even if the
> current value is the default (default.pref). For that we should keep that
> information (the user has changed the value) in some place.
> 
> Actually, Gecko already knows this (since it stores a defaultValue and a
> userValue for the settings) but it isn't exposed to content.

Yes, and both the API and the events do not expose this, and I doubt we can expose it for 2.0.

Comment 38

[Carmen Jimenez Cabezas]

Yep, the semantic is correct. But you will need to add another flag to the test or some way to indicate that the test should change the value a couple of times. With the current test cases semantic there's no way to say: "the current value is the same as the default value, but it's not the default value anymore"
which is what you need for this test

Comment 39

[:gerard-majax]

(In reply to Carmen Jimenez Cabezas from comment #38)
> Yep, the semantic is correct. But you will need to add another flag to the
> test or some way to indicate that the test should change the value a couple
> of times. With the current test cases semantic there's no way to say: "the
> current value is the same as the default value, but it's not the default
> value anymore"
> which is what you need for this test

I don't follow you. What we need for this test is to ensure the 'false' stays in place.

FYI, hacking default Gaia setting value from false to null seems to be a workaround that we will be able to get into 2.0 in time.

Comment 40

[Carmen Jimenez Cabezas]

Hmm... that's a nice trick, that would be enough for this bug because the value can only be true or false-ish.

What I was trying to say before is that if you use 
{
  inputValue: {
    'isEnabled': true,
    'default': false
  },
  currentValue: false,
  expectedValue: true
},

there's no way to distinguish between currentValue being false because the user switched the value twice, and currentValue being false because he didn't change it. In the first case the value should not change, in the second case it should change (and so you would have two test cases like):


User didn't change the value: {
  inputValue: {
    'isEnabled': true,
    'default': false
  },
  currentValue: false,
  expectedValue: true
},

User did change the value: {
  inputValue: {
    'isEnabled': true,
    'default': false
  },
  currentValue: false,
  expectedValue: false
},

where as you can see, the preconditions are the same (same input value and same current value) and yet the expected value is different. That's why I said you needed something more to distinguish both cases.

You're right that using 

{
  inputValue: {
    'isEnabled': true,
    'default': null
  },
  currentValue: false,
  expectedValue: true
},
 
instead offers a way to distinguish both cases. You cannot use that to fix the bug though since... the default value might be true instead of false. I guess you could then set the default to some truish value instead of true though....

Comment 41

[:gerard-majax]

I fear you are right on your last point, if the default setting value is true we are stuck. But then:
 (1) the gaia default value in the repository for nfc.enabled is false
 (2) setting |"true"| instead of |true| should do the trick

Comment 42

[Antonio Manuel Amaya Calvo (:amac)]

(In reply to Carmen Jimenez Cabezas from comment #23)
> There are several issues here: 
> 
> * The activities error is unrelated, I think, and it seems to happen when a
> view activity is invoked with a type filter that it's not a string or an
> array. Antonio, can you take a look at that in case it's related with the
> DRM changes you made?

  Nope, totally unrelated :). Anyway, bug 1032121 has been born.

Comment 43

[:gerard-majax]

So there has been spone offline debates going on about reconsidering this as bocker or not, but the bug itself is still 2.0+. We cannot probably do better than the hack proposed in comment 41.

Comment 44

[Peter Dolanjski [:pdol]]

Marce, we either need to fix this bug or back out the NFC single variant change.
What do you suggest?

Comment 45

[Marcelino Veiga Tuimil [:sonmarce]]

I suggest to update bug for not applying single variant configuration when SIM card is inserted after FTU, as we already discussed. We would do the same also for ringtones, wallpaper and predictive text.

Comment 46

[:gerard-majax]

Changing assignee since Carmen is working on some fix for this.

Comment 47

[Carmen Jimenez Cabezas]

Attachment: V1 Proposed patch

Comment 48

[Jose Manuel Cantera]

Comment on attachment 8458216 [details] [review]
V1 Proposed patch

please address some nit comments on GH

Comment 49

[Carmen Jimenez Cabezas]

https://github.com/mozilla-b2g/gaia/commit/300c4b7d3596758f31d8462d47d011e9a579efe7

Comment 50

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/f0a58389d3b5f7dfd9d7d31edaeed2177ca85657