872983 - event.settingValue in mozSettings.onsettingchange gives wrong value

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[John Hu [:johnhu][:johu][:醬糊小弟]]

While putting an array of object to mozSettings, the event.settingValue of onsettingchange event gives wrong value. It just removes all properties of those objects.

I did a simple test on this issue with the following code:
      if (event.settingName === 'test.key')
      console.log('onsettingchange: ' + JSON.stringify(event.settingValue));
    };
    var cset = [{'a':'b','c':[{'d':'e'}]}];
    console.log('set: ' + JSON.stringify(cset));
    navigator.mozSettings.createLock().set({'test.key': cset});

And the console outputs: 

E/GeckoConsole( 1031): Content JS LOG at app://settings.gaiamobile.org/js/settings.js:115 in settings_init: set: [{"a":"b","c":[{"d":"e"}]}]
E/GeckoConsole( 1031): Content JS LOG at app://settings.gaiamobile.org/js/settings.js:112 in anonymous: onsettingchange: [{}]

But, if we use mozSettings.createLock().get('test.key'), it gives the correct value, [{"a":"b","c":[{"d":"e"}]}].

Comment 1

[John Hu [:johnhu][:johu][:醬糊小弟]]

Sorry, above code is incomplete. 

Please use the following code:

    navigator.mozSettings.onsettingchange = function(event) {
      if (event.settingName === 'test.key')
      console.log('onsettingchange: ' + JSON.stringify(event.settingValue));
    };
    var cset = [{"a":"b","c":[{"d":"e"}]}];
    console.log('set: ' + JSON.stringify(cset));
    navigator.mozSettings.createLock().set({'test.key': cset});

Comment 2

[Gregor Wagner [:gwagner]]

JSON.stringify often lies to you in such cases. Can you try to access/print the "a"property directly?

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

Gregor, 

Sure, here they are:

New version of testing code:
    navigator.mozSettings.onsettingchange = function(event) {
      if (event.settingName === 'test.key') {
        console.log('onsettingchange: a property: ' + event.settingValue[0]['a']);
        console.log('onsettingchange: c property: ' + event.settingValue[0]['c']);
      }
    };
    var cset = [{'a':'b','c':[{'d':'e'}]}];
    console.log('set: ' + JSON.stringify(cset));
    navigator.mozSettings.createLock().set({'test.key': cset});

logcat result:

E/GeckoConsole( 1226): Content JS LOG at app://settings.gaiamobile.org/js/settings.js:117 in settings_init: set: [{"a":"b","c":[{"d":"e"}]}]
E/GeckoConsole( 1226): Content JS LOG at app://settings.gaiamobile.org/js/settings.js:112 in anonymous: onsettingchange: a property: undefined
E/GeckoConsole( 1226): Content JS LOG at app://settings.gaiamobile.org/js/settings.js:113 in anonymous: onsettingchange: c property: undefined

I had tested this in Inari, Unagi, and Leo with b2g18. They all gave me the same result.

Comment 4

[Reuben Morais [:reuben]]

Does it work if you do .get('test.key')?

Comment 5

[John Hu [:johnhu][:johu][:醬糊小弟]]

Reuben, 

Yes. It works if I use get('test.key').

I found this bug because we want to make a memory copy of our settings to prevent multiple querying on mozSettings. Most of them works, but this case, an array of object, doesn't. So, we have to change our code to use get('xxxxxxx');

Comment 6

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

The improvement John described was made in bug 840322. It looks like the settings cache fails in this case.

Comment 7

[Gregor Wagner [:gwagner]]

Attachment: patch

Another expose issue...
Thanks for finding!

Comment 8

[Gregor Wagner [:gwagner]]

John, can you try this patch?

Comment 9

[John Hu [:johnhu][:johu][:醬糊小弟]]

Gregor, 

Sorry, it doesn't works. It gives another error about this patch. I am not sure if this is my problem on Gecko version.

E/GeckoConsole(  641): [JavaScript Error: "this._wrap is not a function" {file: "jar:file:///system/b2g/omni.ja!/components/SettingsManager.js" line: 318}]

my gecko version: dd60c168dc409bc44196d86907dcfef84c7367fb

Comment 10

[Gregor Wagner [:gwagner]]

(In reply to John Hu [:johnhu] from comment #9)
> Gregor, 
> 
> Sorry, it doesn't works. It gives another error about this patch. I am not
> sure if this is my problem on Gecko version.
> 
> E/GeckoConsole(  641): [JavaScript Error: "this._wrap is not a function"
> {file: "jar:file:///system/b2g/omni.ja!/components/SettingsManager.js" line:
> 318}]
> 
> my gecko version: dd60c168dc409bc44196d86907dcfef84c7367fb

You need the patch from bug 849616 as well.
Don't we all love this branching mess.

Comment 11

[John Hu [:johnhu][:johu][:醬糊小弟]]

Gregor, 

Great, it works now, see:

E/GeckoConsole(  640): Content JS LOG at app://settings.gaiamobile.org/js/settings.js:112 in anonymous: onsettingchange: [{"a":"b","c":[{"d":"e"}]}]
E/GeckoConsole(  640): Content JS LOG at app://settings.gaiamobile.org/js/settings.js:113 in anonymous: onsettingchange: a property: b
E/GeckoConsole(  640): Content JS LOG at app://settings.gaiamobile.org/js/settings.js:114 in anonymous: onsettingchange: c property: [object Object]

Comment 12

[Alex Keybl [:akeybl]]

Blocking based on developer impact and the fact that we're close to a fix.

Comment 13

[Reuben Morais [:reuben]]

Note that the patch in bug 849616 needs to be uplifted first.

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/projects/birch/rev/603891a7c4b1

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/603891a7c4b1

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/dfb4cc22e0e7

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/dfb4cc22e0e7