Closed Bug 1026381 Opened 6 years ago Closed 6 years ago

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

Categories

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

Other
Other
defect
Not set

Tracking

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.0 S6 (18july)
blocking-b2g 2.0+
Tracking Status
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: rafael.marquez, Assigned: macajc)

References

Details

(Whiteboard: [systemsfe])

Attachments

(3 files, 4 obsolete files)

*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.
blocking-b2g: --- → 2.0?
Whiteboard: [systemsfe]
Peter,

Please weigh in with expected behavior. A product call would be good here.
Flags: needinfo?(pdolanjski)
Blocking.  This is blowing away a user's selected setting.
blocking-b2g: 2.0? → 2.0+
Flags: needinfo?(pdolanjski)
Let's try to do something.
Assignee: nobody → lissyx+mozillians
Target Milestone: --- → 2.0 S5 (4july)
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
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.
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.
Assignee: lissyx+mozillians → nobody
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!
Flags: needinfo?(acperez)
I've seen Alexandre getting crazy this afternoon try to trigger the OV, Carmen is something else we are missing?

Thanks a lot!
Flags: needinfo?(cjc)
(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.
Flags: needinfo?(acperez)
(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.
Oh, and of course, another sign of not working customization was that there was ... no customization: invalid wallpaper, invalid preloaded stuff, etc.
(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?
(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.
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
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
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.
> root@flame:/ # df /persist/svoperapps
> Filesystem               Size     Used     Free   Blksize
> /persist/svoperapps      4.9M     4.1M   832.0K   4096
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.
Attached file operator.log (obsolete) —
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}]
Flags: needinfo?(acperez)
(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.
Flags: needinfo?(acperez)
(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.
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.
Assignee: nobody → lissyx+mozillians
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
Flags: needinfo?(cjc)
Flags: needinfo?(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?
> 
> * 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.
Attached file operator.nok.log (obsolete) —
Gecko/Gaia log when operatorvariant does not start.
Attachment #8446116 - Attachment is obsolete: true
Attached file operator.ok.log (obsolete) —
Gecko/Gaia logs when operatorvariant properly starts.
Attached file operator.nok2.log
Gecko/Gaia logs when not working
Attachment #8446552 - Attachment is obsolete: true
Attached file operator.ok2.log
Gecko/Gaia logs when working
Attachment #8446553 - Attachment is obsolete: true
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.
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 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.
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.
Depends on: 1031164
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 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.
Attachment #8447052 - Attachment description: Link → Link to the github pull request https://github.com/mozilla-b2g/gaia/pull/21099
Attachment #8447052 - Flags: feedback?(cjc)
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.
(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.
Flags: needinfo?(cjc)
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
Flags: needinfo?(cjc)
(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.
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....
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
Attachment #8447052 - Flags: feedback?(cjc) → feedback-
(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.
Flags: needinfo?(amac)
Target Milestone: 2.0 S5 (4july) → 2.0 S6 (18july)
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.
Marce, we either need to fix this bug or back out the NFC single variant change.
What do you suggest?
Flags: needinfo?(marce)
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.
Flags: needinfo?(marce)
Assignee: lissyx+mozillians → carmen.jimenezcabezas
Changing assignee since Carmen is working on some fix for this.
Whiteboard: [systemsfe] → [systemsfe][ETA=7/19]
Attached file V1 Proposed patch
Attachment #8447052 - Attachment is obsolete: true
Attachment #8458216 - Flags: review?(jmcf)
Blocks: 1040375
Comment on attachment 8458216 [details] [review]
V1 Proposed patch

please address some nit comments on GH
Attachment #8458216 - Flags: review?(jmcf) → review+
https://github.com/mozilla-b2g/gaia/commit/300c4b7d3596758f31d8462d47d011e9a579efe7
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.