prototype: Fix double escaping of query parameters in Hash.prototype.toQueryString, and prevent Safari from iterating over shadowed properties when creating hashes. Closes #7421.

CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Fix double escaping of query parameters in Hash.prototype.toQueryString, and prevent Safari from iterating over shadowed properties when creating hashes.  Closes #7421.  [Tobie Langel, Mislav Marohnić]
+
 * Fix simulated attribute reading for IE for "href", "src" and boolean attributes.  [Mislav Marohnić, Thomas Fuchs]
 
 * Form.Element.disable() will now call blur(), removed blur() call from Form.Element.enable().  Closes #6034.  [tdd]

src/hash.js

@@ -1,34 +1,36 @@
-var Hash = function(obj) {
-  Object.extend(this, obj || {});
+var Hash = function(object) {
+  if (object instanceof Hash) this.merge(object);
+  else Object.extend(this, object || {});
 };
 
 Object.extend(Hash, {
   toQueryString: function(obj) {
     var parts = [];
+    parts.add = arguments.callee.addPair;
     
-	  this.prototype._each.call(obj, function(pair) {
+    this.prototype._each.call(obj, function(pair) {
       if (!pair.key) return;
+      var value = pair.value;
       
-      if (pair.value && pair.value.constructor == Array) {
-        var values = pair.value.compact();
-        if (values.length < 2) pair.value = values.reduce();
-        else {
-        	key = encodeURIComponent(pair.key);
-          values.each(function(value) {
-            value = value != undefined ? encodeURIComponent(value) : '';
-            parts.push(key + '=' + encodeURIComponent(value));
-          });
-          return;
-        }
+      if (value && typeof value == 'object') {
+        if (value.constructor == Array) value.each(function(value) {
+          parts.add(pair.key, value);
+        });
+        return;
       }
-      if (pair.value == undefined) pair[1] = '';
-      parts.push(pair.map(encodeURIComponent).join('='));
-	  });
+      parts.add(pair.key, value);
+    });
     
     return parts.join('&');
   }
 });
 
+Hash.toQueryString.addPair = function(key, value, prefix) {
+  if (value == null) return;
+  key = encodeURIComponent(key);
+  this.push(key + '=' + (value == null ? '' : encodeURIComponent(value)));
+}
+
 Object.extend(Hash.prototype, Enumerable);
 Object.extend(Hash.prototype, {
   _each: function(iterator) {
@@ -86,6 +88,25 @@ Object.extend(Hash.prototype, {
 });
 
 function $H(object) {
-  if (object && object.constructor == Hash) return object;
+  if (object instanceof Hash) return object;
   return new Hash(object);
 };
+
+// Safari iterates over shadowed properties
+if (function() {
+  var i = 0, Test = function(value) { this.key = value };
+  Test.prototype.key = 'foo';
+  for (var property in new Test('bar')) i++;
+  return i > 1;
+}()) Hash.prototype._each = function(iterator) {
+  var cache = [];
+  for (var key in this) {
+    var value = this[key];
+    if ((value && value == Hash.prototype[key]) || cache.include(key)) continue;
+    cache.push(key);
+    var pair = [key, value];
+    pair.key = key;
+    pair.value = value;
+    iterator(pair);
+  }
+};

test/unit/hash.html

@@ -46,6 +46,7 @@
     multiple_nil:     { color: ['r', null, 'g', undefined, 0] },
     multiple_all_nil: { color: [null, undefined] },
     multiple_empty:   { color: [] },
+    multiple_special: { 'stuff[]': $w('$ a ;') },
 
     value_undefined:  { a:"b", c:undefined },
     value_null:       { a:"b", c:null },
@@ -63,6 +64,16 @@
   
   new Test.Unit.Runner({
     
+    testConstruct: function(){ with(this) {
+      var h = $H(Fixtures.one)
+      assertNotIdentical(Fixtures.one, h)
+      assertIdentical(h, $H(h))
+
+      var h2 = new Hash(h)
+      assertNotIdentical(h, h2)
+      assertEqual(h.inspect(), h2.inspect())
+    }},
+    
     testKeys: function(){ with(this) {
       assertEnumEqual([],               $H({}).keys());
       assertEnumEqual(['a'],            $H(Fixtures.one).keys());
@@ -96,15 +107,19 @@
       assertEqual('',                        $H({}).toQueryString());
       assertEqual('a=A%23',                  $H(Fixtures.one).toQueryString());
       assertEqual('a=A&b=B&c=C&d=D%23',      $H(Fixtures.many).toQueryString());
-      assertEqual("a=b&c=",                  $H(Fixtures.value_undefined).toQueryString());
-      assertEqual("a=b&c=",                  $H(Fixtures.value_null).toQueryString());
+      assertEqual("a=b",                     $H(Fixtures.value_undefined).toQueryString());
+      assertEqual("a=b",                     $H(Fixtures.value_null).toQueryString());
       assertEqual("a=b&c=0",                 $H(Fixtures.value_zero).toQueryString());
       assertEqual("color=r&color=g&color=b", $H(Fixtures.multiple).toQueryString());
       assertEqual("color=r&color=g&color=0", $H(Fixtures.multiple_nil).toQueryString());
-      assertEqual("color=",                  $H(Fixtures.multiple_all_nil).toQueryString());
-      assertEqual("color=",                  $H(Fixtures.multiple_empty).toQueryString());
-      
-      assertEqual("_each=E&map=M&keys=K&values=V&collect=C&inject=I", Hash.toQueryString(Fixtures.dangerous));
+      assertEqual("",                        $H(Fixtures.multiple_all_nil).toQueryString());
+      assertEqual("",                        $H(Fixtures.multiple_empty).toQueryString());
+      assertEqual("stuff%5B%5D=%24&stuff%5B%5D=a&stuff%5B%5D=%3B", $H(Fixtures.multiple_special).toQueryString());
+
+      assertEnumEqual($w("_each=E map=M keys=K values=V collect=C inject=I").sort(),
+        Hash.toQueryString(Fixtures.dangerous).split('&').sort());
+      assertEnumEqual($w('_each=E map=M keys=K values=V collect=C inject=I').sort(),
+        $H(Fixtures.dangerous).toQueryString().split('&').sort());
     }},
     
     testInspect: function(){ with(this) {