Added check for table-cells which span more rows than available in their parent element

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

src/java/org/apache/fop/fo/flow/table/RowGroupBuilder.java

@@ -22,6 +22,7 @@ package org.apache.fop.fo.flow.table;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.fop.fo.ValidationException;
 import org.apache.fop.layoutmgr.table.GridUnit;
 import org.apache.fop.layoutmgr.table.PrimaryGridUnit;
 
@@ -113,19 +114,18 @@ abstract class RowGroupBuilder {
     }
 
     /**
-     * Finishes and records the last row-group of the given table-body, if any. If there
-     * is no fo:table-row and the last cell of the table-body didn't have ends-row="true",
-     * then the {@link signalNewRow} method has not been called and the last row group has
-     * yet to be recorded.
+     * Signals that the end of a table-header/footer/body has been reached. The current
+     * row-group is checked for emptiness. This row group builder is reset for handling
+     * further possible table parts.
      * 
-     * @param tableBody
+     * @param tableBody the table part being finished
+     * @throws ValidationException if a cell is spanning further than the given table part
      */
-    void finishLastRowGroup(TableBody tableBody) {
+    void signalEndOfPart(TableBody tableBody) throws ValidationException {
         if (rows.size() > 0) {
-            tableBody.addRowGroup(rows);
+            throw new ValidationException(
+                    "A table-cell is spanning more rows than available in its parent element.");
         }
-        // Reset, in case this rowGroupBuilder is re-used for other
-        // table-header/footer/body
         initialize();
     }
 

src/java/org/apache/fop/fo/flow/table/TableBody.java

@@ -57,6 +57,8 @@ public class TableBody extends TableCellContainer {
 
     private boolean rowsStarted = false;
 
+    private boolean lastCellEndsRow = true;
+
     private List rowGroups = new LinkedList();
 
     /**
@@ -124,7 +126,20 @@ public class TableBody extends TableCellContainer {
                 getParent().removeChild(this);
             }
         } else {
-            getTable().getRowGroupBuilder().finishLastRowGroup(this);
+            finishLastRowGroup();
+        }
+    }
+
+    protected void finishLastRowGroup() throws ValidationException {
+        RowGroupBuilder rowGroupBuilder = getTable().getRowGroupBuilder(); 
+        if (tableRowsFound || !lastCellEndsRow) {
+            rowGroupBuilder.signalRowEnd(this);
+        }
+        try {
+            rowGroupBuilder.signalEndOfPart(this);
+        } catch (ValidationException e) {
+            e.setLocator(locator);
+            throw e;
         }
     }
 
@@ -179,7 +194,8 @@ public class TableBody extends TableCellContainer {
                 rowsStarted = true;
                 TableCell cell = (TableCell) child;
                 addTableCellChild(cell, firstRow);
-                if (cell.endsRow()) {
+                lastCellEndsRow = cell.endsRow(); 
+                if (lastCellEndsRow) {
                     firstRow = false;
                     columnNumberManager.prepareForNextRow(pendingSpans);
                     getTable().getRowGroupBuilder().signalRowEnd(this);
@@ -231,8 +247,7 @@ public class TableBody extends TableCellContainer {
     void signalNewRow() {
         if (rowsStarted) {
             firstRow = false;
-            TableCell previousCell = (TableCell) getChildNodes().lastNode();
-            if (!previousCell.endsRow()) {
+            if (!lastCellEndsRow) {
                 columnNumberManager.prepareForNextRow(pendingSpans);
                 getTable().getRowGroupBuilder().signalRowEnd(this);
             }

src/java/org/apache/fop/fo/flow/table/TableFooter.java

@@ -51,7 +51,7 @@ public class TableFooter extends TableBody {
         if (!(tableRowsFound || tableCellsFound)) {
             missingChildElementError("marker* (table-row+|table-cell+)");
         } else {
-            getTable().getRowGroupBuilder().finishLastRowGroup(this);
+            finishLastRowGroup();
         }
 //      convertCellsToRows();
     }

src/java/org/apache/fop/fo/flow/table/TableHeader.java

@@ -51,7 +51,7 @@ public class TableHeader extends TableBody {
         if (!(tableRowsFound || tableCellsFound)) {
             missingChildElementError("marker* (table-row+|table-cell+)");
         } else {
-            getTable().getRowGroupBuilder().finishLastRowGroup(this);
+            finishLastRowGroup();
         }
 //      convertCellsToRows();
     }

status.xml

@@ -28,6 +28,10 @@
 
   <changes>
     <release version="FOP Trunk">
+      <action context="Code" dev="VH" type="add">
+        Added check for table-cells which span more rows than available in their parent element
+        (table-header/footer/body).
+      </action>
       <action context="Code" dev="AD" type="add">
         Added support for fo:markers in fo:inline and fo:basic-link.
       </action>

test/fotree/unittests/table/illegal-row-span_body_1.fo

@@ -0,0 +1,42 @@
+<?xml version="1.0" standalone="no"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<!-- $Id$ -->
+<fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+  <fo:layout-master-set>
+    <fo:simple-page-master master-name="page" page-height="10cm" page-width="15cm" margin="10pt">
+      <fo:region-body/>
+    </fo:simple-page-master>
+  </fo:layout-master-set>
+  <fo:page-sequence master-reference="page">
+    <fo:flow flow-name="xsl-region-body">
+      <fo:block space-after="10pt">The following table has a cell which spans further than its
+        containing table-body.</fo:block>
+      <fo:table table-layout="fixed" border-collapse="separate" width="100%">
+        <fo:table-column number-columns-repeated="2" column-width="proportional-column-width(1)"/>
+        <fo:table-body>
+          <fo:table-cell border="1pt solid black" number-rows-spanned="2">
+            <fo:block>Cell 1.1</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Cell 1.2</fo:block>
+          </fo:table-cell>
+        </fo:table-body>
+      </fo:table>
+    </fo:flow>
+  </fo:page-sequence>
+</fo:root>

test/fotree/unittests/table/illegal-row-span_body_2.fo

@@ -0,0 +1,66 @@
+<?xml version="1.0" standalone="no"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<!-- $Id$ -->
+<fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+  <fo:layout-master-set>
+    <fo:simple-page-master master-name="page" page-height="10cm" page-width="15cm" margin="10pt">
+      <fo:region-body/>
+    </fo:simple-page-master>
+  </fo:layout-master-set>
+  <fo:page-sequence master-reference="page">
+    <fo:flow flow-name="xsl-region-body">
+      <fo:block space-after="10pt">The following table has a cell which spans further than its
+        containing table-body.</fo:block>
+      <fo:table table-layout="fixed" border-collapse="separate" width="100%">
+        <fo:table-column number-columns-repeated="2" column-width="proportional-column-width(1)"/>
+        <fo:table-body>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Cell 1.1</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black" ends-row="true">
+            <fo:block>Cell 1.2</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black" number-rows-spanned="2">
+            <fo:block>Cell 2.1</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Cell 2.2</fo:block>
+          </fo:table-cell>
+        </fo:table-body>
+        <fo:table-body>
+          <fo:table-row>
+            <fo:table-cell border="1pt solid blue">
+              <fo:block>Cell 3.1</fo:block>
+            </fo:table-cell>
+            <fo:table-cell border="1pt solid blue">
+              <fo:block>Cell 3.2</fo:block>
+            </fo:table-cell>
+          </fo:table-row>
+          <fo:table-row>
+            <fo:table-cell border="1pt solid blue">
+              <fo:block>Cell 4.1</fo:block>
+            </fo:table-cell>
+            <fo:table-cell border="1pt solid blue">
+              <fo:block>Cell 4.2</fo:block>
+            </fo:table-cell>
+          </fo:table-row>
+        </fo:table-body>
+      </fo:table>
+    </fo:flow>
+  </fo:page-sequence>
+</fo:root>

test/fotree/unittests/table/illegal-row-span_footer.fo

@@ -0,0 +1,75 @@
+<?xml version="1.0" standalone="no"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<!-- $Id$ -->
+<fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+  <fo:layout-master-set>
+    <fo:simple-page-master master-name="page" page-height="10cm" page-width="15cm" margin="10pt">
+      <fo:region-body/>
+    </fo:simple-page-master>
+  </fo:layout-master-set>
+  <fo:page-sequence master-reference="page">
+    <fo:flow flow-name="xsl-region-body">
+      <fo:block space-after="10pt">The following table has a cell which spans further than its
+        containing table-footer.</fo:block>
+      <fo:table table-layout="fixed" border-collapse="separate" width="100%">
+        <fo:table-column number-columns-repeated="2" column-width="proportional-column-width(1)"/>
+        <fo:table-header>
+          <fo:table-row>
+            <fo:table-cell border="1pt solid black">
+              <fo:block>Header 1.1</fo:block>
+            </fo:table-cell>
+            <fo:table-cell border="1pt solid black">
+              <fo:block>Header 1.2</fo:block>
+            </fo:table-cell>
+          </fo:table-row>
+        </fo:table-header>
+        <fo:table-footer>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Footer 1.1</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black" ends-row="true">
+            <fo:block>Footer 1.2</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Footer 2.1</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black" number-rows-spanned="3">
+            <fo:block>Footer 2.2</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black" starts-row="true">
+            <fo:block>Footer 3.1</fo:block>
+          </fo:table-cell>
+        </fo:table-footer>
+        <fo:table-body>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Cell 1.1</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black" ends-row="true">
+            <fo:block>Cell 1.2</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Cell 2.1</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Cell 2.2</fo:block>
+          </fo:table-cell>
+        </fo:table-body>
+      </fo:table>
+    </fo:flow>
+  </fo:page-sequence>
+</fo:root>

test/fotree/unittests/table/illegal-row-span_header.fo

@@ -0,0 +1,81 @@
+<?xml version="1.0" standalone="no"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one or more
+  contributor license agreements.  See the NOTICE file distributed with
+  this work for additional information regarding copyright ownership.
+  The ASF licenses this file to You under the Apache License, Version 2.0
+  (the "License"); you may not use this file except in compliance with
+  the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+-->
+<!-- $Id$ -->
+<fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+  <fo:layout-master-set>
+    <fo:simple-page-master master-name="page" page-height="10cm" page-width="15cm" margin="10pt">
+      <fo:region-body/>
+    </fo:simple-page-master>
+  </fo:layout-master-set>
+  <fo:page-sequence master-reference="page">
+    <fo:flow flow-name="xsl-region-body">
+      <fo:block space-after="10pt">The following table has a cell which spans further than its
+        containing table-header.</fo:block>
+      <fo:table table-layout="fixed" border-collapse="separate" width="100%">
+        <fo:table-column number-columns-repeated="2" column-width="proportional-column-width(1)"/>
+        <fo:table-header>
+          <fo:table-row>
+            <fo:table-cell border="1pt solid black" number-rows-spanned="3">
+              <fo:block>Header 1.1</fo:block>
+            </fo:table-cell>
+            <fo:table-cell border="1pt solid black">
+              <fo:block>Header 1.2</fo:block>
+            </fo:table-cell>
+          </fo:table-row>
+          <fo:table-row>
+            <fo:table-cell border="1pt solid black">
+              <fo:block>Header 2.2</fo:block>
+            </fo:table-cell>
+          </fo:table-row>
+        </fo:table-header>
+        <fo:table-body>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Cell 1.1</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black" ends-row="true">
+            <fo:block>Cell 1.2</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Cell 2.1</fo:block>
+          </fo:table-cell>
+          <fo:table-cell border="1pt solid black">
+            <fo:block>Cell 2.2</fo:block>
+          </fo:table-cell>
+        </fo:table-body>
+        <fo:table-body>
+          <fo:table-row>
+            <fo:table-cell border="1pt solid blue">
+              <fo:block>Cell 3.1</fo:block>
+            </fo:table-cell>
+            <fo:table-cell border="1pt solid blue">
+              <fo:block>Cell 3.2</fo:block>
+            </fo:table-cell>
+          </fo:table-row>
+          <fo:table-row>
+            <fo:table-cell border="1pt solid blue">
+              <fo:block>Cell 4.1</fo:block>
+            </fo:table-cell>
+            <fo:table-cell border="1pt solid blue">
+              <fo:block>Cell 4.2</fo:block>
+            </fo:table-cell>
+          </fo:table-row>
+        </fo:table-body>
+      </fo:table>
+    </fo:flow>
+  </fo:page-sequence>
+</fo:root>

test/java/org/apache/fop/fo/flow/table/ErrorCheckTestCase.java

@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* $Id$ */
+
+package org.apache.fop.fo.flow.table;
+
+import org.apache.fop.fo.ValidationException;
+
+/**
+ * Abstract class for testing erroneous files, checking that a ValidationException is thrown.
+ */
+abstract class ErrorCheckTestCase extends AbstractTableTestCase {
+
+    public ErrorCheckTestCase() throws Exception {
+        super();
+    }
+
+    protected void launchTest(String filename) throws Exception {
+        try {
+            setUp(filename);
+            fail();
+        } catch (ValidationException e) {
+            // TODO check location
+        }
+    }
+
+}

test/java/org/apache/fop/fo/flow/table/IllegalRowSpanTestCase.java

@@ -0,0 +1,48 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* $Id$ */
+
+package org.apache.fop.fo.flow.table;
+
+/**
+ * Testcase checking that cells spanning further than their parent element aren't
+ * accepted.
+ */
+public class IllegalRowSpanTestCase extends ErrorCheckTestCase {
+
+    public IllegalRowSpanTestCase() throws Exception {
+        super();
+    }
+
+    public void testBody1() throws Exception {
+        launchTest("table/illegal-row-span_body_1.fo");
+    }
+
+    public void testBody2() throws Exception {
+        launchTest("table/illegal-row-span_body_2.fo");
+    }
+
+    public void testHeader() throws Exception {
+        launchTest("table/illegal-row-span_header.fo");
+    }
+
+    public void testFooter() throws Exception {
+        launchTest("table/illegal-row-span_footer.fo");
+    }
+
+}

test/java/org/apache/fop/fo/flow/table/TooManyColumnsTestCase.java

@@ -19,23 +19,13 @@
 
 package org.apache.fop.fo.flow.table;
 
-import org.apache.fop.fo.ValidationException;
 
-public class TooManyColumnsTestCase extends AbstractTableTestCase {
+public class TooManyColumnsTestCase extends ErrorCheckTestCase {
 
     public TooManyColumnsTestCase() throws Exception {
         super();
     }
 
-    private void launchTest(String filename) throws Exception {
-        try {
-            setUp(filename);
-            fail();
-        } catch (ValidationException e) {
-            // TODO check location
-        }
-    }
-
     public void testBody1() throws Exception {
         launchTest("table/too-many-columns_body_1.fo");
     }
@@ -48,6 +38,10 @@ public class TooManyColumnsTestCase extends AbstractTableTestCase {
         launchTest("table/too-many-columns_body_3.fo");
     }
 
+    public void testBody4() throws Exception {
+        launchTest("table/too-many-columns_body_4.fo");
+    }
+
     public void testHeader() throws Exception {
         launchTest("table/too-many-columns_header.fo");
     }