Properly save modified objects in Prefs

lib/prefs.js

Index: lib/prefs.js
===================================================================
--- a/lib/prefs.js
+++ b/lib/prefs.js
@@ -39,22 +39,34 @@
 
 let listeners = [];
 
+let cachedObjects = {};

[Wladimir Palant, 2013/07/24 08:49:40]

Our property getter and setter are closures, we can use a local variable to
cache their value - no need for a global one for all prefs.

+
 function defineProperty(key)
 {
   Prefs.__defineGetter__(key, function()
   {
+    if (key in cachedObjects)
+      return cachedObjects[key];
+
+    var value = null;
     if (key in localStorage)
     {
       try
       {
-        return JSON.parse(localStorage[key]);
+        value = JSON.parse(localStorage[key]);
       }
       catch(e)
       {
         Cu.reportError(e);
       }
     }
-    return defaults[key];
+    value = value || defaults[key];

[Wladimir Palant, 2013/07/24 08:49:40]

What if value is false?

+
+    if (typeof value !== "object")

[Wladimir Palant, 2013/07/24 08:49:40]

I don't really like the special case for objects here - if we cache preferences
then we should cache all of them.

+      return value;
+
+    cachedObjects[key] = JSON.parse(JSON.stringify(value));

[Wladimir Palant, 2013/07/24 08:49:40]

This is wasteful - we only need the parse/stringify dance this if we take a
value from defaults, not if we already parsed it from localStorage.

+    return cachedObjects[key];
   });
   Prefs.__defineSetter__(key, function(value)
   {
@@ -66,6 +78,10 @@
       localStorage[key] = stringified;
     else
       delete localStorage[key];
+
+    if (key in cachedObjects)
+      delete cachedObjects[key];
+
     for each (let listener in listeners)
       listener(key);
   });