Implemented prefs module properly, data is actually being persisted now

Implemented prefs module properly, data is actually being persisted now

[Thomas Greiner, 2013-07-25 13:20:36]

LGTM with the one change

http://codereview.adblockplus.org/11159032/diff/1/lib/prefs.js
File lib/prefs.js (right):

http://codereview.adblockplus.org/11159032/diff/1/lib/prefs.js#newcode60
lib/prefs.js:60: throw new Error("Attempt to change preference type");
I'd also include a check for the existence of the key before that because even
though this check catches it anyway, "Attempt to change preference type" is not
the best response for that case.

e.g.
if (!(key in defaults))
  throw new Error("Attempt to set unknown preference");

Alternatively, you could simply change the error message to something like
"Attempt to set unknown preference or change type of an existing preference" but
personally, I'm not a fan of that.

[Wladimir Palant, 2013-07-26 09:44:02]

http://codereview.adblockplus.org/11159032/diff/1/lib/prefs.js
File lib/prefs.js (right):

http://codereview.adblockplus.org/11159032/diff/1/lib/prefs.js#newcode60
lib/prefs.js:60: throw new Error("Attempt to change preference type");
On 2013/07/25 13:20:36, Thomas Greiner wrote:
> I'd also include a check for the existence of the key

The property handler is only added for properties that exist in the defaults -
so we cannot possibly get here if there is no default.

I don't think we can catch an attempt to set a random property without some
major effort.

[Thomas Greiner, 2013-11-26 11:08:28]

I see your point
LGTM