system.theme.css adds border-top to all tables

This is a result of: http://drupal.org/node/79346

Subscribing. This was a really annoying one.

This one rule, (system.css: line 29 introduced by http://drupal.org/node/79346 as webernet says) is unnecessary for table with a header, as thead h { has border-bottom property.

As tables can be used for layout (usually without a header), in which case we should leave it to the author to theme, and this rule has no effect when used to present a table such as admin/reports/dblog, I vote the rule should go.

Are there (still) any tables which would be affected by this (ie. without a thead)? If so, can these acquire a header, be styled by the core themes, or with an id/class just for that table? I’ve seen several support requests wanting to get rid of this ‘phantom border’ http://drupal.org/node/210018.

While we’re here (sorry for the feature creep, but I think it sheds light on why this rule is here), system.css: line 32 styles a th that is in tbody instead of thead. But despite trying to catch this, the effect is different, with a 1px border above (from tbody) and below. If we want to do this, I think styling ;

tbody th {
  border-top: 1px solid #ccc;
  border-bottom: 1px solid #ccc;
}

would be better, as it wouldn’t affect anything else. If someone puts a th in a tbody, but not in the top row, should we worry? I think *_II.patch is best:

• tables without thead or th, would have no border, for layout
• tables without thead, but with th, would have 1px border above and below
• tables with th in thead, would have 3px border below

Moving "drupal.css" component issues to "markup".

As someone trying to add Drupal support to existing code, this is a very annoying bug.

Hope that it is fixed soon.

This recently burned me as well.

#4: table-annoyance_II.patch queued for re-testing.

.css file has been renamed: changing to d7 so that it passes tests.

th tags are now styled regardless of being in a thead or tbody with a border-bottom, so tbody doesn’t need a border-top.

If you agree, please set to RTBC and this might get into d8?

PS. Is this really markup? If you move it from drupal.css to markup — then tell me that you’ve moved it from drupal.css to markup — doesn’t really say why…

/**
 * Increment variable
 */
x = x + 1;

@zeta Have you tested this with Bartik, Garland and Seven to make sure removing it doesn't cause any problems? The patches we create to remove/modify CSS in core need to be tested that way, and if there are any issues those themes need to be adjusted in the patch.

Also, what's with the code block in #12? Mistake?

And yes, this is the correct component. General front-end related issues are handled here.

I have tested this with all four core themes. All are tableless designs, so any tables would be created by modules – hopefully through the theme layer.

Tables in Bartik & Seven are not affected because;

Bartik

borders are set in style.css

Seven

lots of properties including borders are reset by reset.css

So the rule that this patch removes is always over-ridden.

In Garland & Stark

• Tables with headers are not affected because the header has a 2 or 3px border-bottom.
• Tables without headers are not affected because the rows have class odd or even, which sets the border.

This just leaves tables without headers whose rows don’t have class odd or even (ie. haven’t been themed); these have the phantom border. No drupal tables should fall into this category, only those written by the site admin or content authors.

Code block in #12 includes a comment ‘explaining’ what is being done but not why cf. #6.

Should the component be called “General front-end related issues”? markup is the M in HTML; there’s no M in css.

~

Ok, tested and this is good to go.

Should the component be called “General front-end related issues”? markup is the M in HTML; there’s no M in css.

Yes, we're working on getting it changed. This is where we monitor issues in the meantime. The reason your patch has been reviewed and marked RTBC is because it's in this component, so please be patient and bear with us.

w00t! Thanks Jacine,
_{___________________
patient since January 30, 2008 at 2:37pm…}

LOL. No problem :)

Hm. Sorry, I don't think we can change this at this point, given it's been like this since Drupal 4.something. There might be themes relying on this behaviour, which would result in visual inconsistency when their end users updated to Drupal-7.0-rc1, requiring upstream changes to restore. Markup freeze was December 2009 except for bug fixes, of which this doesn't qualify (merely an annoyance).

I'm happy to recommend this immediately get committed to D8 though, along with anywhere else system module is hard-coding fonts/colours/borders.

Thanks webchick, I appreciate your concerns regarding timing – 890 themes is a lot to check.

It is not merely an annoyance, though – my motivation for trying to push this though was the number of people, including myself, who were spending time trying to eliminate this border. It isn’t a bug in drupal, but if you add a table to a page, that table already has a bug in it.

/me looks for hard-coding in system module…

#12: table-annoyance_7.x.patch queued for re-testing.

Committed to 8.x. We can't really backport this to Drupal 7 as it could break existing themes. Thus, marking as 'fixed'.

In Drupal 7.4, system.theme.css

tbody {
  border-top: 1px solid #ccc;
}

We, users, need cut this code, before upgrade our site to drupal 8 ?

I feel like the reason of

We can't really backport this to Drupal 7 as it could break existing themes.

isn't justified.

The reason I say this is, If we look at the modules in the community that are just an API and have no other functionality. If an issue was to happen to that module, it would be fixed in the current and back ported if needed. As far as I can tell if you are depending on another module the attitude has been the dependency module made a patch and if it breaks your module then you need to fix it and catch up. Besides Drupal not having a dependency itself I'm not sure why that attitude isn't being applied here.

Valeratal if you are asking if we need to remove this code manually before upgrading to Drupal 8 then the answer is no. When moving to Drupal 8 this will be fixed for you automatically.

If we look at the modules in the community that are just an API and have no other functionality. If an issue was to happen to that module, it would be fixed in the current and back ported if needed.

Yes, our CSS can be considered APIs. But a "border-top on all tables" isn't a broken API, it is a poorly designed one. And we don't break APIs in modules in D7 just because they were poorly designed.

Setting this issue back to "closed".

Incredible that this is closed - took me hours to figure out, thought it /must/ be a problem with the theme I was using. It's still doing it!