Bugzilla #38397:

Bugfix: Spanned cells could lead to an false error message about overlapping cells and ultimately a NullPointerException.

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

src/java/org/apache/fop/layoutmgr/table/TableRowIterator.java

@@ -355,6 +355,9 @@ public class TableRowIterator {
                 colnum++;
             }
         }
+        if (pendingRowSpans < 0) {
+            throw new IllegalStateException("pendingRowSpans must not become negative!");
+        }
         
         //Transfer available cells to their slots
         colnum = 1;
@@ -368,9 +371,15 @@ public class TableRowIterator {
             //shouldn't happen here, since
             //overlapping cells already caught in 
             //fo.flow.TableCell.bind()...
-            if (safelyGetListItem(gridUnits, colnum - 1) != null) {
-                log.error("Overlapping cell at position " + colnum);
-                //TODO throw layout exception
+            GridUnit other = (GridUnit)safelyGetListItem(gridUnits, colnum - 1); 
+            if (other != null) {
+                String err = "A table-cell (" 
+                        + cell.getContextInfo() 
+                        + ") is overlapping with another (" 
+                        + other.getCell().getContextInfo() 
+                        + ") in column " + colnum;
+                throw new IllegalStateException(err 
+                        + " (this should have been catched by FO tree validation)");
             }
             TableColumn col = columns.getColumn(colnum);
 
@@ -390,12 +399,20 @@ public class TableRowIterator {
                 for (int j = 1; j < cell.getNumberColumnsSpanned(); j++) {
                     colnum++;
                     GridUnit guSpan = new GridUnit(gu, columns.getColumn(colnum), colnum - 1, j);
-                    if (safelyGetListItem(gridUnits, colnum - 1) != null) {
-                        log.error("Overlapping cell at position " + colnum);
-                        //TODO throw layout exception
+                    //TODO: remove the check below???
+                    other = (GridUnit)safelyGetListItem(gridUnits, colnum - 1); 
+                    if (other != null) {
+                        String err = "A table-cell (" 
+                            + cell.getContextInfo() 
+                            + ") is overlapping with another (" 
+                            + other.getCell().getContextInfo() 
+                            + ") in column " + colnum;
+                        throw new IllegalStateException(err 
+                            + " (this should have been catched by FO tree validation)");
                     }
                     safelySetListItem(gridUnits, colnum - 1, guSpan);
                     if (hasRowSpanningLeft) {
+                        pendingRowSpans++;
                         safelySetListItem(lastRowsSpanningCells, colnum - 1, gu);
                     }
                     horzSpan[j] = guSpan;

status.xml

@@ -27,6 +27,10 @@
 
   <changes>
     <release version="FOP Trunk">
+      <action context="Code" dev="JM" type="fix" fixes-bug="38397">
+        Bugfix: Spanned cells could lead to an false error message about overlapping 
+        cells and ultimately a NullPointerException.
+      </action>
       <action context="Code" dev="JM" type="fix">
         Bugfix: Regions with non-standard names got ignored in RTF output leading to 
         missing headers and footers.

test/layoutengine/standard-testcases/table-cell_number-rows-spanned_bug38397.xml

@@ -0,0 +1,97 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Copyright 2006 The Apache Software Foundation
+
+  Licensed 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$ -->
+<testcase>
+  <info>
+    <p>
+      This test checks a bug with column number assignement in concert with row spanning. With the bug present,
+      the code produces an NPE in PrimaryGridUnit.getStartEndBorderWidths() as a follow-up problem.
+    </p>
+  </info>
+  <fo>
+    <fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format" xmlns:svg="http://www.w3.org/2000/svg">
+      <fo:layout-master-set>
+        <fo:simple-page-master master-name="normal" page-width="5in" page-height="5in">
+          <fo:region-body/>
+        </fo:simple-page-master>
+      </fo:layout-master-set>
+      <fo:page-sequence master-reference="normal" white-space-collapse="true">
+        <fo:flow flow-name="xsl-region-body">
+          <fo:table table-layout="fixed" width="100%" border-collapse="separate">
+            <fo:table-column column-width="proportional-column-width(1)"/>
+            <fo:table-column column-width="proportional-column-width(1)"/>
+            <fo:table-column column-width="proportional-column-width(2)"/>
+            <fo:table-column column-width="proportional-column-width(2)"/>
+            <fo:table-body>
+              <fo:table-row>
+                <fo:table-cell number-rows-spanned="3" number-columns-spanned="2" display-align="center" border="solid 0.5pt">
+                  <fo:block>cell1</fo:block>
+                </fo:table-cell>
+                <fo:table-cell number-rows-spanned="2" display-align="center" border="solid 0.5pt">
+                  <fo:block>cell2</fo:block>
+                </fo:table-cell>
+                <fo:table-cell border="solid 0.5pt">
+                  <fo:block>cell3</fo:block>
+                </fo:table-cell>
+              </fo:table-row>
+              <fo:table-row background-color="yellow">
+                <fo:table-cell border="solid 0.5pt">
+                  <fo:block>cell4</fo:block>
+                </fo:table-cell>
+              </fo:table-row>
+              <fo:table-row>
+                <fo:table-cell number-rows-spanned="2" display-align="center" border="solid 0.5pt">
+                  <fo:block>cell5</fo:block>
+                </fo:table-cell>
+                <fo:table-cell border="solid 0.5pt">
+                  <fo:block>cell6</fo:block>
+                </fo:table-cell>
+              </fo:table-row>
+              <fo:table-row background-color="yellow">
+                <fo:table-cell number-rows-spanned="3" display-align="center" border="solid 0.5pt">
+                  <fo:block>cell7</fo:block>
+                </fo:table-cell>
+                <fo:table-cell number-rows-spanned="3" display-align="center" border="solid 0.5pt">
+                  <fo:block>cell8</fo:block>
+                </fo:table-cell>
+                <fo:table-cell border="solid 0.5pt">
+                  <fo:block>cell9</fo:block>
+                </fo:table-cell>
+              </fo:table-row>
+              <fo:table-row>
+                <fo:table-cell number-rows-spanned="2" display-align="center" border="solid 0.5pt">
+                  <fo:block>cell10</fo:block>
+                </fo:table-cell>
+                <fo:table-cell border="solid 0.5pt">
+                  <fo:block>cell11</fo:block>
+                </fo:table-cell>
+              </fo:table-row>
+              <fo:table-row background-color="yellow">
+                <fo:table-cell border="solid 0.5pt">
+                  <fo:block>cell12</fo:block>
+                </fo:table-cell>
+              </fo:table-row>
+            </fo:table-body>
+          </fo:table>
+        </fo:flow>
+      </fo:page-sequence>
+    </fo:root>
+  </fo>
+  <checks>
+    <eval expected="1" xpath="count(//pageViewport)"/>
+  </checks>
+</testcase>