Partial fix of FOEventHandler call sequence (see also Bugzilla #45237):

call startOfNode() after addChildNode()
moved initialization for tableFOs to processNode()
moved finishing code to a finalizeNode() method (which is now also used by AbstractRetrieveMarker)
restored protected status of startOfNode()/endOfNode() in fo.flow.table package


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

11 files changed, 77 insertions, 62 deletions

diff --git a/src/java/org/apache/fop/fo/FONode.java b/src/java/org/apache/fop/fo/FONode.java
index 4f84dd717..87d3bfa3c 100644
--- a/src/java/org/apache/fop/fo/FONode.java
+++ b/src/java/org/apache/fop/fo/FONode.java
@@ -298,11 +298,16 @@ public abstract class FONode implements Cloneable {
      * Primarily used for making final content model validation checks
      * and/or informing the {@link FOEventHandler} that the end of this FO
      * has been reached.
+     * The default implementation simply calls {@link #finalizeNode()}, without
+     * sending any event to the {@link FOEventHandler}.
+     * <br/><i>Note: the recommended way to override this method in subclasses is</i>
+     * <br/><br/><code>super.endOfNode(); // invoke finalizeNode()
+     * <br/>getFOEventHandler().endXXX(); // send endOfNode() notification</code>
      *
      * @throws FOPException if there's a problem during processing
      */
     protected void endOfNode() throws FOPException {
-        // do nothing by default
+        this.finalizeNode();
     }
 
     /**
@@ -327,6 +332,20 @@ public abstract class FONode implements Cloneable {
     }
 
     /**
+     * Finalize this node.
+     * This method can be overridden by subclasses to perform finishing
+     * tasks (cleanup, validation checks, ...) without triggering
+     * endXXX() events in the {@link FOEventHandler}.
+     * The method is called by the default {@link #endOfNode()}
+     * implementation.
+     * 
+     * @throws FOPException in case there was an error
+     */
+    public void finalizeNode() throws FOPException {
+        // do nothing by default
+    }
+
+    /**
      * Return the parent node of this node
      *
      * @return the parent node of this node
diff --git a/src/java/org/apache/fop/fo/FOTreeBuilder.java b/src/java/org/apache/fop/fo/FOTreeBuilder.java
index 574e5eb79..d8f712212 100644
--- a/src/java/org/apache/fop/fo/FOTreeBuilder.java
+++ b/src/java/org/apache/fop/fo/FOTreeBuilder.java
@@ -288,7 +288,6 @@ public class FOTreeBuilder extends DefaultHandler {
                         builderContext.switchMarkerContext(true);
                     }
                 }
-                foNode.startOfNode();
             } catch (IllegalArgumentException e) {
                 throw new SAXException(e);
             }
@@ -317,6 +316,7 @@ public class FOTreeBuilder extends DefaultHandler {
             if (propertyList != null && !builderContext.inMarker()) {
                 currentPropertyList = propertyList;
             }
+            currentFObj.startOfNode();
         }
 
         /** {@inheritDoc} */
diff --git a/src/java/org/apache/fop/fo/flow/AbstractRetrieveMarker.java b/src/java/org/apache/fop/fo/flow/AbstractRetrieveMarker.java
index ae06d8399..0ce648c3a 100644
--- a/src/java/org/apache/fop/fo/flow/AbstractRetrieveMarker.java
+++ b/src/java/org/apache/fop/fo/flow/AbstractRetrieveMarker.java
@@ -102,12 +102,6 @@ public abstract class AbstractRetrieveMarker extends FObjMixed {
                         getLocator(),
                         pList,
                         newPropertyList);
-                if (newChild instanceof TableFObj) {
-                    // TODO calling startOfNode (and endOfNode, below) on other fobjs may
-                    // have undesirable side-effects. This is really ugly and will need to
-                    // be addressed sooner or later
-                    ((TableFObj) newChild).startOfNode();
-                }
                 addChildTo(newChild, (FObj) newParent);
                 if (newChild.getNameId() == FO_TABLE) {
                     Table t = (Table) child;
@@ -120,15 +114,13 @@ public abstract class AbstractRetrieveMarker extends FObjMixed {
                 }
                 cloneSubtree(child.getChildNodes(), newChild,
                         marker, newPropertyList);
-                if (newChild instanceof TableFObj) {
-                    // TODO this is ugly
-                    ((TableFObj) newChild).endOfNode();
-                }
             } else if (child instanceof FOText) {
                 FOText ft = (FOText) newChild;
                 ft.bind(parentPropertyList);
                 addChildTo(newChild, (FObj) newParent);
             }
+            
+            newChild.finalizeNode();
             // trigger 'end-of-node' white-space handling
             if (newChild instanceof FObjMixed) {
                 handleWhiteSpaceFor((FObjMixed) newChild, null);
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 771ec0c64..9feb77c9c 100644
--- a/src/java/org/apache/fop/fo/flow/table/Table.java
+++ b/src/java/org/apache/fop/fo/flow/table/Table.java
@@ -157,10 +157,8 @@ public class Table extends TableFObj implements ColumnNumberManagerHolder {
         this.propList = pList;
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    public void startOfNode() throws FOPException {
+    /** {@inheritDoc} */
+    protected void startOfNode() throws FOPException {
         super.startOfNode();
         getFOEventHandler().startTable(this);
     }
@@ -218,11 +216,15 @@ public class Table extends TableFObj implements ColumnNumberManagerHolder {
         }
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    public void endOfNode() throws FOPException {
+    /** {@inheritDoc} */
+    protected void endOfNode() throws FOPException {
+        super.endOfNode();
+        getFOEventHandler().endTable(this);
+    }
 
+    /** {@inheritDoc} */
+    public void finalizeNode() throws FOPException {
+        
         if (!tableBodyFound) {
            missingChildElementError(
                    "(marker*,table-column*,table-header?,table-footer?"
@@ -244,13 +246,10 @@ public class Table extends TableFObj implements ColumnNumberManagerHolder {
             this.propList = null;
             rowGroupBuilder = null;
         }
-        getFOEventHandler().endTable(this);
-
+        
     }
-
-    /**
-     * {@inheritDoc}
-     */
+    
+    /** {@inheritDoc} */
     protected void addChildNode(FONode child) throws FOPException {
 
         int childId = child.getNameId();
@@ -523,17 +522,17 @@ public class Table extends TableFObj implements ColumnNumberManagerHolder {
         return FO_TABLE;
     }
 
-    /**
-     * {@inheritDoc}
-     */
+    /** {@inheritDoc} */
     public FONode clone(FONode parent, boolean removeChildren)
         throws FOPException {
         Table clone = (Table) super.clone(parent, removeChildren);
-        clone.columnsFinalized = false;
         if (removeChildren) {
             clone.columns = new ArrayList();
+            clone.columnsFinalized = false;
+            clone.columnNumberManager = new ColumnNumberManager();
             clone.tableHeader = null;
             clone.tableFooter = null;
+            clone.rowGroupBuilder = null;
         }
         return clone;
     }
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 c3cf772ac..b4071e255 100644
--- a/src/java/org/apache/fop/fo/flow/table/TableBody.java
+++ b/src/java/org/apache/fop/fo/flow/table/TableBody.java
@@ -38,13 +38,13 @@ public class TableBody extends TablePart {
     }
 
     /** {@inheritDoc} */
-    public void startOfNode() throws FOPException {
+    protected void startOfNode() throws FOPException {
         super.startOfNode();
         getFOEventHandler().startBody(this);
     }
 
     /** {@inheritDoc} */
-    public void endOfNode() throws FOPException {
+    protected void endOfNode() throws FOPException {
         super.endOfNode();
         getFOEventHandler().endBody(this);
     }
diff --git a/src/java/org/apache/fop/fo/flow/table/TableCell.java b/src/java/org/apache/fop/fo/flow/table/TableCell.java
index 2603ba8b4..21da54128 100644
--- a/src/java/org/apache/fop/fo/flow/table/TableCell.java
+++ b/src/java/org/apache/fop/fo/flow/table/TableCell.java
@@ -91,17 +91,24 @@ public class TableCell extends TableFObj {
     }
 
     /** {@inheritDoc} */
-    public void startOfNode() throws FOPException {
+    protected void startOfNode() throws FOPException {
         super.startOfNode();
         getFOEventHandler().startCell(this);
     }
 
     /**
      * Make sure content model satisfied, if so then tell the
-     * FOEventHandler that we are at the end of the flow.
+     * FOEventHandler that we are at the end of the table-cell.
      * {@inheritDoc}
      */
-    public void endOfNode() throws FOPException {
+    protected void endOfNode() throws FOPException {
+        super.endOfNode();
+        getFOEventHandler().endCell(this);
+    }
+
+    /** {@inheritDoc} */
+    public void finalizeNode() throws FOPException {
+        
         if (!blockItemFound) {
             missingChildElementError("marker* (%block;)+", true);
         }
@@ -111,9 +118,9 @@ public class TableCell extends TableFObj {
                     getUserAgent().getEventBroadcaster());
             eventProducer.startEndRowUnderTableRowWarning(this, getLocator());
         }
-        getFOEventHandler().endCell(this);
+        
     }
-
+    
     /**
      * {@inheritDoc}
      * <br>XSL Content Model: marker* (%block;)+
diff --git a/src/java/org/apache/fop/fo/flow/table/TableFObj.java b/src/java/org/apache/fop/fo/flow/table/TableFObj.java
index 016046ae2..370bb9de4 100644
--- a/src/java/org/apache/fop/fo/flow/table/TableFObj.java
+++ b/src/java/org/apache/fop/fo/flow/table/TableFObj.java
@@ -34,6 +34,8 @@ import org.apache.fop.fo.properties.NumberProperty;
 import org.apache.fop.fo.properties.Property;
 import org.apache.fop.fo.properties.PropertyMaker;
 import org.apache.fop.layoutmgr.table.CollapsingBorderModel;
+import org.xml.sax.Locator;
+import org.xml.sax.Attributes;
 
 /**
  * Common base class for table-related FOs
@@ -207,8 +209,8 @@ public abstract class TableFObj extends FObj {
     }
 
     /** {@inheritDoc} */
-    public void startOfNode() throws FOPException {
-        super.startOfNode();
+    public void processNode(String elementName, Locator locator, Attributes attlist, PropertyList pList) throws FOPException {
+        super.processNode(elementName, locator, attlist, pList);
         Table table = getTable();
         if (!inMarker() && !table.isSeparateBorderModel()) {
             collapsingBorderModel = CollapsingBorderModel.getBorderModelFor(table
@@ -216,15 +218,7 @@ public abstract class TableFObj extends FObj {
             setCollapsedBorders();
         }
     }
-
-    /*
-     * TODO made public so that RetrieveMarker can access it.
-     */
-    /** {@inheritDoc} */
-    public void endOfNode() throws FOPException {
-        super.endOfNode();
-    }
-
+    
     /**
      * Prepares the borders of this element if the collapsing-border model is in use.
      * Conflict resolution with parent elements is done where applicable.
diff --git a/src/java/org/apache/fop/fo/flow/table/TableFooter.java b/src/java/org/apache/fop/fo/flow/table/TableFooter.java
index 886001e76..a89a2e431 100644
--- a/src/java/org/apache/fop/fo/flow/table/TableFooter.java
+++ b/src/java/org/apache/fop/fo/flow/table/TableFooter.java
@@ -41,13 +41,13 @@ public class TableFooter extends TablePart {
     }
 
     /** {@inheritDoc} */
-    public void startOfNode() throws FOPException {
+    protected void startOfNode() throws FOPException {
         super.startOfNode();
         getFOEventHandler().startFooter(this);
     }
 
     /** {@inheritDoc} */
-    public void endOfNode() throws FOPException {
+    protected void endOfNode() throws FOPException {
         super.endOfNode();
         getFOEventHandler().endFooter(this);
     }
diff --git a/src/java/org/apache/fop/fo/flow/table/TableHeader.java b/src/java/org/apache/fop/fo/flow/table/TableHeader.java
index c42d79b46..7f4173754 100644
--- a/src/java/org/apache/fop/fo/flow/table/TableHeader.java
+++ b/src/java/org/apache/fop/fo/flow/table/TableHeader.java
@@ -40,13 +40,13 @@ public class TableHeader extends TablePart {
     }
 
     /** {@inheritDoc} */
-    public void startOfNode() throws FOPException {
+    protected void startOfNode() throws FOPException {
         super.startOfNode();
         getFOEventHandler().startHeader(this);
     }
 
     /** {@inheritDoc} */
-    public void endOfNode() throws FOPException {
+    protected void endOfNode() throws FOPException {
         super.endOfNode();
         getFOEventHandler().endHeader(this);
     }
diff --git a/src/java/org/apache/fop/fo/flow/table/TablePart.java b/src/java/org/apache/fop/fo/flow/table/TablePart.java
index 67af69e03..4d20db8c4 100644
--- a/src/java/org/apache/fop/fo/flow/table/TablePart.java
+++ b/src/java/org/apache/fop/fo/flow/table/TablePart.java
@@ -79,7 +79,8 @@ public abstract class TablePart extends TableCellContainer {
     public void processNode(String elementName, Locator locator,
                             Attributes attlist, PropertyList pList)
                     throws FOPException {
-
+        
+        super.processNode(elementName, locator, attlist, pList);
         if (!inMarker()) {
             Table t = getTable();
             if (t.hasExplicitColumns()) {
@@ -93,13 +94,11 @@ public abstract class TablePart extends TableCellContainer {
             }
             columnNumberManager = new ColumnNumberManager();
         }
-        super.processNode(elementName, locator, attlist, pList);
-
+        
     }
 
     /** {@inheritDoc} */
-    public void endOfNode() throws FOPException {
-        super.endOfNode();
+    public void finalizeNode() throws FOPException {
         if (!inMarker()) {
             pendingSpans = null;
             columnNumberManager = null;
diff --git a/src/java/org/apache/fop/fo/flow/table/TableRow.java b/src/java/org/apache/fop/fo/flow/table/TableRow.java
index 8b5fc6bd4..ac6eafc2f 100644
--- a/src/java/org/apache/fop/fo/flow/table/TableRow.java
+++ b/src/java/org/apache/fop/fo/flow/table/TableRow.java
@@ -78,12 +78,12 @@ public class TableRow extends TableCellContainer {
     /** {@inheritDoc} */
     public void processNode(String elementName, Locator locator,
             Attributes attlist, PropertyList pList) throws FOPException {
+        super.processNode(elementName, locator, attlist, pList);
         if (!inMarker()) {
             TablePart part = (TablePart) parent;
             pendingSpans = part.pendingSpans;
             columnNumberManager = part.columnNumberManager;
         }
-        super.processNode(elementName, locator, attlist, pList);
     }
 
     /** {@inheritDoc} */
@@ -97,13 +97,19 @@ public class TableRow extends TableCellContainer {
     }
 
     /** {@inheritDoc} */
-    public void startOfNode() throws FOPException {
+    protected void startOfNode() throws FOPException {
         super.startOfNode();
         getFOEventHandler().startRow(this);
     }
 
     /** {@inheritDoc} */
-    public void endOfNode() throws FOPException {
+    protected void endOfNode() throws FOPException {
+        super.endOfNode();
+        getFOEventHandler().endRow(this);
+    }
+
+    /** {@inheritDoc} */
+    public void finalizeNode() throws FOPException {
         if (firstChild == null) {
             missingChildElementError("(table-cell+)");
         }
@@ -111,9 +117,8 @@ public class TableRow extends TableCellContainer {
             pendingSpans = null;
             columnNumberManager = null;
         }
-        getFOEventHandler().endRow(this);
     }
-
+    
     /**
      * {@inheritDoc} String, String)
      * <br>XSL Content Model: (table-cell+)