523538 - Don't sync to/from pref if <setting> has no pref

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Ed Lee :Mardak]

Normally settings are bound to a pref, but Weave might re-use the <setting type="string"> for the existing styling but not the pref sync.

If you enter a value (or password) into a <setting> without a "pref" attribute, it'll get stored in the "" pref. And on restart, it'll get loaded from that pref.

Comment 1

[Ed Lee :Mardak]

For now, I'm grabbing each <setting> and setting their "pref" field to {}.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

This just checks that we have a valid "pref" attribute before trying to sync to the pref. Prefs without "name" attributes already don't do anything, so no need to worry about syncing from them.

Mardak, is this sufficient for your needs?

Comment 3

[Ed Lee :Mardak]

Comment on attachment 410841 [details] [diff] [review]
patch

>+      <constructor><![CDATA[
>+        let prefValue = this.hasAttribute("pref") && this.pref.valueFromPreferences;
>         if (prefValue != null) {
let prefValue = this.pref.valueFromPreferences;
if (this.hasAttribute("pref") && prefValue != null) {

Otherwise, it'll end up setting |false| as the value.

Alternatively, my hack-y solution is to replace the field "pref" to {} ;)

<field name="pref">this.hasAttribute("pref") ? .. : {}</field>

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Ok, I guess that does have the advantage of consolidating the check in the base binding. I'll use a getter though, to make it possible to set the pref attribute after binding attachment.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

Comment 6

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/mobile-browser/rev/02ca91675919