Bugfix: Areas for table-cells that are broken over more than one page are now generated even if all its content is already painted on a previous page. This fixes strange effects like a table grid that is not completely painted.

Some check in the test cases had to be adjusted because the indices weren't correct anymore, since now some previously missing areas are generated.

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

6 files changed, 41 insertions, 18 deletions

diff --git a/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java b/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java
index 9fabeccc5..9b2d8e0f2 100644
--- a/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java
+++ b/src/java/org/apache/fop/layoutmgr/table/TableContentLayoutManager.java
@@ -762,6 +762,7 @@ public class TableContentLayoutManager implements PercentBaseContext {
         public RowPainter(LayoutContext layoutContext) {
             this.layoutContext = layoutContext;
             Arrays.fill(firstRow, -1);
+            Arrays.fill(end, -1);
         }
         
         public int getAccumulatedBPD() {
@@ -898,7 +899,8 @@ public class TableContentLayoutManager implements PercentBaseContext {
                 GridUnit currentGU = lastRow.safelyGetGridUnit(i);
                 if ((gridUnits[i] != null) 
                         && (forcedFlush || ((end[i] == gridUnits[i].getElements().size() - 1))
-                                && (currentGU == null || currentGU.isLastGridUnitRowSpan()))) {
+                                && (currentGU == null || currentGU.isLastGridUnitRowSpan()))
+                    || (gridUnits[i] == null && currentGU != null)) {
                     //the last line in the "if" above is to avoid a premature end of an 
                     //row-spanned cell because no GridUnitParts are generated after a cell is
                     //finished with its content. currentGU can be null if there's no grid unit
@@ -907,13 +909,22 @@ public class TableContentLayoutManager implements PercentBaseContext {
                         log.debug((forcedFlush ? "FORCED " : "") + "flushing..." + i + " " 
                                 + start[i] + "-" + end[i]);
                     }
-                    addAreasForCell(gridUnits[i], start[i], end[i], 
-                            lastRow,  
-                            partLength[i], actualRowHeight);
-                    gridUnits[i] = null;
-                    start[i] = 0;
-                    end[i] = 0;
-                    partLength[i] = 0;
+                    PrimaryGridUnit gu = gridUnits[i];
+                    if (gu == null 
+                            && !currentGU.isEmpty() 
+                            && currentGU.getColSpanIndex() == 0 
+                            && currentGU.isLastGridUnitColSpan()) {
+                        gu = currentGU.getPrimary();
+                    }
+                    if (gu != null) {
+                        addAreasForCell(gu, start[i], end[i], 
+                                lastRow,  
+                                partLength[i], actualRowHeight);
+                        gridUnits[i] = null;
+                        start[i] = 0;
+                        end[i] = -1;
+                        partLength[i] = 0;
+                    }
                 }
             }
             return actualRowHeight;
@@ -950,8 +961,10 @@ public class TableContentLayoutManager implements PercentBaseContext {
             cellLM.setRowHeight(effCellHeight);
             //cellLM.setRowHeight(row.getHeight().opt);
             int prevBreak = ElementListUtils.determinePreviousBreak(pgu.getElements(), startPos);
-            SpaceResolver.performConditionalsNotification(pgu.getElements(), 
-                    startPos, endPos, prevBreak);
+            if (endPos >= 0) {
+                SpaceResolver.performConditionalsNotification(pgu.getElements(), 
+                        startPos, endPos, prevBreak);
+            }
             cellLM.addAreas(new KnuthPossPosIter(pgu.getElements(), 
                     startPos, endPos + 1), layoutContext);
         }
diff --git a/status.xml b/status.xml
index 8f1584344..336490417 100644
--- a/status.xml
+++ b/status.xml
@@ -27,6 +27,11 @@
 
   <changes>
     <release version="FOP Trunk">
+      <action context="Code" dev="JM" type="fix">
+        Bugfix: Areas for table-cells that are broken over more than one page are now
+        generated even if all its content is already painted on a previous page. This
+        fixes strange effects like a table grid that is not completely painted.
+      </action>
       <action context="Code" dev="JM" type="fix" fixes-bug="37828">
         Bugfix: Column balancing produced strange break decisions in certain multi-column
         documents with large amounts of text.
diff --git a/test/layoutengine/disabled-testcases.xml b/test/layoutengine/disabled-testcases.xml
index 0fe5c0c14..33569e2f6 100755
--- a/test/layoutengine/disabled-testcases.xml
+++ b/test/layoutengine/disabled-testcases.xml
@@ -260,8 +260,7 @@
     <name>Problems with border and padding on tables</name>
     <file>table_border_padding.xml</file>
     <description>The element list seems to not be fully correct, yet, causing
-    the layout to look odd. Furthermore, an already finished cell doesn't produce
-    an area on the next page when the table-row is broken over multiple pages.</description>
+    the layout to look odd.</description>
   </testcase>
   <testcase>
     <name>keep-with-previous doesn't work inside tables</name>
diff --git a/test/layoutengine/standard-testcases/table-cell_height_page-break.xml b/test/layoutengine/standard-testcases/table-cell_height_page-break.xml
index e0adfa4d8..b27d30d7f 100644
--- a/test/layoutengine/standard-testcases/table-cell_height_page-break.xml
+++ b/test/layoutengine/standard-testcases/table-cell_height_page-break.xml
@@ -81,7 +81,7 @@
     <eval expected="cell4e" xpath="//pageViewport[1]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/block[4]/block[1]/block[5]/lineArea"/>
 
     <!-- page 2 -->
-    <eval expected="cell4f" xpath="//pageViewport[2]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/block[1]/block[1]/block[1]/lineArea"/>
-    <eval expected="cell4g" xpath="//pageViewport[2]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/block[1]/block[1]/block[2]/lineArea"/>
+    <eval expected="cell4f" xpath="//pageViewport[2]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/block[2]/block[1]/block[1]/lineArea"/>
+    <eval expected="cell4g" xpath="//pageViewport[2]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/block[2]/block[1]/block[2]/lineArea"/>
   </checks>
 </testcase>
diff --git a/test/layoutengine/standard-testcases/table_border_padding_2.xml b/test/layoutengine/standard-testcases/table_border_padding_2.xml
index 07b5cdc1c..84f7f91e9 100644
--- a/test/layoutengine/standard-testcases/table_border_padding_2.xml
+++ b/test/layoutengine/standard-testcases/table_border_padding_2.xml
@@ -39,7 +39,7 @@
             <fo:table-column column-width="proportional-column-width(9)"/>
             <fo:table-body>
               <fo:table-row>
-                <fo:table-cell background-color="yellow">
+                <fo:table-cell background-color="yellow" id="cell1a">
                   <fo:block>cell1</fo:block>
                 </fo:table-cell>
                 <fo:table-cell background-color="orange">
@@ -69,7 +69,7 @@ and threw it up on high and caught it; and this ball was her favorite plaything.
             <fo:table-column column-width="proportional-column-width(9)"/>
             <fo:table-body>
               <fo:table-row>
-                <fo:table-cell background-color="yellow">
+                <fo:table-cell background-color="yellow" id="cell1b">
                   <fo:block>cell1</fo:block>
                 </fo:table-cell>
                 <fo:table-cell background-color="orange">
@@ -199,5 +199,11 @@ and threw it up on high and caught it; and this ball was her favorite plaything.
 
     <eval expected="(solid,#000000,5000)" xpath="//pageViewport[@nr=4]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/@border-after"/>
     <eval expected="20000" xpath="//pageViewport[@nr=4]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/@padding-after"/>
+    
+    <!-- Additional check: areas for table-cells which are finished but its row continues on the subsequent page -->
+    <true xpath="boolean(//pageViewport[@nr=1]//block[@prod-id='cell1a'])"/>
+    <true xpath="boolean(//pageViewport[@nr=2]//block[@prod-id='cell1a'])"/>
+    <true xpath="boolean(//pageViewport[@nr=3]//block[@prod-id='cell1b'])"/>
+    <true xpath="boolean(//pageViewport[@nr=4]//block[@prod-id='cell1b'])"/>
   </checks>
 </testcase>
diff --git a/test/layoutengine/standard-testcases/table_space-before_space-after_2.xml b/test/layoutengine/standard-testcases/table_space-before_space-after_2.xml
index 310c1135a..2e136a5cd 100644
--- a/test/layoutengine/standard-testcases/table_space-before_space-after_2.xml
+++ b/test/layoutengine/standard-testcases/table_space-before_space-after_2.xml
@@ -97,8 +97,8 @@
     <eval expected="1" xpath="count(//pageViewport[@nr=2]/page/regionViewport/regionBody/mainReference/span/flow/block)"/>
     <eval expected="1" xpath="count(//pageViewport[@nr=3]/page/regionViewport/regionBody/mainReference/span/flow/block)"/>
     <eval expected="2" xpath="count(//pageViewport[@nr=1]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/block[2]/block)"/>
-    <eval expected="2" xpath="count(//pageViewport[@nr=2]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/block[1]/block)"/>
-    <eval expected="1" xpath="count(//pageViewport[@nr=3]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/block[1]/block)"/>
+    <eval expected="2" xpath="count(//pageViewport[@nr=2]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/block[2]/block)"/>
+    <eval expected="1" xpath="count(//pageViewport[@nr=3]/page/regionViewport/regionBody/mainReference/span/flow/block[1]/block[2]/block)"/>
     
   </checks>
 </testcase>