Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #7516 (closed defect: fixed)

Opened 2 years ago

Last modified 2 years ago

[PATCH] Form.serialize not serializing empty array inputs

Reported by: stakadush Assigned to: mislav
Priority: normal Milestone: 1.x
Component: Prototype Version: edge
Severity: critical Keywords: ready
Cc:

Description

hello this patch fixes a bug in prototype which makes Form.serialize() produce single value inputs where it should produce an array.

run the following html code to see what i mean:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">

<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<title>Form.serialize() Example</title>
<script type="text/javascript" src="js/prototype.js"></script>

<script type="text/javascript">
Event.observe(window, 'load', function() {
	console.log(Form.serialize('form'));
});
</script>

</head>
<body>

<form id="form">
	<input type="text" name="name[]" value="" />
	<input type="text" name="name[]" value="" />
	<input type="text" name="address" value="" />
	<input type="text" name="phone" value="1234" />
</form>

</body>
</html>

what it prints is: name=&address=&phone=1234 what it should print is something like: name[]=&name[]=&address=&phone=1234

the attached patch fixes the problem which was basically changing the following code in serializeElements if (result[key]) to if (key in result)

Attachments

serialize.diff (450 bytes) - added by stakadush on 02/09/07 03:42:21.

Change History

02/09/07 03:42:21 changed by stakadush

  • attachment serialize.diff added.

(follow-up: ↓ 2 ) 02/12/07 13:53:12 changed by madrobby

Does this happen with latest trunk? If so, can you please provide a proper test for this? (See http://prototypejs.org/contribute for more on testing)

Thanks for reporting.

(in reply to: ↑ 1 ; follow-up: ↓ 3 ) 02/13/07 05:02:23 changed by stakadush

Replying to madrobby:

Does this happen with latest trunk? If so, can you please provide a proper test for this? (See http://prototypejs.org/contribute for more on testing) Thanks for reporting.

the diff i posted was for the latest trunk (1.5.1_pre0)... take a look in form.js line #12 (in serializeForm function)

no test is needed... my correction is a clear bug... if result[key] is an empty string, it will always evaluate as false when doing if (result[key]), which will cause the process failing (check the example i posted).

to do what you want in serializeForm function need to do if (key in result) to check for the existence of the key in result object, and not the value of it... so, no test necessary :)

thanks...

(in reply to: ↑ 2 ) 02/19/07 22:49:05 changed by madrobby

Replying to stakadush:

no test is needed... my correction is a clear bug... if result[key] is an empty string, it will always evaluate as false when doing if (result[key]), which will cause the process failing (check the example i posted).

Certainly there's need for a proper unit test for this, so we don't have breakage in the future if something changes. Note that the inline HTML you provided in the description could easily be made into the form unit tests... :)

03/04/07 17:45:25 changed by mislav

  • keywords set to ready.
  • owner changed from sam to mislav.
  • status changed from new to assigned.

[6309] from the form branch fixes this (test included), ready to be merged back to trunk

03/04/07 18:13:30 changed by mislav

update: [6309] should be merged together with [6312]

03/04/07 21:54:22 changed by madrobby

  • status changed from assigned to closed.
  • resolution set to fixed.

Fixed in [6322].