Inner borders on table cells missing

[johnbeard]

The following HTML:

<table style="margin:auto; border:1px solid red; border-collapse:collapse;">
    <tbody>
    <tr style="border:1px solid black;">
        <td style="border:1px solid pink;">Foo</td>
        <td>Bar</td>
        <td>Quux</td>
    </tr>
    <tr style="">
        <td>Foo</td>
        <td>Bar</td>
        <td>Quux</td>
    </tr>
    </tbody>
</table>

In Koreader, the result is something like this:

The expected output is more like this (Firefox):

Table border.epub.zip

[poire-z]

Yes, not easy to get right.

We take some shortcuts around there to keep the (already tedious) work low:

crengine/crengine/src/lvrend.cpp

Lines 935 to 947 in 8254cef

	// Now, we should disable some borders for this cell,
	// at inter-cell boundaries.
	// We could either keep the right and bottom borders
	// (which would be better to catch coordinates bugs).
	// Or keep the top and left borders, which is better:
	// if a cell carries its top border, when splitting
	// rows to pages, a row at top of page will have its
	// top border (unlike previous alternative), which is
	// clearer for the reader.
	// So, we disable the bottom and right borders, except
	// for cells that are on the right or at the bottom of
	// the table, as these will draw the outer table border
	// on these sides.

For your expected output, we'd have to report some borders from some cells to some other cells - which feels quite more complicated.

[johnbeard]

Interesting. For my own future reference, the border-collapsing model is described here: https://www.w3.org/TR/CSS2/tables.html#borders
Specifically, the section 17.6.2.1 Border conflict resolution is what would be needed to deal with this (and as you say, it's not trivial)

However, what's the thinking behind this bit?

                     // Now, we should disable some borders for this cell,
                    // at inter-cell boundaries.

Since I suppose "most" tables that set borders on rows/cols/cells individually default to no borders and turn them on selectively, it's seems likely that the current "top and left only" method is falsely turning borders off than falsely turning then on (or overriding with the wrong style)?

For example, a border-bottom on a "header row" is pretty common (it's now I found this behaviour).

In the EPUB above, just commenting lines 948 to 951

crengine/crengine/src/lvrend.cpp

Lines 948 to 951 in 8254cef

	if ( !is_at_right )
	newstyle->border_style_right = css_border_none;
	if ( !is_at_bottom )
	newstyle->border_style_bottom = css_border_none;

produces this:

which, while it won't quite get it right for very complex cases with multiple different borders competing for a single edge, is at least closer to the mark than not having any right or bottom borders at all?

[poire-z]

what's the thinking behind this bit?

It's the way crengine does its drawing: it doesn't do: draw borders, and then, between the borders, the cell content.
What it does (for every block element): each block/cell carries its own borders, get the own top/left x/y, and it draws them (border + content) at x/y+w/h.
For non-collapsed borders, that's just fine.

But, for drawing a 1px collapsed/shared border, if each cell would draw its own, we'll have 2px borders. So, the solution for collapsing was to kill 2 borders (nearly arbitrary, but see my comment) on each cell, so each cells draws fully 2 borders, and get the other 2 from adjacent cells.
So, yes, we can falsely turn off some borders - and don't get any if they are not set on the adjacent cells.

Doing differently would need another (tedious) algorithm to decide what to cancel and what not, and possibly shift a bit cells' x/y/h/w so they align correctly whether some provide some borders and others not...

[johnbeard]

Oh, I seeeeeee. Makes much more sense now.
Certainly sounds painful. I think the worst part might actually be that figuring out a edge's neighbors is tricky due to rowspan and colspans.
Once you have a list on "contenders" for an edge, doing the priority part is tedious but it basically a big chain of if's.

[johnbeard]

What it does (for every block element): each block/cell carries its own borders, get the own top/left x/y, and it draws them (border + content) at x/y+w/h.

Technically, it's supposed to offset by half the border width, I think, so the top corner is (x-b/2, y-b/2) and the total size, measured from outer edges of the border, is (w+b, h+b).

For example, from the CSS spec example:

vs (with the lines above commented to show all borders, ignore the missing black line, that's because COL doesn't work):

so the blue and green borders start a little too far down and to the right.

BTW, here's with stock crengine, lines un-commented:

[poire-z]

To not lose it, some related discussion on gitter around https://gitter.im/koreader/koreader?at=5ff4d73dacd1e516f8d637f9