Bug 6888 - Collapsed borders appear on the wrong side or on the wrong cell in RTL tables
Summary: Collapsed borders appear on the wrong side or on the wrong cell in RTL tables
Status: VERIFIED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tables (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords: HasReduction
Depends on:
Blocks: 6838
  Show dependency treegraph
 
Reported: 2006-01-28 05:11 PST by mitz
Modified: 2006-01-30 07:07 PST (History)
0 users

See Also:


Attachments
Testcase (777 bytes, text/html)
2006-01-28 05:12 PST, mitz
no flags Details
Proposed patch (22.04 KB, patch)
2006-01-28 08:18 PST, mitz
darin: review+
Details | Formatted Diff | Diff
Updated patch (21.79 KB, patch)
2006-01-29 08:19 PST, mitz
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2006-01-28 05:11:11 PST
The border-collapsing code assumes that cell 0 is the leftmost and that the last cell is the rightmost, which is wrong for tables with RTL direction. As a result: the table's left border is applied to the left border of the rightmost side and vice versa, and a cell's left border is collapsed with the right border of the cell to its right, and vice versa.
Comment 1 mitz 2006-01-28 05:12:49 PST
Created attachment 6045 [details]
Testcase

Firefox renders this correctly.
Comment 2 mitz 2006-01-28 08:18:40 PST
Created attachment 6050 [details]
Proposed patch
Comment 3 Darin Adler 2006-01-28 16:53:14 PST
Comment on attachment 6050 [details]
Proposed patch

I think it's a little inelegant to have that inLastColumn boolean and those "rtl ? !inLastColumn : (col() > 0)" clauses. Instead, could we just compute an appropriate boolean up front? Something like "righmostColumn" or "leftmostColumn"?

Looks great to me. r=me
Comment 4 mitz 2006-01-29 08:19:15 PST
Created attachment 6072 [details]
Updated patch

Replaced the inLastColumn booleans with leftmostColumn/rightmostColumn per Darin's suggestion.
Comment 5 Darin Adler 2006-01-29 08:52:27 PST
Comment on attachment 6072 [details]
Updated patch

Looks great. I think it's ready to land, but I do have 2 more ideas to improve it. I'll probably just do these if I land it:

    1) cellPrevious and cellNext should be named cellBefore and cellAfter to match the names cellAbove and cellBelow
    2) code to compute leftmostColumn and rightmostColumn should have more closely-matching sides of the if, no need to use an if statement in the RTL case
Comment 6 mitz 2006-01-30 07:07:55 PST
Verified in r12476 nightly