Simply to fix.

CommentFileSizeAuthor
#8 theme_inc_tbody_DRUPAL-5-dev.patch1.48 KBm1mic
#5 theme_inc_head_tbody_3.patch1.5 KBdvessel
#4 theme_inc_head_tbody2.patch1.48 KBm1mic
#2 theme_inc_head_tbody.patch1.48 KBm1mic
#1 theme_inc_tbody.patch1.45 KBm1mic
empty_tbody.patch842 byteschx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

m1mic’s picture

m1mic CreditAttribution: m1mic commented
FileSize
theme_inc_tbody.patch1.45 KB

@chx

Tested with:

  • Drupal-5
  • Apache 2.2.4
  • PHP 5.2.4

Your patch worked to remove the empty tbody as advertised. However, if there are no rows in the table (in Calendar module's Prev/Next bar for example), validation still fails. This is because HTML requires that anything that has a thead tags must also have tbody tags. I updated your patch to include a ternary operator to check if there are rows, and if so, add the thead tags.

m1mic’s picture

m1mic CreditAttribution: m1mic commented
Version: 5.x-dev » 6.x-dev
FileSize
theme_inc_head_tbody.patch1.48 KB

Looks like the same issue is in Head. Updating the version number to 6.x-dev and adding a patch for Head.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Needs work

- We don't use /* and */ multiline comments inside code.
- We do use spaces around ? in ternary operators.

Apart from this, this patch obviously only needs some testing, it looks good from here.

m1mic’s picture

m1mic CreditAttribution: m1mic commented
Status: Needs work » Needs review
FileSize
theme_inc_head_tbody2.patch1.48 KB

Thanks for the review Gábor! Converted mulitline comments into single line // comments. Added the proper spacing to the ternary operator.

dvessel’s picture

dvessel CreditAttribution: dvessel commented
Status: Needs review » Reviewed & tested by the community
FileSize
theme_inc_head_tbody_3.patch1.5 KB

ejhildreth, you have to diff from the root install. Here's a proper patch.

The closing /tbody was misplaced so that was corrected. Everything else is pretty much the same from your previous patch.

Tested and validates.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)
m1mic’s picture

m1mic CreditAttribution: m1mic commented
Version: 6.x-dev » 5.x-dev
Status: Closed (fixed) » Needs review
FileSize
theme_inc_tbody_DRUPAL-5-dev.patch1.48 KB

Backporting: This bug is still in DRUPAL-5-dev

m1mic’s picture

m1mic CreditAttribution: m1mic commented
Title: empty tbody fails validation » D6 Backport: empty tbody fails validation
drumm’s picture

drumm
he/him
Location NY, US
CreditAttribution: drumm commented
Status: Needs review » Fixed

Committed to 5.x.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.