I'm still kind of new to Drupal but I ran into a problem using theme_table() and finally tracked it down to tablesort_header(). Basically the problem is the same as the one mentioned here.

In short the $cell['data'] is being used in place of the $cell['field'] in the table header so the link is created with &order = $cell['data'] which seems like the wrong variable according to the documentation. It seems wrong since the $cell['field'] is never used. Maybe this is by design but the documentation should be updated. It's also possible that its done this way to hide the actual names of your table fields. But then why even have a 'field' parameter at all?

If this is a bug it sounds like its been around since 4.6x. I've checked the code for 6.8 and 6.9 and it appears in both.

Line 91 tablesort.inc

$cell['data'] = l($cell['data'] . $image, $_GET['q'], array('attributes' => array('title' => $title), 'query' => 'sort='. $ts['sort'] .'&order='. urlencode($cell['data']) . $ts['query_string'], 'html' => TRUE));

Should be:

$cell['data'] = l($cell['data'] . $image, $_GET['q'], array('attributes' => array('title' => $title), 'query' => 'sort='. $ts['sort'] .'&order='. urlencode($cell['field']) . $ts['query_string'], 'html' => TRUE));

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

thaichaiguy’s picture

Component: theme system » base system
mdupont’s picture

Version: 6.8 » 6.10
danielnolde’s picture

Version: 6.10 » 6.11

I just run accross the same bug, according to Drupal API Docs this still is buggy and NOT fixed for D5, D6 *and* D7. This is really a bug and *not* in line with the specification of table header parameter usage, see
http://api.drupal.org/api/function/theme_table

The plain trivial proposed here is correct, applied in seconds and works for for all versions.

Please commit/include.

brianV’s picture

Version: 6.11 » 7.x-dev
Status: Active » Needs review
FileSize
1.07 KB
1.01 KB

Rolled this into patches for D7 & D6.

Dries’s picture

Spacing is not conform our coding style.

It would be awesome if we could have a test for this.

Heine’s picture

Huh?

From http://api.drupal.org/api/function/tablesort_get_order/6

  $order = isset($_GET['order']) ? $_GET['order'] : '';
  foreach ($headers as $header) {
    if (isset($header['data']) && $order == $header['data']) {
      return array('name' => $header['data'], 'sql' => isset($header['field']) ? $header['field'] : '');
    }

What is the bug here?

Status: Needs review » Needs work

The last submitted patch failed testing.

Heine’s picture

Status: Needs work » Closed (works as designed)
timtoon’s picture

Status: Closed (works as designed) » Needs review

#4: 363292-fix-tablesort-inc.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, 363292-fix-tablesort-inc.patch, failed testing.