Increase ordering consistency in form_render / drupal_render

Creazion has a good point. Items with the same weight get all mixed up when they go through form_render. Logically items would be sorted by weight, then alphabetically in the same weights. We're using uasort to organize the items by weight, but when things go in with the same weight, they come out in a different order than they went in. The PHP page for usort explains why:

Note: If two members compare as equal, their order in the sorted array is undefined. Up to PHP 4.0.6 the user defined functions would keep the original order for those elements, but with the new sort algorithm introduced with 4.1.0 this is no longer the case as there is no solution to do so in an efficient way.

So basically there's no way to guarantee a consistent ordering unless we sort by more keys. I think sorting by #title would probably be sufficient. I've attached a patch to the 4.7 branch which sorts by weight, then title.

This problem also exists in HEAD, only the function applies to more than just forms. Here's the same patch for common.inc's _element_sort function.

This would break a lot of code which assumes form items will appear in the order that they are set.

Thanks for the response. This is exactly the problem I am looking to solve. The order in which elements are put into the $form array does NOT determine their order for elements of equal weight. Read over the quote from the PHP website in the original post. Items instead come out in a completely unpredictable manner. With the advent of CCK and users frequently generating their own forms, I think that many people will expect that elements sort themselves by their titles as well as weight.

Here's a little more evidence to backup my claim. I've attached a test module for further investigation.

I've defined a simple form for node creation as follows:

  $form = array();
  $form['zappa'] = array (
    '#type' => 'textfield',
    '#title' => 'zappa',
    '#weight' => 0,
  );
  $form['item1'] = array (
    '#type' => 'textfield',
    '#title' => 'item1',
    '#weight' => 0,
  );
  $form['item2'] = array (
    '#type' => 'textfield',
    '#title' => 'item2',
    '#weight' => 0,
  );
  drupal_set_message("".print_r($form,true)."");
  return $form;

The drupal set message simply confirms that this is the order PHP sees the array was created. However, when the actual form is produced instead of coming out in ANY logical way it comes out:

* item2
* zappa
* item1

We've made the assumption that elements come out the same way that they went in, but this is not the case. Because there is no way to tell how the form will be generated on output, we can assume that existing code for creating forms already weight all their fields, unless by chance the 'undefined' sorting manner just so happened to get it right.

This is a change which greatly increases the predictability of generated forms. If needed, I can create a more significant patch which actually returns elements in the same order as they went in, but I believe that alphabetical is a more expected behavior (at least from an administrator's view, maybe not a programmer's) than the defined order.

Bump this back up for another review

Correction to the above post. The forms system seems to be aware of this shortcoming in PHP's uasort function. Intellegently, it assigns all form items that do not include a #weight to some miniscule value. In the above example, when all the items do not have a weight set, they are assigned the following values before they are sorted:

*zappa: 0.006
*item1: 0.007
*item2: 0.008

This in turn makes the form come out the same way it went in. However, if they are all set to 0 by the user, then we get the problem I describe above. I do not think it would be wise to extend this system to defined weights. We could for instance, assign all items with a defined '1' weight values such as 1.006, 1.007, 1.008 to make them come out the same way they go in, but I believe alphabetical will be desired by users who do not define their fields directly in code (i.e. through CCK). In my patch, the alphabetical case only takes effect if items are assigned the same weight, so it doesn't affect any hard-coded forms which do not assign any weights.

Now we're in an ideal situation, users who defined fields via CCK get their fields in weight, title sorted order and developers get their forms ordered the same way as they wrote them (unless they define weights).

the patch still applies with a small offset.
The reasoning makes sense and won't adversely affect any existing code.

Maybe that automatic micro-weight assignment code should be removed then?

I have some problems with this patch and it's performance. Some benchmarks would be helpful. And yes, the auto-weighting can be removed then. But, I will wait for performance numbers (please check node/add/story with path and menu and upload switched on) before saying -1 or +1. If there is a serious performance hit then we will begin thinking on some tricky encoding of titles into weights.

Looks like this needs work until some questions are answered.

>> Re dries: Maybe that automatic micro-weight assignment code should be removed then?

Leave the micro-weight assignment. We still want forms to come out in the same order they went in if no weight is defined. Removing the microweight would require all the existing forms to be re-written with defined weights.

>> Re chx: Benchmarking etc.

Performance impact? For all existing forms this patch has a negligible impact. Look carefully, the comparisions on titles are only done if the weights equal. This patch will only affect the performance of forms where the weights are explicitely set AND multiple items have the same weight. Therefor, this patch does not affect the node/add/story form, or in fact ANY form in core, because none of them explicitly set a weight (and are automatically assigned unique microweights).

Once again, this patch should have little to no effect on any existing form written in code. This patch was written to benifit users who are creating forms through other means, such as webform.module or CCK. End-users will assume that forms sort in this behaviour.

This issue was corrected sometime before the final release of Drupal 5, as ordering now occurs first by weight then by title in CCK, webform, and other places. I couldn't find the code responsible, but it now works consistently and well.