[PATCH]: improved ROWSPAN handling
Here's a page demonstrating some problems Dillo has when all the cells in a table have ROWSPAN > 1 . http://starurchin.org/dillo/bugs/table_rowspan.html The cause of these issues is that Dillo has (excuse me) a rather clumsy way of calculating the row heights once it has assigned column widths and calculated the cell heights. It first calculates row heights based on the cells with ROWSPAN = 1 , and then adds extra height to rows to accommodate cells with ROWSPAN > 1 . This algorithm gives poor results when no cells have ROWSPAN = 1 . It can add unnecessary vertical space (see example on the page). Worse, if there is no vertical padding in the table as well as no cells with ROWSPAN = 1 , the table has zero height after the first pass. The second pass can't cope with this (because it would be dividing by 0) so it does nothing, so the table still has no height when the layout is finished. This messes up the redisplaying of the contents and can create "dead" text that can't be selected or clicked. This patch fixes all these problems by rewriting the algorithm that calculates row heights. Nothing else in the layout code is changed. The new code scans all the cells once, top-down, and keeps track of the minimum height we must insert between the top of the table and the bottom of the cell. Once each cell is dealt with, the height assigned to its top will not change (because we work top-down) and the height assigned to its bottom can only increase, so by the end all cells have at least as much vertical height as they need. Then another pass tours the height constraints and writes the correct values into the cumHeights vector. Advantages of this patch: * Fixes the bugs noted above. * Assigns the minimal possible vertical height to the table (given the column widths that have already been assigned). * Treats all cells the same no matter what their ROWSPAN, so there are no special cases when no cells have ROWSPAN = 1 . * Doesn't do any division, so there are no special cases when the available padding is 0 . If this change is merged I'll submit a cleanup patch that removes the now-unused apportionRowSpan() method. This was a purely internal change, so I didn't need to update any docs. (Phew!) I need to look at the handling of ROWSPAN="0" attributes. Are these even legal? Regards, Jeremy Henty
Jeremy wrote:
I need to look at the handling of ROWSPAN="0" attributes. Are these even legal?
I didn't change html.cc to allow it, but it's legal: "rowspan = number [p.50] [CN] [p.49] This attribute specifies the number of rows spanned by the current cell. The default value of this attribute is one ("1"). The value zero ("0") means that the cell spans all rows from the current row to the last row of the table section (THEAD, TBODY, or TFOOT) in which the cell is defined."
Here's a page demonstrating some problems Dillo has when all the cells in a table have ROWSPAN > 1 .
Actually, on second thoughts I'm not so happy with this patch. It resolves all the slack in the row boundaries by pushing them up as far as possible. Let me see if I can get it to spread things out better. Jeremy Henty
participants (2)
-
onepoint@starurchin.org
-
place@gobigwest.com