Bugzilla #51009: RTF generates unexpected lines for blocks in tables. Also fix three findbugs issues.

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

9 files changed, 199 insertions, 60 deletions

diff --git a/src/java/org/apache/fop/area/RegionViewport.java b/src/java/org/apache/fop/area/RegionViewport.java
index 7eeadbffd..093b891bc 100644
--- a/src/java/org/apache/fop/area/RegionViewport.java
+++ b/src/java/org/apache/fop/area/RegionViewport.java
@@ -23,6 +23,7 @@ import java.awt.Rectangle;
 import java.awt.geom.Rectangle2D;
 import java.io.IOException;
 import java.util.HashMap;
+import java.util.TreeMap;
 
 import org.apache.fop.traits.WritingModeTraitsGetter;
 
@@ -108,7 +109,7 @@ public class RegionViewport extends Area implements Cloneable, Viewport {
         out.writeFloat((float) viewArea.getWidth());
         out.writeFloat((float) viewArea.getHeight());
         out.writeBoolean(clip);
-        out.writeObject(traits);
+        out.writeObject((TreeMap)traits);
         out.writeObject(regionReference);
     }
 
@@ -117,7 +118,7 @@ public class RegionViewport extends Area implements Cloneable, Viewport {
         viewArea = new Rectangle2D.Float(in.readFloat(), in.readFloat(),
                                          in.readFloat(), in.readFloat());
         clip = in.readBoolean();
-        traits = (HashMap)in.readObject();
+        traits = (TreeMap)in.readObject();
         setRegionReference((RegionReference) in.readObject());
     }
 
@@ -131,7 +132,7 @@ public class RegionViewport extends Area implements Cloneable, Viewport {
         RegionViewport rv = new RegionViewport((Rectangle2D)viewArea.clone());
         rv.regionReference = (RegionReference)regionReference.clone();
         if (traits != null) {
-            rv.traits = new HashMap(traits);
+            rv.traits = new TreeMap(traits);
         }
         if (foreignAttributes != null) {
             rv.foreignAttributes = new HashMap(foreignAttributes);
diff --git a/src/java/org/apache/fop/area/inline/InlineViewport.java b/src/java/org/apache/fop/area/inline/InlineViewport.java
index 202a7dad4..c2e1243c0 100644
--- a/src/java/org/apache/fop/area/inline/InlineViewport.java
+++ b/src/java/org/apache/fop/area/inline/InlineViewport.java
@@ -22,7 +22,7 @@ package org.apache.fop.area.inline;
 import java.awt.Rectangle;
 import java.awt.geom.Rectangle2D;
 import java.io.IOException;
-import java.util.HashMap;
+import java.util.TreeMap;
 
 import org.apache.fop.area.Area;
 import org.apache.fop.area.Viewport;
@@ -132,7 +132,7 @@ public class InlineViewport extends InlineArea implements Viewport {
             out.writeFloat((float) contentPosition.getHeight());
         }
         out.writeBoolean(clip);
-        out.writeObject(traits);
+        out.writeObject((TreeMap)traits);
         out.writeObject(content);
     }
 
@@ -145,7 +145,7 @@ public class InlineViewport extends InlineArea implements Viewport {
                                                     in.readFloat());
         }
         this.clip = in.readBoolean();
-        this.traits = (HashMap) in.readObject();
+        this.traits = (TreeMap) in.readObject();
         this.content = (Area) in.readObject();
     }
 
diff --git a/src/java/org/apache/fop/render/rtf/RTFHandler.java b/src/java/org/apache/fop/render/rtf/RTFHandler.java
index 95c4fec12..956ea8b57 100644
--- a/src/java/org/apache/fop/render/rtf/RTFHandler.java
+++ b/src/java/org/apache/fop/render/rtf/RTFHandler.java
@@ -119,6 +119,7 @@ import org.apache.fop.render.rtf.rtflib.rtfdoc.RtfList;
 import org.apache.fop.render.rtf.rtflib.rtfdoc.RtfListItem;
 import org.apache.fop.render.rtf.rtflib.rtfdoc.RtfListItem.RtfListItemLabel;
 import org.apache.fop.render.rtf.rtflib.rtfdoc.RtfPage;
+import org.apache.fop.render.rtf.rtflib.rtfdoc.RtfParagraphBreak;
 import org.apache.fop.render.rtf.rtflib.rtfdoc.RtfSection;
 import org.apache.fop.render.rtf.rtflib.rtfdoc.RtfTable;
 import org.apache.fop.render.rtf.rtflib.rtfdoc.RtfTableCell;
@@ -430,8 +431,14 @@ public class RTFHandler extends FOEventHandler {
                     true, this);
 
             RtfTextrun textrun = container.getTextrun();
+            RtfParagraphBreak par = textrun.addParagraphBreak();
+
+            RtfTableCell cellParent = (RtfTableCell)textrun.getParentOfClass(RtfTableCell.class);
+            if (cellParent != null && par != null) {
+                int iDepth = cellParent.findChildren(textrun);
+                cellParent.setLastParagraph(par, iDepth);
+            }
 
-            textrun.addParagraphBreak();
             int breakValue = toRtfBreakValue(bl.getBreakAfter());
             textrun.popBlockAttributes(breakValue);
 
@@ -878,6 +885,14 @@ public class RTFHandler extends FOEventHandler {
         if (bDefer) {
             return;
         }
+        try {
+            RtfTableCell cell = (RtfTableCell)builderContext.getContainer(RtfTableCell.class, false, this);
+            cell.finish();
+
+        } catch (Exception e) {
+            log.error("endCell: " + e.getMessage());
+            throw new RuntimeException(e.getMessage());
+        }
 
         builderContext.popContainer();
         builderContext.getTableContext().selectNextColumn();
diff --git a/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfContainer.java b/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfContainer.java
index 349cdd097..4e86f0091 100644
--- a/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfContainer.java
+++ b/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfContainer.java
@@ -112,6 +112,30 @@ public class RtfContainer extends RtfElement {
         return children.size();
     }
 
+    private int findChildren(RtfElement aChild, int iStart) {
+        for (Iterator it = this.getChildren().iterator(); it.hasNext();) {
+          final RtfElement e = (RtfElement)it.next();
+          if (aChild == e) {
+              return iStart;
+          } else if (e instanceof RtfContainer) {
+              int iFound = ((RtfContainer)e).findChildren(aChild, (iStart + 1));
+              if (iFound != -1) {
+                  return iFound;
+              }
+          }
+        }
+        return -1;
+    }
+
+    /**
+     * Find the passed child in the current container
+     * @param aChild the child element
+     * @return the depth (nested level) inside the current container
+     */
+    public int findChildren(RtfElement aChild) {
+        return findChildren(aChild, 0);
+    }
+
     /**
      * Add by Boris Poudérous on 07/22/2002
      * Set the children list
diff --git a/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfElement.java b/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfElement.java
index c582287a3..1b13fd783 100644
--- a/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfElement.java
+++ b/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfElement.java
@@ -299,7 +299,7 @@ public abstract class RtfElement {
     /** find the first parent where c.isAssignableFrom(parent.getClass()) is true
      *  @return null if not found
      */
-    RtfElement getParentOfClass(Class c) {
+    public RtfElement getParentOfClass(Class c) {
         RtfElement result = null;
         RtfElement current = this;
         while (current.parent != null) {
diff --git a/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfParagraphBreak.java b/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfParagraphBreak.java
new file mode 100644
index 000000000..851deb84b
--- /dev/null
+++ b/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfParagraphBreak.java
@@ -0,0 +1,71 @@
+/*
+ * 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.render.rtf.rtflib.rtfdoc;
+
+import java.io.IOException;
+import java.io.Writer;
+
+/**  Class which represents a paragraph break.*/
+
+public class RtfParagraphBreak extends RtfElement {
+    private static final String DEFAULT_PARAGRAPH = "par";
+
+    private String controlWord = DEFAULT_PARAGRAPH;
+
+    RtfParagraphBreak(RtfContainer parent, Writer w)
+            throws IOException {
+        super(parent, w);
+    }
+
+    /**
+     * @return true if this element would generate no "useful" RTF content
+     */
+    public boolean isEmpty() {
+        return false;
+    }
+
+    /**
+     * write RTF code of all our children
+     * @throws IOException for I/O problems
+     */
+    protected void writeRtfContent() throws IOException {
+        if (controlWord != null ) {
+            writeControlWord(controlWord);
+        }
+    }
+
+    /**
+     * Whether or not the break can be skipped.
+     * If the paragraph marks a table cell end it is not possible
+     * @return boolean
+     */
+    public boolean canHide() {
+        return this.controlWord.equals ( DEFAULT_PARAGRAPH );
+    }
+
+    /**
+     * Sets a different control word for this paragraph. If this method
+     * is used the paragraph will always be displayed (@see canHide))
+     * @param controlWord the new control word
+     */
+    public void switchControlWord(String controlWord) {
+        this.controlWord = controlWord;
+    }
+}
diff --git a/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfTableCell.java b/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfTableCell.java
index 1e2a64b51..faa0852ed 100644
--- a/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfTableCell.java
+++ b/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfTableCell.java
@@ -48,6 +48,11 @@ public class RtfTableCell
     private boolean setCenter;
     private boolean setRight;
     private int id;
+    private RtfParagraphBreak lastBreak = null;
+    private int lastBreakDepth = Integer.MIN_VALUE;
+
+    private static final String TABLE_CELL_PARAGRAPH = "cell";
+    private static final String TABLE_CELL_NESTED_PARAGRAPH = "nestcell";
 
     /** default cell width (in twips ??) */
     public static final int DEFAULT_CELL_WIDTH = 2000;
@@ -312,7 +317,9 @@ public class RtfTableCell
 
         if (getRow().getTable().isNestedTable()) {
             //nested table
-            writeControlWordNS("nestcell");
+            if (lastBreak == null) {
+                writeControlWordNS("nestcell");
+            }
             writeGroupMark(true);
             writeControlWord("nonesttables");
             writeControlWord("par");
@@ -352,7 +359,9 @@ public class RtfTableCell
                 //writeControlWord("par");
             }
 
-            writeControlWord("cell");
+            if (lastBreak == null) {
+                writeControlWord("cell");
+            }
         }
     }
 
@@ -535,4 +544,38 @@ public class RtfTableCell
 
         return null;
     }
+
+    /**
+     * The table cell decides whether or not a newly added paragraph break
+     * will be used to write the cell-end control word.
+     * For nested tables it is not necessary.
+     *
+     * @param parBreak the paragraph break element
+     * @param breakDepth The depth is necessary for picking the correct break element.
+     * If it is deeper inside the whole cell it will be used, and if there is something on
+     * the same level (depth) it is also set because the method is called for all breaks
+     * in the correct order.
+     */
+    public void setLastParagraph(RtfParagraphBreak parBreak, int breakDepth) {
+        if (parBreak != null && breakDepth >= lastBreakDepth) {
+            lastBreak = parBreak;
+            lastBreakDepth = breakDepth;
+        }
+    }
+
+    /**
+     * The last paragraph break was just stored before,
+     * now the control word is really switched
+     */
+    public void finish() {
+      //If it is nested and contains another table do not set it
+      if (getRow().getTable().isNestedTable() && table != null) {
+          lastBreak = null;
+      } else if (lastBreak != null) {
+              lastBreak.switchControlWord(
+                                          getRow().getTable().isNestedTable()
+                                          ? TABLE_CELL_NESTED_PARAGRAPH
+                                          : TABLE_CELL_PARAGRAPH);
+      }
+    }
 }
diff --git a/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfTextrun.java b/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfTextrun.java
index 9a407fe30..afa4416ed 100644
--- a/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfTextrun.java
+++ b/src/java/org/apache/fop/render/rtf/rtflib/rtfdoc/RtfTextrun.java
@@ -144,30 +144,6 @@ public class RtfTextrun extends RtfContainer {
         }
     }
 
-    /**  Class which represents a paragraph break.*/
-    private class RtfParagraphBreak extends RtfElement {
-
-        RtfParagraphBreak(RtfContainer parent, Writer w)
-                throws IOException {
-            super(parent, w);
-        }
-
-        /**
-         * @return true if this element would generate no "useful" RTF content
-         */
-        public boolean isEmpty() {
-            return false;
-        }
-
-        /**
-         * write RTF code of all our children
-         * @throws IOException for I/O problems
-         */
-        protected void writeRtfContent() throws IOException {
-            writeControlWord("par");
-        }
-    }
-
     /** Create an RTF container as a child of given container */
     RtfTextrun(RtfContainer parent, Writer w, RtfAttributes attrs) throws IOException {
         super(parent, w, attrs);
@@ -291,32 +267,35 @@ public class RtfTextrun extends RtfContainer {
      * Inserts paragraph break before all close group marks.
      *
      * @throws IOException  for I/O problems
+     * @return The paragraph break element
      */
-    public void addParagraphBreak() throws IOException {
-      // get copy of children list
-      List children = getChildren();
-      Stack tmp = new Stack();
-
-      // delete all previous CloseGroupMark
-      int deletedCloseGroupCount = 0;
-
-      ListIterator lit = children.listIterator(children.size());
-      while (lit.hasPrevious()
-              && (lit.previous() instanceof RtfCloseGroupMark)) {
-          tmp.push(Integer.valueOf(((RtfCloseGroupMark)lit.next()).getBreakType()));
-          lit.remove();
-          deletedCloseGroupCount++;
-      }
-
-      if (children.size() != 0) {
-          // add paragraph break and restore all deleted close group marks
-          setChildren(children);
-          new RtfParagraphBreak(this, writer);
-          for (int i = 0; i < deletedCloseGroupCount; i++) {
-              addCloseGroupMark(((Integer)tmp.pop()).intValue());
-          }
-      }
-  }
+    public RtfParagraphBreak addParagraphBreak() throws IOException {
+        // get copy of children list
+        List children = getChildren();
+        Stack tmp = new Stack();
+        RtfParagraphBreak par = null;
+
+        // delete all previous CloseGroupMark
+        int deletedCloseGroupCount = 0;
+
+        ListIterator lit = children.listIterator(children.size());
+        while (lit.hasPrevious()
+                && (lit.previous() instanceof RtfCloseGroupMark)) {
+            tmp.push(Integer.valueOf(((RtfCloseGroupMark)lit.next()).getBreakType()));
+            lit.remove();
+            deletedCloseGroupCount++;
+        }
+
+        if (children.size() != 0) {
+            // add paragraph break and restore all deleted close group marks
+            setChildren(children);
+            par = new RtfParagraphBreak(this, writer);
+            for (int i = 0; i < deletedCloseGroupCount; i++) {
+                addCloseGroupMark(((Integer)tmp.pop()).intValue());
+            }
+        }
+        return par;
+    }
 
     /**
      * Inserts a leader.
@@ -486,6 +465,8 @@ public class RtfTextrun extends RtfContainer {
              * child.
              * -If the RtfTextrun is the last child of its parent, write a
              * RtfParagraphBreak only, if it is not the last child.
+             * -If the RtfParagraphBreak can not be hidden (e.g. a table cell requires it)
+             * it is also written
              */
             boolean bHide = false;
             bHide = bRtfParagraphBreak;
@@ -494,7 +475,8 @@ public class RtfTextrun extends RtfContainer {
                     || bFirst
                     || (bSuppressLastPar && bLast && lastParagraphBreak != null
                         && e == lastParagraphBreak)
-                    || bBookmark);
+                    || bBookmark)
+                && ((RtfParagraphBreak)e).canHide();
 
             if (!bHide) {
                 newLine();
diff --git a/status.xml b/status.xml
index cc9eeb213..8b09facf5 100644
--- a/status.xml
+++ b/status.xml
@@ -62,6 +62,9 @@
       documents. Example: the fix of marks layering will be such a case when it's done.
     -->
     <release version="FOP Trunk" date="TBD">
+      <action context="Code" dev="GA" type="fix" fixes-bug="51009" due-to="Max Aster">
+        RTF generates unexpected lines for blocks in tables. Also fix three findbugs issues.
+      </action>
       <action context="Code" dev="GA" type="fix" fixes-bug="51007" due-to="Max Aster">
         RTF tables do not support percent column-widths.
       </action>