Comments

aspilicious’s picture

I confirm this

avpaderno’s picture

StatusFileSize
new32.42 KB

I can confirm it also in the block settings page.

avpaderno’s picture

Title: Border on Customize Shortcuts page » Table borders are not correctly drawn
StatusFileSize
new17.52 KB
new20.57 KB
new15.61 KB

The problem is present in different tables. I see the problem on Safari 4 for Mac OS 10.6.

nvanhove’s picture

cosmicdreams’s picture

Possibly relative to this issue #746678: Markup issue with tables (admin/appearance/update)

Check to see if the call to theme_table is properly implemented. That was the cause of the issue linked above.

rickvug’s picture

Note that the problem here only effects Webkit based browsers and does not effect all table rows. On the blocks listing page only draggable rows display the bug. To debug I've tried very specific CSS selectors targeting specific table rows and have not been able to style the righthand side of the table or rows contained within it.

eliza411’s picture

Assigned: Unassigned » eliza411
eliza411’s picture

Assigned: eliza411 » Unassigned
seutje’s picture

Status: Active » Needs review
StatusFileSize
new3.36 KB

this just pisses me off

I'll write some more info on what and why, but I gotta run right now

seutje’s picture

StatusFileSize
new464 bytes

heh, just realized, since this is a webkit only bug, we can use :last-child and we don't need to jump through hoops to slip a class in there

jcarrig’s picture

StatusFileSize
new41.29 KB

I just applied this patch in comment #11 and tested in the following:

  • Chrome 5.0.342.9 beta
  • Safari Version 4.0.5

on Mac OS 10.5.8

It fixes the issue. See attached png.

nvanhove’s picture

Status: Needs review » Reviewed & tested by the community

Let's get it in :)

cosmicdreams’s picture

Status: Reviewed & tested by the community » Needs work

still applies, still works for Chrome. I tested the rest of the browsers I have and found it works in : Chrome 5 beta, Firefox 3.5, IE 6, IE 7, Safari 4, Opera 10

However, with this patch there is an issue with IE8 not being able to correctly render the right border of the tables.

cosmicdreams’s picture

When the borders collapse the right border of these rows is clobbered. Perhaps if applied a "lastcell" or "endrow" class to the last cell of a row we can support this formatting better.

cosmicdreams’s picture

This looks to be the result of IE8 table rendering bugs. The bug is demonstrated here: http://www.gtalbot.org/BrowserBugsSection/MSIE8Bugs/TableRulesAll-BCSepa...

seutje’s picture

patch in #10 illustrates the craziness required to add a class

does that fix it for IE8?

cosmicdreams’s picture

I'll give it a try

Jeff Burnz’s picture

Status: Needs work » Needs review

#11: 674082-seven-table-webkit-11.patch queued for re-testing.

Jeff Burnz’s picture

Status: Needs review » Needs work
+++ themes/seven/style.css
@@ -519,6 +519,13 @@ table tr.selected td {
+/**
+ * Exception for weird webkit bug with the right border of the last cell
+ * in some tables, since it's webkit only, we can use :last-child
+ */

Looks like this is RTBC but I'm not to sure about using the phrase "weird bug" in a core comment :)

Powered by Dreditor.

adrinux’s picture

StatusFileSize
new458 bytes

Confirmed to fix the issue in Safari 5.0.2

So. I changed comment text to 'bug' instead of 'weird bug'.

avpaderno’s picture

Status: Needs work » Needs review

I can confirm the issue on Google Chrome too; applying the patch resolved the issue for me (Windows 7).
I applied the patch manually, because in the last Drupal 7 beta there is a style more before the Fieldsets comment; I am not sure if the patch applies to the latest developed code for Drupal 7.

I am changing the status to allow the test bot to test the patch.

avpaderno’s picture

Status: Needs review » Reviewed & tested by the community

As the bot is happy, I am changing the status to RTBTC.

webchick’s picture

Status: Reviewed & tested by the community » Needs review

Hm. While I certainly want to see this fixed, are we supportive of these kind of browser-specific hacks in core CSS? What exactly is triggering the webkit bug?

Pinged Jacine about this.

mark trapp’s picture

Here is the list of bug reports regarding border-collapse causing weird issues with borders in WebKit: https://bugs.webkit.org/buglist.cgi?quicksearch=border-collapse. Bug 8914 seems to have a test case reduction similar to the problem in this issue, but it hasn't been touched since 2006.

As of right now, it's mostly unconfirmed, but enough people are experiencing it to consider it to be a real issue, I think. Really odd that it hasn't been caught before, and really sad that we have to resort to WebKit-specific hacks.

adrinux’s picture

@webchick @Mark Trapp referring to it as a "browser specific hack" is a bit OTT. Firstly :last-child is a perfectly valid pseudo class in CSS3, and secondly it's not browser specific so much as browser rendering engine specific, Safari and Chrome are listed in this thread, and no doubt other webkit based browsers like Konqueror are also afflicted.

And whilst the extra ruleset is redundant for other CSS3 supporting browsers re-declaring a border is hardly likely to cause problems.

I think seutje's teeny patch is an elegant solution and in keeping with http://drupal.org/node/769692
"This means that layouts must not break, and obvious differences/bugs should be worked out."

mark trapp’s picture

Standards-compliant browsers are referred as that because they all render the same markup/CSS in roughly the same way: it allows us to use the same markup and styling without modification and have the final result look the same regardless of the browser one uses.

Special casing (i.e. so-called "hacks") is something that is reserved for when a browser doesn't render the way all other browsers do: either because the feature is experimental (i.e. using prefixed attributes) or the browser's rendering is noncompliant due to a bug (e.g. IE). It's not supposed to be the normal action: it's meant to compensate for problems with how the browser renders things by exploiting its quirks.

In this case, it's a WebKit bug: it's the only browser that messes the current markup/CSS up, and the patch is introducing a "hack" to fix it. Whether other browsers ignore the "hack" is beside the point: compliant browsers ignore all the hacks we use for IE6, too.

The patch might be the only way to get it to render correctly in WebKit, but if there's a way to accomplish the same result without the use of special-casing for WebKit, that'd definitely be preferable.

adrinux’s picture

@Mark Trapp
1. There is nothing 'hackish' about the CSS in this patch, it's a perfectly valid CSS3 selector and a border rule. A CSS hack implies something quite different, taking advantage of parsing errors etc, those we avoid for good reasons. This is a workaround, not a hack.
2. Webkit is NOT a browser*. It's a browser rendering engine. This rendering bug is present in one engine but multiple browsers.
3. The only 'hack' I use for IE6 is conditional comments, and being an IE specific feature of course they only target IE6.
4. Of course it's preferable if it can be achieved without adding what should be a superflous ruleset. If you have a better workaround lets hear it.

* Well, ok you can download nightly builds of safari called webkit http://nightly.webkit.org/ but that's not what we're talking about.
And if you still say it's a hack affecting one browser we'll have to agree to disagree. :)

@everyone
- Poking about on the manage display tabs for taxonomy admin/structure/taxonomy/to/display the right border definitely goes missing when tabledrag.js runs. And the border reappears when you 'show row weights'.
- On the image styles table, at admin/config/media/image-styles if you change the colspan on the operations th tag from 3 to 2 in the webkit inspector:

<th colspan="3">Operations</th>
to:
<th colspan="2">Operations</th>

The border reappears.

It really does seem like a bug in webkit as per previous comments. And it does seem to involve unnecessary colspans.

Then again looking at admin/structure/block and the missing right borders it looks like the table rows are 1px too short and playing with colspan has no effect.

Where are the colspans being added to the tables? Too late for me to hunt today...

mark trapp’s picture

@adrinux: change "browser" to "rendering engine" and my point still stands. And a quick check points to a dozen instances of use of the star HTML hack—meant to target only IE6's engine—in core.

But all that is beside the point: we're arguing over semantics here. I think we can all agree that, if theres a way to get the end result without special-casing, hacks, or quirks for specific browsers, browser engines, or versions of browser engines, we should explore that before introducing what the patch provides into core.

jacine’s picture

I've been dealing with this really annoying issue for what seems like forever. The patch in #21 is what I do to fix it, and I'm pretty sure it's a bug. A good place to see it in action is on admin/config/regional/date-time/formats with JavaScript disabled just so you can see that there's no scripts affecting it.

<table class="sticky-enabled">
 <thead>
   <tr>
     <th>Format</th>
     <th colspan="2">Operations</th>
   </tr>
 </thead>
 <tbody>
   <tr class="odd">
     <td colspan="3" class="empty message">No custom date formats available. <a href="/admin/config/regional/date-time/formats/add">Add date format</a>.</td>
    </tr>
</tbody>
</table>

That's a perfectly good table, and the bug appears like clockwork. If you open up the inspector and screw around with the td colspan value, i.e. change it to 2, and then back to 3, you'll see the border is redrawn properly.

Since the patch is how I've been fixing it for the last ~2 years, I'm totally fine with it in Seven.

jacine’s picture

Status: Needs review » Reviewed & tested by the community

Oh, and I forgot to mark it RTBC.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Ok, cool. That works for me.

Committed to HEAD. Thanks!

Status: Fixed » Closed (fixed)

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