Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.0.1) Gecko/20020826 Bidi : Border-collapse property in RTL direction is not working correctly Des : 1) Load the test case 2) Cell borders in RTL direction are not displayed correctly 3) It works fine for LTR direction in Mozilla 4) Check the attached images between Mozilla 1.0.1 and IE 5.5

*** Bug 176685 has been marked as a duplicate of this bug. ***

I think this really belongs in Layout: Tables

Confirming the cell borders are not correctly displayed with build: 2002-11-05-08-trunk on WINXP.

the cells in an rtl table merge for some reason

The border collapse code paints the table borders as they are ltr over the table.

This is a big problem for BiDi users. Can someone tell me where I should look for the code for collapsed table borders?

The bug is somewhere in this code: http://lxr.mozilla.org/seamonkey/source/layout/html/style/src/nsCSSRendering.cpp#2252

*** Bug 216445 has been marked as a duplicate of this bug. ***

*** Bug 221935 has been marked as a duplicate of this bug. ***

(In reply to comment #9) Actually, my guess would be that the bug is in the code which _calls_ the special border-collapsed code (i.e. calls nsCSSRendering::DrawTableBorderSegment). Using the LXR this may be http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableFrame.cpp with the conspicuously-named function 'CalcBCBorders' at http://lxr.mozilla.org/seamonkey/source/layout/html/table/src/nsTableFrame.cpp#5647 one would think that function would check the 'mLeftToRight' member, which it doesn't, apparently.

I nailed it... although I think the code for border-collapsed border drawing needs some rewriting so as to make less assumptions about the table, to encapsulate and generalize the iteration better, etc. I can't do this since I know epsilon << 1 about the layout; I was simply able to make it get the correct table columns for border segment x-coordinate calculations.

I nailed it... although I think the code for border-collapsed border drawing needs some rewriting so as to make less assumptions about the table, to encapsulate and generalize the iteration better, etc. I can't do this since I know epsilon << 1 about the layout; I was simply able to make it get the correct table columns for border segment x-coordinate calculations.

Comment on attachment 141451 [details] [diff] [review] proposed fix I'm requesting both review types since I believe this fix can have no itegration issues - it's only about drawing pixels at different X coordinates.

Comment on attachment 141451 [details] [diff] [review] proposed fix On further testing, it seems some more tweaking is necessary to properly handle cases more complicated than the bug testcase shown here. So consider this patch a partial fix. Updates to follow.

Sorry for the spam, but talking to bernd and looking at bug 229883 (and to some extent also 4510) made me realize I can't really squeeze a fix in without a major rewrite, of hundreds if not thousands of lines of code, for RTL compatibility - and this would not be a good idea before at least the patch for 229883 lands (or so it seems to me anyway), so I am tentatively marking this bug as depending on 229883. PS - The relevant files are nsTableFrame.* , nsCellMap.* and possibly also nsTableRowGroupFrame.* The main 'issues' AFAICT are the order of iteration over columns, the order of iteration over cells in a row, the determining of which borders/cells/columns are rightmost, the decision who owns which border and the decision of when to sort-of-duplicate borders to prevent overlapping the border of one cell with the cell content of the next one.

Eyal, as you guessed this will be anything but a small patch. So its good if you start to study the code in CalcBCBorder. Its full of the ltr assumption. Furthermore the testcase in this bug demonstrates the wrong behaviour but for a fix we would need a testsuite that verifies the correctness of the border computations especially when the left and the right border are asymmetric in width and style.

This is not a comprehensive testsuit, but it does have some pitfalls which are circumvented in the original testcase, which my fix-attempt fails to avoid.

*** Bug 236884 has been marked as a duplicate of this bug. ***

Bug 139627 ("Table rules drawn ignoring dir=rtl setting") is related. Prog.

*** Bug 242855 has been marked as a duplicate of this bug. ***

*** Bug 246244 has been marked as a duplicate of this bug. ***

I found out that something the previous example looks OK. Most of the times it looks like the bug is there. Maybe the cause isn't only in the places people thought it will be.

(In reply to comment #25) > I found out that something the previous example looks OK. Most of the times it > looks like the bug is there. something = sometimes I got it right more than 5 times... Another test site: http://starcomsystems.com/hu/mozbug001.htm

*** Bug 260612 has been marked as a duplicate of this bug. ***

> I found out that something the previous example looks OK. Most of the times it > looks like the bug is there. But in the png the two tables are not mirrors of each other. What do you mean when you say it "looks ok"? The bottom table structure is not a mirroring of the top table structure.

These are two approaches to solving one of the two halves of this problem -- the painting half, rather than the collapsing half. The first approach tries to correct things manually, the second approach is simply to translate and scale the rendering context. The second could probably be made to work with a bit of GFX hacking; the first could probably be made to work by making the code a good bit uglier still.

My limited intuition says that the eventual solution to this problem would be to draw each cell's border separately (see bug 203686). In the general case there is no way of avoiding this (e.g. cells with position: relative), plus it has been suggested that some careful coding would make this not-so-expensive. Still, a temporary fix for the specific issue of RTL tables is still very welcome. As for the two approaches - do they both not work at the moment?

I think in the collapsing border model, cells and rows with 'position: relative' shouldn't move the borders.

And, FWIW, I suspect it would be easier to get the inverse of approach 1 working fully -- that is, paint the borders going from left-most to right-most rather than from start-most to end-most.

(Note that I think that's what I was doing for the vertical ones, but not the horizontal ones -- and it's the horizontal ones that have most of the problems.)

Don't forget to remove the printfs, and also don't forget to file a separate bug on 'border-left' and 'border-right' being backwards for border-collapsed tables.

> 'border-left' and 'border-right' being backwards for border-collapsed tables. what?

Sorry, for any table elements in RTL border-collapsed tables.

So if I set table { border-left: solid red; } the border will be on the right side of the table??

Yep (with this patch). It should be pretty easy to fix, though.

The patch is more a demonstration that we could do it the dirty way now instead of waiting for the big and more correct solution. It fixes the wrong painting (there are still some pixel alignment issues - see the border close to the 2). It still asserts on Davids testcase (during scroll) and has a invalidation problem. I would like to split the problem into the painting issue here, and the rtl border computation in another bug, for 95% percent of the cases I guess this will help our users already, I mean all those excel generated tables. I hope I will have it review ready on sunday.

Comment on attachment 162208 [details] [diff] [review] patch with working invalidation David, could you please review it. The patch is modified from your first approach. I am thankfull for your help as I am weak at architectural problems. The main differences are some pixel aligment, side reversal and the startColX computation. Once this goes in I will file the bugs for the remaining issues (left to righ reversion, the testcase asserts even in the ltr case, some borders are wrong, the 2 is covered by the border)

Comment on attachment 162208 [details] [diff] [review] patch with working invalidation r+sr=dbaron, although I didn't go through all the details. But please rename |PRBool aTableDir| (what do true and false mean?) to something clearer like |PRBool aTableIsLTR| (which I think is what it means).

I checked in the fix, I will keep this bug open till I file all the followups

I filed: - bug 267418 on the assert that I see when parts of the table (even ltr) are painted. - bug 267419 on the rendering of the testcase in ltr conditions - bug 267420 on the correct computation of bc borders under rtl conditions.

*** Bug 139627 has been marked as a duplicate of this bug. ***

border-left acts like border-right with this fix (build 2004-11-04 06).

this is exactly what bug 267420 is about

*** Bug 119882 has been marked as a duplicate of this bug. ***

Unfortunatly this hasn't been fixed yet. Table borders are still incorrect on "rtl" direction, while "border-collapse: collapse" is set. Here is an example: http://nomaed.evil.co.il/moz_bugs/rtl_collapse_mistake.html

One more case of the bug: http://www.twinadv.com/setlanguage.php?id=2&langpath=/index.php I hope this will help ;)

(In reply to comment #55) > One more case of the bug: > http://www.twinadv.com/setlanguage.php?id=2&langpath=/index.php > I hope this will help ;) As a solution for meanwhile, just set in the table itself style="border- collapse: separate" That's what I did, and it helped (http://e-dent.msn.co.il - the main menu)

This is fixed on trunk, get either a ff trunk build or a Mozilla 1.8.5a before further commenting.

*** Bug 275498 has been marked as a duplicate of this bug. ***

Hello, Problem with collapsed borders in left to right is not completely over. When you first load the page, table borders are not correct. If you press the Refresh it will be rendered correctly this time. (I am not sure what can be the reason but I have tested it several times and every time refresh has fixed my table). http://cloob.com/bug/fire1.jpg and http://cloob.com/bug/fire1.jpg Please see the pictures (before and after refresh). Regards, Mac

Sorry, Links are : http://cloob.com/bug/fire1.jpg and http://cloob.com/bug/fire2.jpg and my version is : Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a6) Gecko/20041227 Firefox/1.0+ Regards, Mac

Yep happens here too. See this: http://www.twinadv.com/setlanguage.php?id=2

*** Bug 294946 has been marked as a duplicate of this bug. ***

*** Bug 302997 has been marked as a duplicate of this bug. ***

*** Bug 305251 has been marked as a duplicate of this bug. ***

This is not fixed!! take a look at http://ar.wikipedia.org/wiki/%D8%A5%D9%86%D8%B8%D9%8A%D9%85 for example, eaven reloading the page doesnt fix the table!! Please reopen this bug

Did you test with a recent build http://www.mozilla.org/projects/firefox/ like Firefox 1.5 Beta 2 ?

That wikipedia page looks fine to me in the latest trunk build.

Great ! with Beta 2

*** Bug 312622 has been marked as a duplicate of this bug. ***

Mass-assigning the new rtl keyword to RTL-related (see bug 349193).