Created attachment 128273 [details] testcase When a table have this "border-collapse: collapse;", table border rendering is getting wrong. You can see that by comparing attached HTML in webkit and not webkit browsers.
If rtl is specified along with border-collapse: collapse and border-left, the border appears on the right hand side. If border-collapse is not specified the border appears on the left.
Hi! Please be more serious about this bug :D
CC'ing David Hyatt to this ticket to get some advice. The root cause of the bug is that the text direction on the table->style() is different from the text direction on the cell style. In RenderTableCell, 'computeCollapsed[Start/End/Before/After]Border' use the text direction from 'table->style()' to determine the color component of the border, but use the text direction from 'style()' to determine the other components of the border. Additionally, in 'paintCollapsedBorders', it uses the text direction from 'table->style()' when looking up the cached collapsed borders. In order to fix this, we need to make sure the text direction is obtained from the same style object in all these places. There are 2 ways I can think of to fix this bug, but I'm not sure which would be more "correct": 1. Change 'computeCollapsed*Border' to use 'style()' when computing the color component of the border. Also change 'paintCollapsedBorder' to use 'style()' when looking up the cached collapsed borders. 2. Change 'computerCollapsed*Border' to use 'table->style()' when determining all the other components of the border. This would involve adding methods like: const BorderValue& borderBefore(RenderStyle* tableStyle) const; const BorderValue& borderAfter(RenderStyle* tableStyle) const; const BorderValue& borderStart(RenderStyle* tableStyle) const; const BorderValue& borderEnd(RenderStyle* tableStyle) const; to the RenderTable[Cell|Col|Section] classes. I'm leaning towards option (1), but the fact that tableStyle is explicitly used in the cached border lookups makes me wonder whether there is a reason table->style() needs to override the cell's style?
Also note that rev 71251, where RenderStyle's border[Start|End|Before|After] were introduced, the checkin comment says "This change still doesn’t support cells whose block flow differs from the table’s."
Created attachment 137610 [details] modified testcase Attaching a modified testcase that demonstrates more clearly what is going on. Notice that when "border-collapse: collapse" is specified: "10px solid black" is rendered as "10px solid green" "20px dotted green" is rendered as "20px dotted black" (in addition to each of them being on the wrong side of the cell)
@Shezan Baig: Thanks for assigning this bug to yourself :) I believe this bug is a regression, can you check very old Safari for it? I think if you find the changes/commit that caused this bug you can fix it easier, thanks in advance :)
Ebrahim, if you move "direction: rtl;" to the table element instead of the td element, I believe this will fix the problem in your case.
Thanks for your propose but I saw this bug somewhere that I had not access to edit, also that table must have mixed RTL and LTR cells because it was a table for translations, one column was in English (message title) and other column was for translated RTL message (and some cell must be English because they was not translated), I know for this case they could use a display: block RTL wrapper but I think this problem worth to be fixed. Thanks again :)
Sorry for the delay, I completely missed this bug. (In reply to comment #6) > @Shezan Baig: Thanks for assigning this bug to yourself :) I believe this bug is a regression, can you check very old Safari for it? I think if you find the changes/commit that caused this bug you can fix it easier, thanks in advance :) I really wonder if it's a regression as we have had issues with mixed directionality for border collapsed computation for quite some time. After bug 87900 we support mixed directionality at the section level but not below it (this was the cause of super nasty bugs where we would pick up an unset border). @Shezan Baig, your option (1) is only part of the fix and is missing the big picture. I am not supportive of implementing a band-aid solution and we should do several things: * disable 'direction' at the row level as it's a lot of complexity for a corner case usage with an easy work-around (just wrap your cells in row-groups). It's possible to implement that but it's pretty hard to do properly. * add support for mixed directionality at the cell level. This will need testing as there are lots of cases that we need to get right, involving mixed directionality at different levels. It's not a simple bug to fix and I would advise looking at bug 87900 for some context on the issues to overcome. Also feel free to poke me as needed for guidance.
Created attachment 155836 [details] Safari 5.0 vs Chrome
Created attachment 155839 [details] Safari 5.1.7 vs Chrome
I tested #5 testcase on http://browsershots.org/http://jsfiddle.net/ebraminio/cfzYt/embedded/result/ difference is available between chrome 8 and 9: http://browsershots.org/screenshots/06d410c03a3a36881f066b6d4de4d8e6 http://browsershots.org/screenshots/e53045f432e199ce342757d8a1f9c183
Ebrahim, thanks for the snapshots. It's definitely a regression, bumping the priority. My take would have been from r119594 but it was landed after the bug report. Shezan, do you feel like fixing this bug soon? If not, I will need to take a stab at it. As said, the testing is pretty important here and should thoroughly cover mixed directionality between table, table-row-group and table-cell in horizontal and vertical writing-modes. Currently we don't properly handle mixed directionality with table-column / table-column-group. table-row is a hard case and could be postponed to another bug (it could regress painting performance AFAICT).
(In reply to comment #13) > Shezan, do you feel like fixing this bug soon? If not, I will need to take a stab at it. > Julien, would love to take a stab at this, but I'm currently tied up with a bunch of other priorities that may take a while. Feel free to take this if you want. Thanks, Shezan
Created attachment 172290 [details] Proposed fix: handle mixed directions at the cell level.
Comment on attachment 172290 [details] Proposed fix: handle mixed directions at the cell level. Attachment 172290 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14725676 New failing tests: css2.1/20110323/border-conflict-element-038.htm css2.1/20110323/border-conflict-element-029.htm fast/css/border-conflict-element-002.htm
Comment on attachment 172290 [details] Proposed fix: handle mixed directions at the cell level. Attachment 172290 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14680218 New failing tests: css2.1/20110323/border-conflict-element-038.htm css2.1/20110323/border-conflict-element-029.htm fast/css/border-conflict-element-002.htm
Comment on attachment 172290 [details] Proposed fix: handle mixed directions at the cell level. The failures are related to handling mixed direction between cells and columns / column-groups - which I though I could get away with postponing to a follow-up bug.
Still exists, let's see what we can do about it now.
WebKit still flails on this test. Blink and Gecko produce valid results.
<rdar://problem/96918171>