don't set pref values that aren't of type String, Number, or Boolean

Preferences.js

@@ -96,27 +96,37 @@ Preferences.prototype = {
       return;
     }
 
-    switch (typeof prefValue) {
-      case "number":
+    let prefType;
+    if (typeof prefValue != "undefined" && prefValue != null)
+      prefType = prefValue.constructor.name;
+
+    switch (prefType) {
+      case "String":
+        {
+          let string = Cc["@mozilla.org/supports-string;1"].
+                       createInstance(Ci.nsISupportsString);
+          string.data = prefValue;
+          this._prefSvc.setComplexValue(prefName, Ci.nsISupportsString, string);
+        }
+        break;
+
+      case "Number":
         this._prefSvc.setIntPref(prefName, prefValue);
         if (prefValue % 1 != 0)
-          Cu.reportError("WARNING: setting " + prefName + " pref to non-integer number " +
-                         prefValue + " converts it to integer number " + this.get(prefName) +
-                         "; to retain precision, store non-integer numbers as strings");
+          Cu.reportError("Warning: setting the " + prefName + " pref to the " +
+                         "non-integer number " + prefValue + " converted it " +
+                         "to the integer number " + this.get(prefName) +
+                         "; to retain fractional precision, store non-integer " +
+                         "numbers as strings.");
         break;
 
-      case "boolean":
+      case "Boolean":
         this._prefSvc.setBoolPref(prefName, prefValue);
         break;
 
-      case "string":
-      default: {
-        let string = Cc["@mozilla.org/supports-string;1"].
-                     createInstance(Ci.nsISupportsString);
-        string.data = prefValue;
-        this._prefSvc.setComplexValue(prefName, Ci.nsISupportsString, string);
-        break;
-      }
+      default:
+        throw "can't set pref " + prefName + " to value '" + prefValue +
+              "'; it isn't a String, Number, or Boolean";
     }
   },
 
@@ -186,13 +196,17 @@ Preferences.prototype = {
 Preferences.__proto__ = Preferences.prototype;
 
 function isArray(val) {
-  // We can't check for |val.constructor == Array| here, since we might have
-  // a different global object, so we check the constructor name instead.
-  return (typeof val == "object" && val.constructor.name == Array.name);
+  // We can't check for |val.constructor == Array| here, since the value
+  // might be from a different context whose Array constructor is not the same
+  // as ours, so instead we match based on the name of the constructor.
+  return (typeof val != "undefined" && val != null && typeof val == "object" &&
+          val.constructor.name == "Array");
 }
 
 function isObject(val) {
-  // We can't check for |val.constructor == Object| here, since we might have
-  // a different global object, so we check the constructor name instead.
-  return (typeof val == "object" && val.constructor.name == Object.name);
+  // We can't check for |val.constructor == Object| here, since the value
+  // might be from a different context whose Object constructor is not the same
+  // as ours, so instead we match based on the name of the constructor.
+  return (typeof val != "undefined" && val != null && typeof val == "object" &&
+          val.constructor.name == "Object");
 }

test/unit/test_Preferences.js

@@ -39,6 +39,33 @@ function test_set_get_unicode_pref() {
   Preferences.reset("test_set_get_unicode_pref");
 }
 
+function test_set_null_pref() {
+  try {
+    Preferences.set("test_set_null_pref", null);
+    // We expect this to throw, so the test is designed to fail if it doesn't.
+    do_check_true(false);
+  }
+  catch(ex) {}
+}
+
+function test_set_undefined_pref() {
+  try {
+    Preferences.set("test_set_undefined_pref");
+    // We expect this to throw, so the test is designed to fail if it doesn't.
+    do_check_true(false);
+  }
+  catch(ex) {}
+}
+
+function test_set_unsupported_pref() {
+  try {
+    Preferences.set("test_set_unsupported_pref", new Array());
+    // We expect this to throw, so the test is designed to fail if it doesn't.
+    do_check_true(false);
+  }
+  catch(ex) {}
+}
+
 // Make sure that we can get a string pref that we didn't set ourselves
 // (i.e. that the way we get a string pref using getComplexValue doesn't
 // hork us getting a string pref that wasn't set using setComplexValue).