Skip to content

Commit

Permalink
Sortable: Return an empty URL param for empty sortable lists. Fixed #…
Browse files Browse the repository at this point in the history 
…5794 - sortable("serialize", {key: "foo[]"}) returns an empty string for an empty list
  • Loading branch information
@holger @scottgonzalez
holger authored and scottgonzalez committed Jul 16, 2010
1 parent 325a262 commit 9d01ab5
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions ui/jquery.ui.sortable.js
View file
Expand Up @@ -409,6 +409,10 @@ $.widget("ui.sortable", $.ui.mouse, {
if(res) str.push((o.key || res[1]+'[]')+'='+(o.key && o.expression ? res[1] : res[2]));
});

if(!str.length && o.key) {
str.push(o.key + '=');
}

return str.join('&');

},
Expand Down

5 comments on commit 9d01ab5

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@holger Can you please sign our CLA?

@holger
Copy link
Contributor Author

@holger holger commented on 9d01ab5 Dec 12, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@janekh
Copy link

@janekh janekh commented on 9d01ab5 Jun 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's been several years but I just stumbled upon this when trying to use custom "key" with serialize method.

What was the actual reason this change was made? I would add couple of counter arguments why this change shouldn't have been made and should maybe even be reversed.

  1. There is now a difference in how ("serialize") and ("serialize", {key:"foo[]"}) work. There is no reason why those two should work differently. Either both should return "foo[]=" for empty lists or both should return empty string.

  2. When submitting HTML form with un-checked checkboxes they also do not generate empty values and have to be handled on server side. Just as submitting empty sortable list should be handled on server side. https://api.jquery.com/serialize/ doesn't include un-checked checkboxes in the resulting string. This is another difference in the JQuery universe.

  3. Most of the server side languages translate foo[]= as non empty array. Which is wrong considering that the sortable list was actually empty.

In case this is not going to be reversed, please update sortable documentation to reflect this anomaly and save the next person a lot of time trying to figure out what is going on.

@holger @scottgonzalez

@scottgonzalez
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, any change is a breaking change for code written in the past seven years. However, this plugin is scheduled for a complete rewrite, at which point every part of the API will be reviewed.

Please sign in to comment.