Re: svn commit: r530727 - in /xmlgraphics/fop/trunk: src/java/org/apache/fop/fo/flow/Table.java status.xml

[Jeremias Maerki <de...@jeremias-maerki.ch>]

Great news, Vincent! Good stuff!

Of course, I had to try it out immediately and it looks promising. Still,
I've got a few points for you to look into:
- Please reenable the two test cases for border collapse. That'll help
catching regressions. You may need to add additional tests (including
checks for the element lists) to cover all possibilities .
- table_border-collapse_collapse_1.xml shows a little flaw in border
painting. Check out the lower left edge of cell 1. Looks like a special
situation where the normal border logic that I've come up with in 2005
doesn't look so nice. I mean, inside the table there's little that you
can do to avoid such an effect, I think, but in the table's outer border
the border could be fully painted if the adjacent cell doesn't have a
border, it could be improved. Probably low priority as it is a special
situation (and other implementations look much worse than FOP now). But
maybe it's good to keep an eye on it.
- There seems to be a bug with the area generation when you have padding
and a background. The cell's content area doesn't declare any padding
and the horizontal position doesn't take padding into account. Therefore
content is vertically displaced and the cell background doesn't cover
the padding. See attached FO (the supposedly black cells at the bottom).
Should be easy to fix.

On 20.04.2007 12:21:29 vhennebert wrote:
> Author: vhennebert
> Date: Fri Apr 20 03:21:19 2007
> New Revision: 530727
> 
> URL: http://svn.apache.org/viewvc?view=rev&rev=530727
> Log:
> Time to enable border-collapse="collapse"
<snip/>

Jeremias Maerki

Re: svn commit: r530727 - in /xmlgraphics/fop/trunk: src/java/org/apache/fop/fo/flow/Table.java status.xml

[Vincent Hennebert <vi...@anyware-tech.com>]

Hi Jeremias,

Jeremias Maerki a écrit :
> Great news, Vincent! Good stuff!

Thanks! Although there is still some work to do.


> Of course, I had to try it out immediately and it looks promising. Still,
> I've got a few points for you to look into:
> - Please reenable the two test cases for border collapse. That'll help
> catching regressions. You may need to add additional tests (including
> checks for the element lists) to cover all possibilities .

Actually the first test still doesn't pass because of a forced height
set on a row which is handled wrongly. The fixed height is directly
applied to the BPDs of the contained cells, instead of substracting the
cells' border-top/bottom widths. This is independent of the collapsing
model.
The second test needs to be completed, I'll do that in the next days,
along with creating additional testcases.


> - table_border-collapse_collapse_1.xml shows a little flaw in border
> painting. Check out the lower left edge of cell 1. Looks like a special
> situation where the normal border logic that I've come up with in 2005
> doesn't look so nice. I mean, inside the table there's little that you
> can do to avoid such an effect, I think, but in the table's outer border
> the border could be fully painted if the adjacent cell doesn't have a
> border, it could be improved. Probably low priority as it is a special
> situation (and other implementations look much worse than FOP now). But
> maybe it's good to keep an eye on it.

Hmmm, indeed. I agree with you that this is of low priority, because I
really don't expect such a case to occur often in real life. Moreover,
this belongs to the border-painting part and I don't feel like starting
to look at it now.

While we're speaking of border-painting, FOP's behavior doesn't match
Firefox's one. At the grid intersections, Firefox draws the biggest
border, instead of drawing each of the four borders merging at the
intersection point (hope I'm clear, can't find the words). We can see an
illustration of that in the CSS2 spec:
http://www.w3.org/TR/REC-CSS2/tables.html#border-conflict-resolution

I don't think either of the two behaviors is required by the spec. But I
think both might be desirable as they let achieve different effects. The
Firefox one will cause... interesting problems to occur as a cell's
content may be encroached upon by a border which doesn't play in the
resolution of that cell's borders!


> - There seems to be a bug with the area generation when you have padding
> and a background. The cell's content area doesn't declare any padding
> and the horizontal position doesn't take padding into account. Therefore
> content is vertically displaced and the cell background doesn't cover
> the padding. See attached FO (the supposedly black cells at the bottom).
> Should be easy to fix.

Thanks for spotting that, I've committed a fix. Testcases pending.


Cheers,
Vincent