This project is archived and is in readonly mode.

#169 ✓resolved
Roman

Element.toQueryString incorrect behavior

Reported by Roman | June 26th, 2008 @ 09:06 PM | in 1.2.1

toQueryString incorrectly ignores empty elements

Example HTML:

<form method="post">
<input name="foo" value="">
<input name="bar" value="test">
</form>

When Submitted:

Current Behavior:

POST DATA IS "bar=test"

Correct Behavior:

POST DATA IS "foo=&bar=test"

How to fix:

Current Function:

toQueryString: function(){
	var queryString = [];
	this.getElements('input, select, textarea').each(function(el){
		if (!el.name || el.disabled) return;
		var value = (el.tagName.toLowerCase() == 'select') ? Element.getSelected(el).map(function(opt){
			return opt.value;
		}) : ((el.type == 'radio' || el.type == 'checkbox') && !el.checked) ? null : el.value;
		$splat(value).each(function(val){
			if (val) queryString.push(el.name + '=' + encodeURIComponent(val));
		});
	});
	return queryString.join('&');
}

Proposed Function:

toQueryString: function(){
	var queryString = [];
	this.getElements('input, select, textarea').each(function(el){
		if (!el.name || el.disabled) return;
		var value = (el.tagName.toLowerCase() == 'select') ? Element.getSelected(el).map(function(opt){
			return opt.value;
		}) : ((el.type == 'radio' || el.type == 'checkbox') && !el.checked) ? null : el.value;
		$splat(value).each(function(val){
			if (val != null) queryString.push(el.name + '=' + encodeURIComponent(val));
		});
	});
	return queryString.join('&');
}

Comments and changes to this ticket

  • Jan Kassens

    Jan Kassens June 26th, 2008 @ 10:36 PM

    • State changed from “new” to “hold”

    imo, an empty string is also data and should be transmitted.

  • 131

    131 June 27th, 2008 @ 12:10 AM

    There's more that an empty string

    Type your phone number here

    How could you "erase" a value if you dont submit it ...

    A lack of something has nothing to do with something that should be erased.

  • Roman

    Roman June 27th, 2008 @ 04:15 AM

    I'm pretty sure that the toQueryString method is supposed to mimic standard form submission.

    If so then it should submit empty values also becuase standard forms do.

    And either im not understanding 131 or he seems confused.

  • digitarald

    digitarald June 27th, 2008 @ 06:27 AM

    • Tag changed from 1.2, 1.2release, core, critical, major to 1.2, 1.2release, core, major

    Jan Kassens: toQueryString IGNORES empty values. Putting that on hold and commenting that it should do what the tickets fixes, confuses me ;)

  • Jan Kassens

    Jan Kassens June 27th, 2008 @ 11:57 AM

    • State changed from “hold” to “open”

    well, i wasn't sure if empty values are intentionally ignored, but it seems no-one has another opinion, setting it to open now.

  • Thomas Aylott

    Thomas Aylott June 27th, 2008 @ 11:57 AM

    • Milestone changed from 2.0 to 1.2.1
    • Assigned user changed from “Valerio” to “Jan Kassens”

    I would have to agree that it should give the same querystring that you'd get if you submitted the form normally. Who wants to accept this patch and write up some specs? Earliest I can get to it would be next week.

    Also, how was this handled back in 1.11? Is this a regression?

  • Roman

    Roman June 27th, 2008 @ 08:14 PM

    In 1.11, a different algorithm was used and it was handled correctly.

  • Bryan J Swift

    Bryan J Swift July 10th, 2008 @ 09:56 PM

    • Tag changed from 1.2, 1.2release, core, major to 1.2, 1.2release, core, major, regression

    I don't remember seeing a fix to this yet so I thought I'd take a stab at it.

    I used typeof instead of val != null but I figure the specs are the more important part of the patch I've attached.

    Cheers.

  • Thomas Aylott

    Thomas Aylott July 10th, 2008 @ 10:26 PM

    • Assigned user changed from “Jan Kassens” to “Thomas Aylott”

    I've been thinking about looking for something to commit. This looks like a good place to start.

    I'm going to plan on getting this in by the end of the day today. IE: Before I stagger off to bed at 4am.

    I lovez me some specs :D

  • Thomas Aylott

    Thomas Aylott July 11th, 2008 @ 08:37 AM

    Looks like there's another issue with this method in IE6

  • Thomas Aylott

    Thomas Aylott July 11th, 2008 @ 11:56 AM

    • State changed from “open” to “resolved”

    fixed in 3f21e1d5f34f6d64693b409a3d5e0c6f0dcef0ec

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Shared Ticket Bins

Attachments

Tags

Referenced by

Pages