There is a problem in the current CVS with table colors on CSS1 browsers (Internet Explorer, probably others). Drupal.css contains rules like this:
tr td.watchdog-httpd.active {
background: #cec;
}

The double class selector is CSS2. IE mistakenly sees it as:
tr td.active {
background: #cec;
}

This causes every active column to have the last watchdog color specified (red for errors).

To fix this, we need to avoid the double class selector. We can apply the "watchdog-type" classes on the <tr> rather than on each <td>. Theme_table()'s argument structure did not accomodate for <tr> attributes though.

Before:
* @param $rows
* An array of arrays containing the table cells. Each cell can be either a
* string or and associative array with the following keys:
* - "data": The string to display in the table cell.
* - Any HTML attributes, such as "colspan", to apply to the table cell.

After:
* @param $rows
* An array of table rows. Every row is an array of cells and attributes.
* Array items with string keys are treated as HTML attributes to be applied
* to the table row. Items with numerical keys are treated as cells.
*
* Each cell can be either a string or an associative array with the following keys:
* - "data": The string to display in the table cell.
* - Any HTML attributes, such as "colspan", to apply to the table cell.
*
* Here's an example for $rows:
* @verbatim
* $rows = array(
* array(
* 'Cell 1', array('data' => 'Cell 2', 'colspan' => 2), 'style' => 'color: red;'
* )
* array(
* 'Cell 1', 'Cell 2', 'Cell 3', 'id' => 'row2'
* )
* );

I avoided using the array('data' => ..., 'attribute' => ..., 'attribute' => ...) structure for rows (like we have for cells) because it would make the array even more multidimensional and would require recursive dimension checks to be backwards compatible.

The attached patch includes this change, as well as a change to watchdog.module and drupal.css to fix the actual bug. I also tweaked the CSS selectors for active/odd/even/watchdog so they are less specific (and thus easier to override by themes).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Steven’s picture

FileSize
5.81 KB

Ack, I forgot about drupal_attributes(). Attached is a slightly cleaner patch.

Steven’s picture

FileSize
5.74 KB

Dries didn't like my approach, so here's one which uses the array('data' => ...) approach. In theory there can be a conflict when a cell is specified with 'data' for its key, but in practice all cells are created using $row[] or array('value'), which means they have numerical keys.

* @param $rows
*   An array of table rows. Every row is an array of cells, or an associative
*   array with the following keys:
*   - "data": an array of cells
*   - Any HTML attributes, such as "class", to apply to the table row.
*
*   Each cell can be either a string or an associative array with the following keys:
*   - "data": The string to display in the table cell.
*   - Any HTML attributes, such as "colspan", to apply to the table cell.
*
*   Here's an example for $rows:
*   @verbatim
*   $rows = array(
*     // Simple row
*     array(
*       'Cell 1', 'Cell 2', 'Cell 3'
*     ),
*     // Row with attributes on the row and some of its cells.
*     array(
*       'data' => array('Cell 1', array('data' => 'Cell 2', 'colspan' => 2)), 'class' => 'funky'
*     )
*   );
*   @endverbatim
*

This now means that $rows can be 4 dimensional:
- a dimension for the different rows
- a dimension for row attributes and a data field containing:
- a dimension for the different cells
- a dimension for cell attributes and a data field containing a string.
In the simplest case, $rows is 2 dimensional.

It sounds icky, but if you look at the watchdog.module patch, it's not too bad in practice with some nice indentation.

Steven’s picture

This patch was applied a couple of days ago.

Anonymous’s picture