Merge pull request prototypejs#66 from rydenius/1384-form-serialize-fix

Fix regression in `Form.serialize` when handling `<select multiple>` elements. [#1384 state:resolved]
victor-homyakov · Sep 27, 2012 · d017062 · d017062
2 parents fccc5c3 + a03039d
commit d017062
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 13 deletions.
diff --git a/src/prototype/dom/form.js b/src/prototype/dom/form.js
@@ -113,22 +113,27 @@ var Form = {
       accumulator = function(result, key, value) {
         if (key in result) {
           if (!Object.isArray(result[key])) result[key] = [result[key]];
-          result[key].push(value);
+          result[key] = result[key].concat(value);
         } else result[key] = value;
         return result;
       };
     } else {
       initial = '';
-      accumulator = function(result, key, value) {
-        // Normalize newlines as \r\n because the HTML spec says newlines should
-        // be encoded as CRLFs.
-        value = value.gsub(/(\r)?\n/, '\r\n');
-        value = encodeURIComponent(value);
-        // Likewise, according to the spec, spaces should be '+' rather than
-        // '%20'.
-        value = value.gsub(/%20/, '+');
-        return result + (result ? '&' : '') + encodeURIComponent(key) + '=' + value;
-      }
+      accumulator = function(result, key, values) {
+        if (!Object.isArray(values)) {values = [values];}
+        if (!values.length) {return result;}
+        // According to the spec, spaces should be '+' rather than '%20'.
+        var encodedKey = encodeURIComponent(key).gsub(/%20/, '+');
+        return result + (result ? "&" : "") + values.map(function (value) {
+          // Normalize newlines as \r\n because the HTML spec says newlines should
+          // be encoded as CRLFs.
+          value = value.gsub(/(\r)?\n/, '\r\n');
+          value = encodeURIComponent(value);
+          // According to the spec, spaces should be '+' rather than '%20'.
+          value = value.gsub(/%20/, '+');
+          return encodedKey + "=" + value;
+        }).join("&");
+      };
     }
 
     return elements.inject(initial, function(result, element) {

diff --git a/test/unit/fixtures/form.html b/test/unit/fixtures/form.html
@@ -118,10 +118,29 @@
 </form>
 
 <form id="form_with_inputs_needing_encoding" style="display:none">
-  <input type="hidden" name="user[wristbands][][nickname]" id="fine_1" value="Hässlich" />
+  <input type="hidden" name="user[wristbands][ ][nickname]" id="fine_1" value="Hässlich" />
 </form>
 
 <form id="form_with_troublesome_input_names">
   <input type="text" name="length" value="foo" />
   <input type="text" name="bar"    value="baz" />
 </form>
+
+<form id="form_with_multiple_select">
+  <input type="text" name="peewee" value="herman" />
+  <input type="hidden" name="colors" value="pink" />
+  <select name="colors" multiple="multiple">
+    <option value="blue" selected>blue</option>
+    <option value="red">red</option>
+    <option value="green">green</option>
+    <option value="yellow" selected>yellow</option>
+    <option value="not grey" selected>grey</option>
+  </select>
+  <select name="number">
+    <option value="0">0</option>
+    <option value="1">1</option>
+    <option value="2" selected>2</option>
+    <option value="3">3</option>
+  </select>
+  <input type="submit" />
+</form>
diff --git a/test/unit/form_test.js b/test/unit/form_test.js
@@ -301,7 +301,7 @@ new Test.Unit.Runner({
   },
 
   testFormSerializeURIEncodesInputs: function() {
-    this.assertEqual("user%5Bwristbands%5D%5B%5D%5Bnickname%5D=H%C3%A4sslich", $('form_with_inputs_needing_encoding').serialize(false));
+    this.assertEqual("user%5Bwristbands%5D%5B+%5D%5Bnickname%5D=H%C3%A4sslich", $('form_with_inputs_needing_encoding').serialize(false));
   },
 
   testFormMethodsOnExtendedElements: function() {
@@ -325,6 +325,17 @@ new Test.Unit.Runner({
     this.assert(!select.anInputMethod);      
     this.assertEqual('select', select.aSelectMethod());
   },
+
+  testFormSerializeMultipleSelect: function () {
+    var form = $("form_with_multiple_select");
+    this.assertEqual("peewee=herman&colors=pink&colors=blue&colors=yellow&colors=not+grey&number=2", form.serialize(false));
+    var hash = {
+      peewee: 'herman',
+      colors: ['pink', 'blue', 'yellow', 'not grey'],
+      number: '2'
+    };
+    this.assertHashEqual(hash, form.serialize(true));
+},
 
   testFormRequest: function() {
     var request = $("form").request();