The method used to sort fields in core has a major bug IMHO, it's a usort(), also all key sort methods in php have it too.

for example:

$numbers = array('one' => 1, 'two' => 2, 'four' => 4, 'three' => 3, 'five' => 5, 'second one' => 1, 'third one' => 1, 'six' => 6);

uasort($numbers, 'element_sort');
print_r($numbers);

function element_sort($a, $b) {
  if ($a == $b) {
    return 0;
  }
  return ($a < $b) ? -1 : 1;
}

The previous code will print:

Array ( [third one] => 1 [second one] => 1 [one] => 1 [two] => 2 [three] => 3 [four] => 4 [five] => 5 [six] => 6 )

Strange! at least for me, sort method should respect the order precedence if two or more items has the same sorting value. The correct behavior should print:

Array ( [one] => 1 [second one] => 1 [third one] => 1 [two] => 2 [three] => 3 [four] => 4 [five] => 5 [six] => 6 )

The previous method uasort() is used in Drupal (common.inc) to sort Fields based on their weights.

Steps to reproduce:
1- Enable Profile module
2- Add two fields to profile (e.g. First Name, Last Name) with defaults value don't touch weight for now.
3- In admin preview page for profile, fields appear exactly as you created them First name the Last name.
4- Fill these fields in your profile edit page.
5- View your profile, you'll get a reverse order Last name then First name.

I can tell that this problem only occurs in profile, in content types, new field already assigned a new different weight when created.

I've fixed this problem by replacing the sort method with quick sort method that respect the keys.

CommentFileSizeAuthor
#1 fields_weights.patch1.75 KBgood_man
fields_weights.patch1.75 KBgood_man
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

good_man’s picture

Status: Active » Needs review
FileSize
1.75 KB

For TB

Status: Needs review » Needs work

The last submitted patch, fields_weights.patch, failed testing.

good_man’s picture

Status: Needs work » Needs review

I'm not so good with TB yet, anyone can tell me what's wrong?

good_man’s picture

Status: Needs review » Needs work

Well I had a talk with chx today, after benchmarking the new method, it's 3 times slower than the old one. I'm looking for a quicker stable sort method, maybe TimSort that implemented in Python.

If you have further suggestions please add it below.

p.s. the previous patch has also an error, array_merge in php renumber numeric keys in array so better using + instead of array_merge.