- added full support for keep-with-previous on table-row and in table-cell

- added more testcases for keeps in tables
- assign the right table-row element to grid units that are not on the first row spanned
- slightly re-worked the RowGroupBuilder interface to make it more SAX-like


git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@630814 13f79535-47bb-0310-9956-ffa450edef68

9 files changed, 150 insertions, 52 deletions

diff --git a/src/java/org/apache/fop/fo/flow/table/EffRow.java b/src/java/org/apache/fop/fo/flow/table/EffRow.java
index 7989f3d9d..0b00b9620 100644
--- a/src/java/org/apache/fop/fo/flow/table/EffRow.java
+++ b/src/java/org/apache/fop/fo/flow/table/EffRow.java
@@ -162,10 +162,65 @@ public class EffRow {
         }
     }
 
+    /**
+     * Returns true if the enclosing (if any) fo:table-row element of this row, or if any
+     * of the cells starting on this row, have keep-with-previous set.
+     * 
+     * @return true if this row must be kept with the previous content
+     */
+    public boolean mustKeepWithPrevious() {
+        boolean keepWithPrevious = false;
+        TableRow row = getTableRow();
+        if (row != null) {
+            keepWithPrevious = row.mustKeepWithPrevious();
+        }
+        for (Iterator iter = gridUnits.iterator(); iter.hasNext();) {
+            GridUnit gu = (GridUnit) iter.next();
+            if (gu.isPrimary()) {
+                keepWithPrevious |= gu.getPrimary().mustKeepWithPrevious();
+            }
+        }
+        return keepWithPrevious;
+    }
+
+    /**
+     * Returns true if the enclosing (if any) fo:table-row element of this row, or if any
+     * of the cells ending on this row, have keep-with-next set.
+     * 
+     * @return true if this row must be kept with the next content
+     */
+    public boolean mustKeepWithNext() {
+        boolean keepWithNext = false;
+        TableRow row = getTableRow();
+        if (row != null) {
+            keepWithNext = row.mustKeepWithNext();
+        }
+        for (Iterator iter = gridUnits.iterator(); iter.hasNext();) {
+            GridUnit gu = (GridUnit) iter.next();
+            if (!gu.isEmpty() && gu.getColSpanIndex() == 0 && gu.isLastGridUnitRowSpan()) {
+                keepWithNext |= gu.getPrimary().mustKeepWithNext();
+            }
+        }
+        return keepWithNext;
+    }
+
+    /**
+     * Returns true if this row is enclosed by an fo:table-row element that has
+     * keep-together set.
+     * 
+     * @return true if this row must be kept together
+     */
+    public boolean mustKeepTogether() {
+        TableRow row = getTableRow();
+        return row != null && row.mustKeepTogether();
+    }
 
     /**
      * Returns the break class for this row. This is a combination of break-before set on
      * the first children of any cells starting on this row.
+     * <p><strong>Note:</strong> this method doesn't take into account break-before set on
+     * the enclosing fo:table-row element, if any, as it must be ignored if the row
+     * belongs to a group of spanned rows (see XSL-FO 1.1, 7.20.2).
      * <p><strong>Note:</strong> this works only after getNextKuthElements on the
      * corresponding TableCellLM have been called!</p>
      * 
@@ -187,6 +242,9 @@ public class EffRow {
     /**
      * Returns the break class for this row. This is a combination of break-after set on
      * the last children of any cells ending on this row.
+     * <p><strong>Note:</strong> this method doesn't take into account break-after set on
+     * the enclosing fo:table-row element, if any, as it must be ignored if the row
+     * belongs to a group of spanned rows (see XSL-FO 1.1, 7.20.1).
      * <p><strong>Note:</strong> this works only after getNextKuthElements on the
      * corresponding TableCellLM have been called!</p>
      * 
diff --git a/src/java/org/apache/fop/fo/flow/table/EmptyGridUnit.java b/src/java/org/apache/fop/fo/flow/table/EmptyGridUnit.java
index 583abcaa3..201029ff1 100644
--- a/src/java/org/apache/fop/fo/flow/table/EmptyGridUnit.java
+++ b/src/java/org/apache/fop/fo/flow/table/EmptyGridUnit.java
@@ -33,7 +33,8 @@ public class EmptyGridUnit extends GridUnit {
      * @param colIndex column index, 0-based
      */
     EmptyGridUnit(Table table, TableRow row, int colIndex) {
-        super(table, row, 0, 0);
+        super(table, 0, 0);
+        setRow(row);
     }
 
     /** {@inheritDoc} */
diff --git a/src/java/org/apache/fop/fo/flow/table/FixedColRowGroupBuilder.java b/src/java/org/apache/fop/fo/flow/table/FixedColRowGroupBuilder.java
index 62cf3e26d..28a30c6f7 100644
--- a/src/java/org/apache/fop/fo/flow/table/FixedColRowGroupBuilder.java
+++ b/src/java/org/apache/fop/fo/flow/table/FixedColRowGroupBuilder.java
@@ -20,6 +20,7 @@
 package org.apache.fop.fo.flow.table;
 
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 import java.util.ListIterator;
 
@@ -80,14 +81,14 @@ class FixedColRowGroupBuilder extends RowGroupBuilder {
             rows.add(effRow);
         }
         int columnIndex = cell.getColumnNumber() - 1;
-        PrimaryGridUnit pgu = new PrimaryGridUnit(cell, currentTableRow, columnIndex);
+        PrimaryGridUnit pgu = new PrimaryGridUnit(cell, columnIndex);
         List row = (List) rows.get(currentRowIndex);
         row.set(columnIndex, pgu);
         // TODO
         GridUnit[] cellRow = new GridUnit[cell.getNumberColumnsSpanned()];
         cellRow[0] = pgu;
         for (int j = 1; j < cell.getNumberColumnsSpanned(); j++) {
-            GridUnit gu = new GridUnit(pgu, currentTableRow, j, 0);
+            GridUnit gu = new GridUnit(pgu, j, 0);
             row.set(columnIndex + j, gu);
             cellRow[j] = gu;
         }
@@ -96,7 +97,7 @@ class FixedColRowGroupBuilder extends RowGroupBuilder {
             row = (List) rows.get(currentRowIndex + i);
             cellRow = new GridUnit[cell.getNumberColumnsSpanned()];
             for (int j = 0; j < cell.getNumberColumnsSpanned(); j++) {
-                GridUnit gu = new GridUnit(pgu, currentTableRow, j, i);
+                GridUnit gu = new GridUnit(pgu, j, i);
                 row.set(columnIndex + j, gu);
                 cellRow[j] = gu;
             }
@@ -111,21 +112,30 @@ class FixedColRowGroupBuilder extends RowGroupBuilder {
     }
 
     /** {@inheritDoc} */
-    void startRow(TableRow tableRow) {
+    void startTableRow(TableRow tableRow) {
         currentTableRow = tableRow;
     }
 
     /** {@inheritDoc} */
-    void endRow(TableRow row) {
-        if (currentRowIndex > 0 && row.getBreakBefore() != Constants.EN_AUTO) {
-            row.attributeWarning("break-before ignored because of row spanning "
+    void endTableRow() {
+        assert currentTableRow != null;
+        if (currentRowIndex > 0 && currentTableRow.getBreakBefore() != Constants.EN_AUTO) {
+            currentTableRow.attributeWarning("break-before ignored because of row spanning "
                     + "in progress (See XSL 1.1, 7.20.2)");
         }
-        if (currentRowIndex < rows.size() - 1 && row.getBreakAfter() != Constants.EN_AUTO) {
-            row.attributeWarning("break-after ignored because of row spanning "
+        if (currentRowIndex < rows.size() - 1
+                && currentTableRow.getBreakAfter() != Constants.EN_AUTO) {
+            currentTableRow.attributeWarning("break-after ignored because of row spanning "
                     + "in progress (See XSL 1.1, 7.20.1)");
         }
-        handleRowEnd(row);
+        for (Iterator iter = ((List) rows.get(currentRowIndex)).iterator(); iter.hasNext();) {
+            GridUnit gu = (GridUnit) iter.next();
+            // The row hasn't been filled with empty grid units yet
+            if (gu != null) {
+                gu.setRow(currentTableRow);
+            }
+        }
+        handleRowEnd(currentTableRow);
     }
 
     /** {@inheritDoc} */
@@ -174,7 +184,7 @@ class FixedColRowGroupBuilder extends RowGroupBuilder {
     }
 
     /** {@inheritDoc} */
-    void endTable(TableBody lastTablePart) {
+    void endTable() {
         borderResolver.endTable();
     }
 }
diff --git a/src/java/org/apache/fop/fo/flow/table/GridUnit.java b/src/java/org/apache/fop/fo/flow/table/GridUnit.java
index edf0c99ea..23d1cc001 100644
--- a/src/java/org/apache/fop/fo/flow/table/GridUnit.java
+++ b/src/java/org/apache/fop/fo/flow/table/GridUnit.java
@@ -76,12 +76,11 @@ public class GridUnit {
      * Creates a new grid unit.
      * 
      * @param table the containing table
-     * @param row the table-row element this grid unit belongs to (if any)
      * @param colSpanIndex index of this grid unit in the span, in column direction
      * @param rowSpanIndex index of this grid unit in the span, in row direction
      */
-    protected GridUnit(Table table, TableRow row, int colSpanIndex, int rowSpanIndex) {
-        this(row, colSpanIndex, rowSpanIndex);
+    protected GridUnit(Table table, int colSpanIndex, int rowSpanIndex) {
+        this(colSpanIndex, rowSpanIndex);
         setBorders(table);
     }
 
@@ -89,12 +88,11 @@ public class GridUnit {
      * Creates a new grid unit.
      * 
      * @param cell table cell which occupies this grid unit
-     * @param row the table-row element this grid unit belongs to (if any)
      * @param colSpanIndex index of this grid unit in the span, in column direction
      * @param rowSpanIndex index of this grid unit in the span, in row direction
      */
-    protected GridUnit(TableCell cell, TableRow row, int colSpanIndex, int rowSpanIndex) {
-        this(row, colSpanIndex, rowSpanIndex);
+    protected GridUnit(TableCell cell, int colSpanIndex, int rowSpanIndex) {
+        this(colSpanIndex, rowSpanIndex);
         this.cell = cell;
         setBorders(cell.getTable());
     }
@@ -103,17 +101,15 @@ public class GridUnit {
      * Creates a new grid unit.
      * 
      * @param primary the before-start grid unit of the cell containing this grid unit
-     * @param row the table-row element this grid unit belongs to (if any)
      * @param colSpanIndex index of this grid unit in the span, in column direction
      * @param rowSpanIndex index of this grid unit in the span, in row direction
      */
-    GridUnit(PrimaryGridUnit primary, TableRow row, int colSpanIndex, int rowSpanIndex) {
-        this(primary.getCell(), row, colSpanIndex, rowSpanIndex);
+    GridUnit(PrimaryGridUnit primary, int colSpanIndex, int rowSpanIndex) {
+        this(primary.getCell(), colSpanIndex, rowSpanIndex);
         this.primary = primary;
     }
 
-    private GridUnit(TableRow row, int colSpanIndex, int rowSpanIndex) {
-        this.row = row;
+    private GridUnit(int colSpanIndex, int rowSpanIndex) {
         this.colSpanIndex = colSpanIndex;
         this.rowSpanIndex = rowSpanIndex;
     }
@@ -165,6 +161,10 @@ public class GridUnit {
         return row;
     }
 
+    void setRow(TableRow row) {
+        this.row = row;
+    }
+
     public TableBody getBody() {
         FONode node = getCell();
         while (node != null && !(node instanceof TableBody)) {
diff --git a/src/java/org/apache/fop/fo/flow/table/PrimaryGridUnit.java b/src/java/org/apache/fop/fo/flow/table/PrimaryGridUnit.java
index c95f3f8c3..1a47a7dcf 100644
--- a/src/java/org/apache/fop/fo/flow/table/PrimaryGridUnit.java
+++ b/src/java/org/apache/fop/fo/flow/table/PrimaryGridUnit.java
@@ -52,6 +52,8 @@ public class PrimaryGridUnit extends GridUnit {
     private boolean isSeparateBorderModel;
     private int halfBorderSeparationBPD;
 
+    private boolean keepWithPrevious;
+    private boolean keepWithNext;
     private int breakBefore = Constants.EN_AUTO;
     private int breakAfter = Constants.EN_AUTO;
 
@@ -59,11 +61,10 @@ public class PrimaryGridUnit extends GridUnit {
      * Creates a new primary grid unit.
      *
      * @param cell table cell which occupies this grid unit
-     * @param row the table-row element this grid unit belongs to (if any)
      * @param colIndex index of the column this grid unit belongs to, zero-based
      */
-    PrimaryGridUnit(TableCell cell, TableRow row, int colIndex) {
-        super(cell, row, 0, 0);
+    PrimaryGridUnit(TableCell cell, int colIndex) {
+        super(cell, 0, 0);
         this.colIndex = colIndex;
         this.isSeparateBorderModel = cell.getTable().isSeparateBorderModel(); // TODO
         this.halfBorderSeparationBPD = cell.getTable().getBorderSeparation().getBPD().getLength()
@@ -325,6 +326,40 @@ public class PrimaryGridUnit extends GridUnit {
     }
 
     /**
+     * Returns true if the first child block (or its descendants) of this cell has
+     * keep-with-previous.
+     * 
+     * @return the value of keep-with-previous
+     */
+    public boolean mustKeepWithPrevious() {
+        return keepWithPrevious;
+    }
+
+    /**
+     * Don't use, reserved for TableCellLM. TODO
+     */
+    public void setKeepWithPrevious() {
+        this.keepWithPrevious = true;
+    }
+
+    /**
+     * Returns true if the last child block (or its descendants) of this cell has
+     * keep-with-next.
+     * 
+     * @return the value of keep-with-next
+     */
+    public boolean mustKeepWithNext() {
+        return keepWithNext;
+    }
+
+    /**
+     * Don't use, reserved for TableCellLM. TODO
+     */
+    public void setKeepWithNext() {
+        this.keepWithNext = true;
+    }
+
+    /**
      * Returns the class of the before break for the first child element of this cell.
      * 
      * @return one of {@link Constants#EN_AUTO}, {@link Constants#EN_COLUMN}, {@link
diff --git a/src/java/org/apache/fop/fo/flow/table/RowGroupBuilder.java b/src/java/org/apache/fop/fo/flow/table/RowGroupBuilder.java
index 3f7549787..c954be711 100644
--- a/src/java/org/apache/fop/fo/flow/table/RowGroupBuilder.java
+++ b/src/java/org/apache/fop/fo/flow/table/RowGroupBuilder.java
@@ -53,16 +53,14 @@ abstract class RowGroupBuilder {
      * 
      * @param tableRow the row being started
      */
-    abstract void startRow(TableRow tableRow);
+    abstract void startTableRow(TableRow tableRow);
 
     /**
      * Receives notification of the end of the current row. If the current row finishes
      * the row group, the {@link TableBody#addRowGroup(List)} method of the parent table
      * part will be called.
-     * 
-     * @param row the row being finished
      */
-    abstract void endRow(TableRow row);
+    abstract void endTableRow();
 
     /**
      * Receives notification of the end of the current row, when the source contains no
@@ -70,7 +68,7 @@ abstract class RowGroupBuilder {
      * {@link TableBody#addRowGroup(List)} method of the given table part will be called.
      * 
      * <p>If the source does contain explicit fo:table-row elements, then the
-     * {@link #endRow(TableRow)} method will be called instead.</p>
+     * {@link #endTableRow()} method will be called instead.</p>
      * 
      * @param part the part containing the current row
      */
@@ -95,8 +93,7 @@ abstract class RowGroupBuilder {
     /**
      * Receives notification of the end of the table.
      * 
-     * @param lastTablePart the last part of the table
      * @throws ValidationException if a row-spanning cell overflows one of the table's parts
      */
-    abstract void endTable(TableBody lastTablePart) throws ValidationException;
+    abstract void endTable() throws ValidationException;
 }
diff --git a/src/java/org/apache/fop/fo/flow/table/Table.java b/src/java/org/apache/fop/fo/flow/table/Table.java
index 2b6570dcc..7d6611435 100644
--- a/src/java/org/apache/fop/fo/flow/table/Table.java
+++ b/src/java/org/apache/fop/fo/flow/table/Table.java
@@ -126,8 +126,8 @@ public class Table extends TableFObj implements ColumnNumberManagerHolder {
         orphanContentLimit = pList.get(PR_X_ORPHAN_CONTENT_LIMIT).getLength();
 
         if (!blockProgressionDimension.getOptimum(null).isAuto()) {
-            attributeWarning("only a value of \"auto\" for block-progression-dimension has a well-specified"
-                    + " behavior on fo:table. Falling back to \"auto\"");
+            attributeWarning("only a value of \"auto\" for block-progression-dimension has a"
+                    + " well-specified behavior on fo:table. Falling back to \"auto\"");
             // Anyway, the bpd of a table is not used by the layout code
         }
         if (tableLayout == EN_AUTO) {
@@ -226,11 +226,7 @@ public class Table extends TableFObj implements ColumnNumberManagerHolder {
                        + ",table-body+)");
         }
         if (!inMarker()) {
-            if (tableFooter != null) {
-                rowGroupBuilder.endTable(tableFooter);
-            } else {
-                rowGroupBuilder.endTable((TableBody) getChildNodes().lastNode());
-            }
+            rowGroupBuilder.endTable();
             /* clean up */
             for (int i = columns.size(); --i >= 0;) {
                 TableColumn col = (TableColumn) columns.get(i);
@@ -290,6 +286,7 @@ public class Table extends TableFObj implements ColumnNumberManagerHolder {
         }
     }
 
+    /** {@inheritDoc} */
     protected void setCollapsedBorders() {
         createBorder(CommonBorderPaddingBackground.START);
         createBorder(CommonBorderPaddingBackground.END);
diff --git a/src/java/org/apache/fop/fo/flow/table/TableBody.java b/src/java/org/apache/fop/fo/flow/table/TableBody.java
index c3642c260..de7bfda84 100644
--- a/src/java/org/apache/fop/fo/flow/table/TableBody.java
+++ b/src/java/org/apache/fop/fo/flow/table/TableBody.java
@@ -140,7 +140,7 @@ public class TableBody extends TableCellContainer {
         if (!inMarker()) {
             RowGroupBuilder rowGroupBuilder = getTable().getRowGroupBuilder();
             if (tableRowsFound) {
-                rowGroupBuilder.endRow(lastRow);
+                rowGroupBuilder.endTableRow();
             } else if (!lastCellEndsRow) {
                 rowGroupBuilder.endRow(this);
             }
@@ -167,9 +167,9 @@ public class TableBody extends TableCellContainer {
             } else if (localName.equals("table-row")) {
                 tableRowsFound = true;
                 if (tableCellsFound) {
-                    invalidChildError(loc, nsURI, localName, "Either fo:table-rows" +
-                      " or fo:table-cells may be children of an " + getName() +
-                      " but not both");
+                    invalidChildError(loc, nsURI, localName, "Either fo:table-rows"
+                            + " or fo:table-cells may be children of an " + getName()
+                            + " but not both");
                 }
             } else if (localName.equals("table-cell")) {
                 tableCellsFound = true;
@@ -198,11 +198,11 @@ public class TableBody extends TableCellContainer {
                     getTable().getRowGroupBuilder().startTablePart(this);
                 } else {
                     columnNumberManager.prepareForNextRow(pendingSpans);
-                    getTable().getRowGroupBuilder().endRow(lastRow);
+                    getTable().getRowGroupBuilder().endTableRow();
                 }
                 rowsStarted = true;
                 lastRow = (TableRow) child;
-                getTable().getRowGroupBuilder().startRow(lastRow);
+                getTable().getRowGroupBuilder().startTableRow(lastRow);
                 break;
             case FO_TABLE_CELL:
                 if (!rowsStarted) {
diff --git a/src/java/org/apache/fop/fo/flow/table/VariableColRowGroupBuilder.java b/src/java/org/apache/fop/fo/flow/table/VariableColRowGroupBuilder.java
index 801153ce9..d59870f0a 100644
--- a/src/java/org/apache/fop/fo/flow/table/VariableColRowGroupBuilder.java
+++ b/src/java/org/apache/fop/fo/flow/table/VariableColRowGroupBuilder.java
@@ -64,19 +64,19 @@ class VariableColRowGroupBuilder extends RowGroupBuilder {
     }
 
     /** {@inheritDoc} */
-    void startRow(final TableRow tableRow) {
+    void startTableRow(final TableRow tableRow) {
         events.add(new Event() {
             public void play(RowGroupBuilder rowGroupBuilder) {
-                rowGroupBuilder.startRow(tableRow);
+                rowGroupBuilder.startTableRow(tableRow);
             }
         });
     }
 
     /** {@inheritDoc} */
-    void endRow(final TableRow row) {
+    void endTableRow() {
         events.add(new Event() {
             public void play(RowGroupBuilder rowGroupBuilder) {
-                rowGroupBuilder.endRow(row);
+                rowGroupBuilder.endTableRow();
             }
         });
     }
@@ -110,11 +110,11 @@ class VariableColRowGroupBuilder extends RowGroupBuilder {
     }
 
     /** {@inheritDoc} */
-    void endTable(final TableBody lastTablePart) throws ValidationException {
+    void endTable() throws ValidationException {
         RowGroupBuilder delegate = new FixedColRowGroupBuilder(table);
         for (Iterator eventIter = events.iterator(); eventIter.hasNext();) {
             ((Event) eventIter.next()).play(delegate);
         }
-        delegate.endTable(lastTablePart);
+        delegate.endTable();
     }
 }