Merged revisions 655489,655500,655522,655531,655578,655601,655603,655614,655766,655771,655775 via svnmerge from

https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk

........
  r655489 | vhennebert | 2008-05-12 13:30:42 +0100 (Mon, 12 May 2008) | 2 lines
  
  Replaced hack in TableStepper to get the steps' penalty values with a more proper implementation. Allows in the same time to avoid skimming, at each step, through the active cells' LinkedLists of elements, which may have a negative impact on performance
........
  r655500 | adelmelle | 2008-05-12 14:42:49 +0100 (Mon, 12 May 2008) | 2 lines
  
  Extended caching to CondLengthProperty CommonBorderPaddingBackground.BorderInfo and CommonBorderPaddingBackground.
........
  r655522 | adelmelle | 2008-05-12 16:24:06 +0100 (Mon, 12 May 2008) | 2 lines
  
  Tweak: avoid preloading the background-image with each pass through the constructor; only do so for non-cached instances
........
  r655531 | adelmelle | 2008-05-12 16:34:49 +0100 (Mon, 12 May 2008) | 2 lines
  
  Avoid an error if unspecified...
........
  r655578 | spepping | 2008-05-12 18:53:21 +0100 (Mon, 12 May 2008) | 3 lines
  
  Restore the previous behaviour of the column-number property, viz. if
  it is negative, it is set the next free column
........
  r655601 | adelmelle | 2008-05-12 20:06:04 +0100 (Mon, 12 May 2008) | 1 line
  
  Added missing file from r655500
........
  r655603 | adelmelle | 2008-05-12 20:11:00 +0100 (Mon, 12 May 2008) | 1 line
  
  Re-enabled testcases that pass again after r655578
........
  r655614 | vhennebert | 2008-05-12 20:37:39 +0100 (Mon, 12 May 2008) | 3 lines
  
  Put the resolutions of collapsed borders together into the CollapsingBorderResolver class.
  The previous scheme allowed for early resolution of borders where possible, but made it hard to understand since the resolution was spread in the various table classes. Now everything is done inside a single class
........
  r655766 | adelmelle | 2008-05-13 08:58:31 +0100 (Tue, 13 May 2008) | 2 lines
  
  Added support for auto-generated initial value for the "id" property.
........
  r655771 | adelmelle | 2008-05-13 09:11:12 +0100 (Tue, 13 May 2008) | 1 line
  
  Alter auto-id naming to avoid confusion with URI fragment identifiers
........
  r655775 | adelmelle | 2008-05-13 09:20:57 +0100 (Tue, 13 May 2008) | 1 line
  
  Javadoc fixups
........


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

src/java/org/apache/fop/fo/FOEventHandler.java

@@ -79,7 +79,11 @@ public abstract class FOEventHandler {
      * This is used so we know if the FO tree contains duplicates.
      */
     private Set idReferences = new HashSet();
-
+    /**
+     * Keeps track of the last automatically generated id in the current document
+     */
+    private long lastGeneratedId = 1;
+    
     /**
      * The property list maker.
      */
@@ -131,13 +135,17 @@ public abstract class FOEventHandler {
 
     /**
      * Return the propertyListMaker.
-    */
+     * 
+     * @return the currently active {@link PropertyListMaker}
+     */
     public PropertyListMaker getPropertyListMaker() {
         return propertyListMaker;
     }
      
     /**
      * Set a new propertyListMaker.
+     * 
+     * @param propertyListMaker the new {@link PropertyListMaker} to use
      */
     public void setPropertyListMaker(PropertyListMaker propertyListMaker) {
         this.propertyListMaker = propertyListMaker;
@@ -155,6 +163,9 @@ public abstract class FOEventHandler {
      * Switch to or from marker context
      * (used by FOTreeBuilder when processing
      *  a marker)
+     * 
+     * @param inMarker  true if a marker is being processed; 
+     *                  false otherwise
      *
      */
     protected void switchMarkerContext(boolean inMarker) {
@@ -163,11 +174,22 @@ public abstract class FOEventHandler {
     
     /**
      * Check whether in marker context
+     * 
+     * @return true if a marker is being processed
      */
     protected boolean inMarker() {
         return this.inMarker;
     }
     
+    /**
+     * Return the next value for automatically generated ids
+     * 
+     * @return the next value to append to automatically generated ids
+     */
+    public long getNextId() {
+        return this.lastGeneratedId++;
+    }
+    
     /**
      * This method is called to indicate the start of a new document run.
      * @throws SAXException In case of a problem

src/java/org/apache/fop/fo/FOPropertyMapping.java

@@ -2503,7 +2503,7 @@ public final class FOPropertyMapping implements Constants {
         addPropertyMaker("content-type", m);
 
         // id
-        m  = new StringProperty.Maker(PR_ID);
+        m  = new StringProperty.IdMaker(PR_ID);
         m.setInherited(false);
         m.setDefault("");
         addPropertyMaker("id", m);

src/java/org/apache/fop/fo/PropertyList.java

@@ -349,13 +349,10 @@ public abstract class PropertyList {
                         findBasePropertyName(propertyName));
         int subpropId = FOPropertyMapping.getSubPropertyId(
                         findSubPropertyName(propertyName));
-        
-        if (propId == -1 
-                || (subpropId == -1 
-                        && findSubPropertyName(propertyName) != null)) {
-            return false;
-        }
-        return true;
+
+        return !(propId == -1
+                || (subpropId == -1
+                && findSubPropertyName(propertyName) != null));
     }
 
     /**
@@ -574,7 +571,7 @@ public abstract class PropertyList {
      */
     public CommonBorderPaddingBackground getBorderPaddingBackgroundProps() 
                 throws PropertyException {
-        return new CommonBorderPaddingBackground(this);
+        return CommonBorderPaddingBackground.getInstance(this);
     }
     
     /**

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

@@ -24,6 +24,7 @@ import java.util.Iterator;
 import java.util.List;
 
 import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
+import org.apache.fop.layoutmgr.table.CollapsingBorderModel;
 
 /**
  * A class that implements the border-collapsing model.
@@ -32,6 +33,8 @@ class CollapsingBorderResolver implements BorderResolver {
 
     private Table table;
 
+    private CollapsingBorderModel collapsingBorderModel;
+
     /**
      * The previously registered row, either in the header or the body(-ies), but not in
      * the footer (handled separately).
@@ -74,6 +77,9 @@ class CollapsingBorderResolver implements BorderResolver {
 
         protected boolean firstInPart;
 
+        private BorderSpecification borderStartTableAndBody;
+        private BorderSpecification borderEndTableAndBody;
+
         /**
          * Integrates border-before specified on the table and its column.
          * 
@@ -174,6 +180,10 @@ class CollapsingBorderResolver implements BorderResolver {
         void startPart(TableBody part) {
             tablePart = part;
             firstInPart = true;
+            borderStartTableAndBody = collapsingBorderModel.determineWinner(table.borderStart,
+                    tablePart.borderStart);
+            borderEndTableAndBody = collapsingBorderModel.determineWinner(table.borderEnd,
+                    tablePart.borderEnd);
         }
 
         /**
@@ -188,6 +198,8 @@ class CollapsingBorderResolver implements BorderResolver {
          * @param container the containing element
          */
         void endRow(List/*<GridUnit>*/ row, TableCellContainer container) {
+            BorderSpecification borderStart = borderStartTableAndBody;
+            BorderSpecification borderEnd = borderEndTableAndBody;
             // Resolve before- and after-borders for the table-row
             if (container instanceof TableRow) {
                 TableRow tableRow = (TableRow) container;
@@ -200,6 +212,10 @@ class CollapsingBorderResolver implements BorderResolver {
                     gu.integrateBorderSegment(CommonBorderPaddingBackground.AFTER, tableRow,
                             last, last, true);
                 }
+                borderStart = collapsingBorderModel.determineWinner(borderStart,
+                        tableRow.borderStart);
+                borderEnd = collapsingBorderModel.determineWinner(borderEnd,
+                        tableRow.borderEnd);
             }
             if (firstInPart) {
                 // Integrate the border-before of the part
@@ -215,7 +231,7 @@ class CollapsingBorderResolver implements BorderResolver {
             Iterator colIter = table.getColumns().iterator();
             TableColumn col = (TableColumn) colIter.next();
             gu.integrateBorderSegment(CommonBorderPaddingBackground.START, col);
-            gu.integrateBorderSegment(CommonBorderPaddingBackground.START, container);
+            gu.integrateBorderSegment(CommonBorderPaddingBackground.START, borderStart);
             while (guIter.hasNext()) {
                 GridUnit nextGU = (GridUnit) guIter.next();
                 TableColumn nextCol = (TableColumn) colIter.next();
@@ -228,7 +244,7 @@ class CollapsingBorderResolver implements BorderResolver {
                 col = nextCol;
             }
             gu.integrateBorderSegment(CommonBorderPaddingBackground.END, col);
-            gu.integrateBorderSegment(CommonBorderPaddingBackground.END, container);
+            gu.integrateBorderSegment(CommonBorderPaddingBackground.END, borderEnd);
         }
 
         void endPart() {
@@ -371,7 +387,26 @@ class CollapsingBorderResolver implements BorderResolver {
 
     CollapsingBorderResolver(Table table) {
         this.table = table;
+        collapsingBorderModel = CollapsingBorderModel.getBorderModelFor(table.getBorderCollapse());
         firstInTable = true;
+        // Resolve before and after borders between the table and each table-column
+        int index = 0;
+        do {
+            TableColumn col = table.getColumn(index);
+            // See endRow method in ResolverInHeader for an explanation of the hack
+            col.borderBefore.integrateSegment(table.borderBefore, true, false, true);
+            col.borderBefore.leadingTrailing = col.borderBefore.rest;
+            col.borderAfter.integrateSegment(table.borderAfter, true, false, true);
+            col.borderAfter.leadingTrailing = col.borderAfter.rest;
+            /*
+             * TODO The border resolution must be done only once for each table column,
+             * even if it's repeated; otherwise, re-resolving against the table's borders
+             * will lead to null border specifications.
+             * 
+             * Eventually table columns should probably be cloned instead.
+             */
+            index += col.getNumberColumnsRepeated();
+        } while (index < table.getNumberOfColumns());
     }
 
     /** {@inheritDoc} */
@@ -388,9 +423,7 @@ class CollapsingBorderResolver implements BorderResolver {
                 // No header, leading borders determined by the table
                 leadingBorders = new ArrayList(table.getNumberOfColumns());
                 for (Iterator colIter = table.getColumns().iterator(); colIter.hasNext();) {
-                    // See endRow method in ResolverInHeader for an explanation of the hack
                     ConditionalBorder border = ((TableColumn) colIter.next()).borderBefore;
-                    border.leadingTrailing = border.rest;
                     leadingBorders.add(border);
                 }
             }
@@ -402,9 +435,7 @@ class CollapsingBorderResolver implements BorderResolver {
                     // No footer, trailing borders determined by the table
                     trailingBorders = new ArrayList(table.getNumberOfColumns());
                     for (Iterator colIter = table.getColumns().iterator(); colIter.hasNext();) {
-                        // See endRow method in ResolverInHeader for an explanation of the hack
                         ConditionalBorder border = ((TableColumn) colIter.next()).borderAfter;
-                        border.leadingTrailing = border.rest;
                         trailingBorders.add(border);
                     }
                 }

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

@@ -374,6 +374,24 @@ public class GridUnit {
         }
     }
 
+    /**
+     * For the given side, integrates in the conflict resolution the given border segment.
+     * 
+     * @param side the side to consider (one of CommonBorderPaddingBackground.START|END)
+     * @param segment a border specification to integrate at the given side
+     */
+    void integrateBorderSegment(int side, BorderSpecification segment) {
+        switch(side) {
+        case CommonBorderPaddingBackground.START:
+            borderStart = collapsingBorderModel.determineWinner(borderStart, segment);
+            break;
+        case CommonBorderPaddingBackground.END:
+            borderEnd = collapsingBorderModel.determineWinner(borderEnd, segment);
+            break;
+        default: assert false;
+        }
+    }
+
     void integrateCompetingBorder(int side, ConditionalBorder competitor,
             boolean withNormal, boolean withLeadingTrailing, boolean withRest) {
         switch (side) {

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

@@ -265,7 +265,7 @@ public class Table extends TableFObj implements ColumnNumberManagerHolder {
         case FO_TABLE_HEADER:
         case FO_TABLE_FOOTER:
         case FO_TABLE_BODY:
-            if (!columnsFinalized) {
+            if (!inMarker() && !columnsFinalized) {
                 columnsFinalized = true;
                 if (hasExplicitColumns) {
                     finalizeColumns();
@@ -291,14 +291,6 @@ public class Table extends TableFObj implements ColumnNumberManagerHolder {
         }
     }
 
-    /** {@inheritDoc} */
-    protected void setCollapsedBorders() {
-        createBorder(CommonBorderPaddingBackground.START);
-        createBorder(CommonBorderPaddingBackground.END);
-        createBorder(CommonBorderPaddingBackground.BEFORE);
-        createBorder(CommonBorderPaddingBackground.AFTER);
-    }
-
     private void finalizeColumns() throws FOPException {
         for (int i = 0; i < columns.size(); i++) {
             if (columns.get(i) == null) {

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

@@ -211,15 +211,6 @@ public class TableBody extends TableCellContainer {
         super.addChildNode(child);
     }
 
-    /** {inheritDoc} */
-    protected void setCollapsedBorders() {
-        Table table = (Table) parent;
-        createBorder(CommonBorderPaddingBackground.START, table);
-        createBorder(CommonBorderPaddingBackground.END, table);
-        createBorder(CommonBorderPaddingBackground.BEFORE);
-        createBorder(CommonBorderPaddingBackground.AFTER);
-    }
-
     void addRowGroup(List rowGroup) {
         rowGroups.add(rowGroup);
     }

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

@@ -133,14 +133,6 @@ public class TableCell extends TableFObj {
         }
     }
 
-    /** {@inheritDoc} */
-    protected void setCollapsedBorders() {
-        createBorder(CommonBorderPaddingBackground.BEFORE);
-        createBorder(CommonBorderPaddingBackground.AFTER);
-        createBorder(CommonBorderPaddingBackground.START);
-        createBorder(CommonBorderPaddingBackground.END);
-    }
-
     /** {@inheritDoc} */
     public boolean generatesReferenceAreas() {
         return true;

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

@@ -133,15 +133,6 @@ public class TableColumn extends TableFObj {
         setCollapsedBorders();
     }
 
-    /** {@inheritDoc} */
-    protected void setCollapsedBorders() {
-        Table table = (Table) parent;
-        createBorder(CommonBorderPaddingBackground.BEFORE, table);
-        createBorder(CommonBorderPaddingBackground.AFTER, table);
-        createBorder(CommonBorderPaddingBackground.START);
-        createBorder(CommonBorderPaddingBackground.END);
-    }
-
     /** {@inheritDoc} */
     public void endOfNode() throws FOPException {
         getFOEventHandler().endColumn(this);

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

@@ -28,8 +28,11 @@ import org.apache.fop.fo.FObj;
 import org.apache.fop.fo.PropertyList;
 import org.apache.fop.fo.expr.PropertyException;
 import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
+import org.apache.fop.fo.properties.EnumNumber;
+import org.apache.fop.fo.properties.EnumProperty;
 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;
 
 /**
@@ -117,7 +120,7 @@ public abstract class TableFObj extends FObj {
      * PropertyMaker subclass for the column-number property
      *
      */
-    public static class ColumnNumberPropertyMaker extends NumberProperty.PositiveIntegerMaker {
+    public static class ColumnNumberPropertyMaker extends PropertyMaker {
 
         /**
          * Constructor
@@ -172,6 +175,32 @@ public abstract class TableFObj extends FObj {
 
             return p;
         }
+        
+        /**
+         * If the value is not positive, return a property whose value is the next column number
+         * 
+         * {@inheritDoc}
+         */
+        public Property convertProperty(Property p, 
+                                        PropertyList propertyList, FObj fo) 
+                    throws PropertyException {
+            if (p instanceof EnumProperty) {
+                return EnumNumber.getInstance(p);
+            }
+            Number val = p.getNumber();
+            if (val != null) {
+                int i = Math.round(val.floatValue());
+                if (i <= 0) {
+                    ColumnNumberManagerHolder parent =
+                        (ColumnNumberManagerHolder) propertyList.getParentFObj();
+                    ColumnNumberManager columnIndexManager =  parent.getColumnNumberManager();
+                    i = columnIndexManager.getCurrentColumnNumber();
+                }
+                return NumberProperty.getInstance(i);
+            }
+            return convertPropertyDatatype(p, propertyList, fo);
+        }
+
     }
 
     /** {@inheritDoc} */
@@ -197,7 +226,12 @@ public abstract class TableFObj extends FObj {
      * Prepares the borders of this element if the collapsing-border model is in use.
      * Conflict resolution with parent elements is done where applicable.
      */
-    protected abstract void setCollapsedBorders();
+    protected void setCollapsedBorders() {
+        createBorder(CommonBorderPaddingBackground.START);
+        createBorder(CommonBorderPaddingBackground.END);
+        createBorder(CommonBorderPaddingBackground.BEFORE);
+        createBorder(CommonBorderPaddingBackground.AFTER);
+    }
 
     /**
      * Creates a BorderSpecification from the border set on the given side. If no border
@@ -205,7 +239,7 @@ public abstract class TableFObj extends FObj {
      * 
      * @param side one of CommonBorderPaddingBackground.BEFORE|AFTER|START|END
      */
-    protected void createBorder(int side) {
+    private void createBorder(int side) {
         BorderSpecification borderSpec = new BorderSpecification(
                 getCommonBorderPaddingBackground().getBorderInfo(side), getNameId());
         switch (side) {
@@ -224,33 +258,4 @@ public abstract class TableFObj extends FObj {
         default: assert false;
         }
     }
-
-    /**
-     * Creates a BorderSpecification from the border set on the given side, performing
-     * conflict resolution with the same border on the given object.
-     * 
-     * @param side one of CommonBorderPaddingBackground.BEFORE|AFTER|START|END
-     * @param competitor a parent table element whose side coincides with the given side
-     * on this element
-     */
-    protected void createBorder(int side, TableFObj competitor) {
-        createBorder(side);
-        switch (side) {
-        case CommonBorderPaddingBackground.BEFORE:
-            borderBefore.integrateSegment(competitor.borderBefore, true, true, true);
-            break;
-        case CommonBorderPaddingBackground.AFTER:
-            borderAfter.integrateSegment(competitor.borderAfter, true, true, true);
-            break;
-        case CommonBorderPaddingBackground.START:
-            borderStart = collapsingBorderModel.determineWinner(borderStart,
-                    competitor.borderStart);
-            break;
-        case CommonBorderPaddingBackground.END:
-            borderEnd = collapsingBorderModel.determineWinner(borderEnd,
-                    competitor.borderEnd);
-            break;
-        default: assert false;
-        }
-    }
 }

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

@@ -138,15 +138,6 @@ public class TableRow extends TableCellContainer {
         return true;
     }
 
-    /** {@inheritDoc} */
-    protected void setCollapsedBorders() {
-        TableBody body = (TableBody) parent;
-        createBorder(CommonBorderPaddingBackground.START, body);
-        createBorder(CommonBorderPaddingBackground.END, body);
-        createBorder(CommonBorderPaddingBackground.BEFORE);
-        createBorder(CommonBorderPaddingBackground.AFTER);
-    }
-
     /** @return the "break-after" property. */
     public int getBreakAfter() {
         return breakAfter;

src/java/org/apache/fop/fo/properties/CommonBorderPaddingBackground.java

@@ -38,35 +38,44 @@ import org.apache.fop.fo.expr.PropertyException;
  * See Sec. 7.7 of the XSL-FO Standard.
  */
 public class CommonBorderPaddingBackground {
+
+    /** 
+     *  cache holding all canonical instances 
+     *  (w/ absolute background-position-* and padding-*)
+     */
+    private static final PropertyCache cache = new PropertyCache(CommonBorderPaddingBackground.class);
+    
+    private int hash = -1;
+    
     /**
      * The "background-attachment" property.
      */
-    public int backgroundAttachment;
+    public final int backgroundAttachment;
 
     /**
      * The "background-color" property.
      */
-    public Color backgroundColor;
+    public final Color backgroundColor;
 
     /**
      * The "background-image" property.
      */
-    public String backgroundImage;
+    public final String backgroundImage;
 
     /**
      * The "background-repeat" property.
      */
-    public int backgroundRepeat;
+    public final int backgroundRepeat;
 
     /**
      * The "background-position-horizontal" property.
      */
-    public Length backgroundPositionHorizontal;
+    public final Length backgroundPositionHorizontal;
 
     /**
      * The "background-position-vertical" property.
      */
-    public Length backgroundPositionVertical;
+    public final Length backgroundPositionVertical;
 
 
     private ImageInfo backgroundImageInfo;
@@ -81,29 +90,68 @@ public class CommonBorderPaddingBackground {
     /** the "end" edge */
     public static final int END = 3;
 
+    /**
+     * 
+     */
     public static class BorderInfo {
+        
+        /** cache holding all canonical instances */
+        private static final PropertyCache cache = new PropertyCache(BorderInfo.class);
+        
         private int mStyle; // Enum for border style
         private Color mColor; // Border color
         private CondLengthProperty mWidth;
+        
+        private int hash = -1;
 
-        BorderInfo(int style, CondLengthProperty width, Color color) {
+        /**
+         * Hidden constructor
+         */
+        private BorderInfo(int style, CondLengthProperty width, Color color) {
             mStyle = style;
             mWidth = width;
             mColor = color;
         }
 
+        /**
+         * Returns a BorderInfo instance corresponding to the given values
+         * 
+         * @param style the border-style
+         * @param width the border-width
+         * @param color the border-color
+         * @return a cached BorderInfo instance
+         */
+        public static BorderInfo getInstance(int style, CondLengthProperty width, Color color) {
+            return cache.fetch(new BorderInfo(style, width, color));
+        }
+
+        /**
+         * @return  the border-style
+         */
         public int getStyle() {
             return this.mStyle;
         }
 
+        /**
+         * @return the border-color
+         */
         public Color getColor() {
             return this.mColor;
         }
 
+        /**
+         * @return the border-width
+         */
         public CondLengthProperty getWidth() {
             return this.mWidth;
         }
 
+        /**
+         * Convenience method returning the border-width,
+         * taking into account values of "none" and "hidden"
+         * 
+         * @return  the retained border-width
+         */
         public int getRetainedWidth() {
             if ((mStyle == Constants.EN_NONE)
                     || (mStyle == Constants.EN_HIDDEN)) {
@@ -125,14 +173,43 @@ public class CommonBorderPaddingBackground {
             sb.append("}");
             return sb.toString();
         }
+        
+        /** {@inheritDoc} */
+        public boolean equals(Object obj) {
+            if (this == obj) {
+                return true;
+            }
+            
+            if (obj instanceof BorderInfo) {
+                BorderInfo bi = (BorderInfo)obj;
+                return (this.mColor == bi.mColor
+                    && this.mStyle == bi.mStyle
+                    && this.mWidth == bi.mWidth);
+            }
+            
+            return false;
+        }
+        
+        /** {@inheritDoc} */
+        public int hashCode() {
+            if (this.hash == -1) {
+                int hash = 17;
+                hash = 37 * hash + (mColor == null ? 0 : mColor.hashCode());
+                hash = 37 * hash + mStyle;
+                hash = 37 * hash + (mWidth == null ? 0 : mWidth.hashCode());
+                this.hash = hash;
+            }
+            return this.hash;
+        }
     }
 
     /**
-     * A border info with style none. Used as a singleton, in the collapsing-border model,
+     * A border info with style "none". Used as a singleton, in the collapsing-border model,
      * for elements which don't specify any border on some of their sides.
      */
-    private static BorderInfo defaultBorderInfo;
-
+    private static final BorderInfo defaultBorderInfo 
+            = BorderInfo.getInstance(Constants.EN_NONE, new ConditionalNullLength(), null);
+    
     /**
      * A conditional length of value 0. Returned by the
      * {@link CommonBorderPaddingBackground#getBorderInfo(int)} method when the
@@ -191,62 +268,44 @@ public class CommonBorderPaddingBackground {
      * 
      * @return a BorderInfo instance with style set to {@link Constants#EN_NONE}
      */
-    public static synchronized BorderInfo getDefaultBorderInfo() {
-        if (defaultBorderInfo == null) {
-            /* It is enough to set color to null, as it should never be consulted */
-            defaultBorderInfo = new BorderInfo(Constants.EN_NONE,
-                    new ConditionalNullLength(), null);
-        }
+    public static BorderInfo getDefaultBorderInfo() {
         return defaultBorderInfo;
     }
 
     private BorderInfo[] borderInfo = new BorderInfo[4];
     private CondLengthProperty[] padding = new CondLengthProperty[4];
 
-    /**
-     * Construct a CommonBorderPaddingBackground object.
-     */
-    public CommonBorderPaddingBackground() {
-    }
-
     /**
      * Construct a CommonBorderPaddingBackground object.
      * 
      * @param pList The PropertyList to get properties from.
      * @throws PropertyException if there's an error while binding the properties
      */
-    public CommonBorderPaddingBackground(PropertyList pList) throws PropertyException {
+    private CommonBorderPaddingBackground(PropertyList pList) throws PropertyException {
 
         backgroundAttachment = pList.get(Constants.PR_BACKGROUND_ATTACHMENT).getEnum();
-        backgroundColor = pList.get(Constants.PR_BACKGROUND_COLOR).getColor(
+        
+        Color bc = pList.get(Constants.PR_BACKGROUND_COLOR).getColor(
                                         pList.getFObj().getUserAgent());
-        if (backgroundColor.getAlpha() == 0) {
+        if (bc.getAlpha() == 0) {
             backgroundColor = null;
+        } else {
+            backgroundColor = bc;
         }
 
-        backgroundImage = pList.get(Constants.PR_BACKGROUND_IMAGE).getString();
-        if (backgroundImage == null || "none".equals(backgroundImage)) {
-            backgroundImage = null;
+        String img = pList.get(Constants.PR_BACKGROUND_IMAGE).getString();
+        if (img == null || "none".equals(img)) {
+            backgroundImage = "";
+            backgroundRepeat = -1;
+            backgroundPositionHorizontal = null;
+            backgroundPositionVertical = null;
         } else {
+            backgroundImage = img;
             backgroundRepeat = pList.get(Constants.PR_BACKGROUND_REPEAT).getEnum();
             backgroundPositionHorizontal = pList.get(
                     Constants.PR_BACKGROUND_POSITION_HORIZONTAL).getLength();
             backgroundPositionVertical = pList.get(
                     Constants.PR_BACKGROUND_POSITION_VERTICAL).getLength();
-
-            //Additional processing: preload image
-            String uri = URISpecification.getURL(backgroundImage);
-            FOUserAgent userAgent = pList.getFObj().getUserAgent();
-            ImageManager manager = userAgent.getFactory().getImageManager();
-            ImageSessionContext sessionContext = userAgent.getImageSessionContext();
-            ImageInfo info;
-            try {
-                info = manager.getImageInfo(uri, sessionContext);
-                this.backgroundImageInfo = info;
-            } catch (Exception e) {
-                Property.log.error("Background image not available: " + uri);
-            }
-            //TODO Report to caller so he can decide to throw an exception
         }
 
         initBorderInfo(pList, BEFORE,
@@ -272,18 +331,67 @@ public class CommonBorderPaddingBackground {
 
     }
 
+    /**
+     * Obtain a CommonBorderPaddingBackground instance based on the 
+     * related property valus in the given {@link PropertyList}
+     * 
+     * @param pList the {@link PropertyList} to use
+     * @return a CommonBorderPaddingBackground instance (cached if possible)
+     * @throws PropertyException in case of an error
+     */
+    public static CommonBorderPaddingBackground getInstance(PropertyList pList) 
+        throws PropertyException {
+        
+        CommonBorderPaddingBackground newInstance
+                = new CommonBorderPaddingBackground(pList);
+        CommonBorderPaddingBackground cachedInstance = null;
+        /* if padding-* and background-position-* resolve to absolute lengths
+         * the whole instance can be cached */
+        if ((newInstance.padding[BEFORE] == null || newInstance.padding[BEFORE].getLength().isAbsolute())
+                && (newInstance.padding[AFTER] == null || newInstance.padding[AFTER].getLength().isAbsolute())
+                && (newInstance.padding[START] == null || newInstance.padding[START].getLength().isAbsolute())
+                && (newInstance.padding[END] == null || newInstance.padding[END].getLength().isAbsolute())
+                && (newInstance.backgroundPositionHorizontal == null || newInstance.backgroundPositionHorizontal.isAbsolute())
+                && (newInstance.backgroundPositionVertical == null || newInstance.backgroundPositionVertical.isAbsolute())) {
+            cachedInstance = cache.fetch(newInstance);
+        }
+        
+        /* for non-cached, or not-yet-cached instances, preload the image */
+        if ((cachedInstance == null 
+                || cachedInstance == newInstance)
+                && !("".equals(newInstance.backgroundImage))) {
+            //Additional processing: preload image
+            String uri = URISpecification.getURL(newInstance.backgroundImage);
+            FOUserAgent userAgent = pList.getFObj().getUserAgent();
+            ImageManager manager = userAgent.getFactory().getImageManager();
+            ImageSessionContext sessionContext = userAgent.getImageSessionContext();
+            ImageInfo info;
+            try {
+                info = manager.getImageInfo(uri, sessionContext);
+                newInstance.backgroundImageInfo = info;
+            } catch (Exception e) {
+                Property.log.error("Background image not available: " + uri);
+            }
+            //TODO Report to caller so he can decide to throw an exception
+        }
+        
+        return (cachedInstance != null ? cachedInstance : newInstance);
+    }
+
     private void initBorderInfo(PropertyList pList, int side,
                     int colorProp, int styleProp, int widthProp, int paddingProp)
                 throws PropertyException {
+        
         padding[side] = pList.get(paddingProp).getCondLength();
         // If style = none, force width to 0, don't get Color (spec 7.7.20)
         int style = pList.get(styleProp).getEnum();
         if (style != Constants.EN_NONE) {
             FOUserAgent ua = pList.getFObj().getUserAgent();
-            setBorderInfo(new BorderInfo(style,
+            setBorderInfo(BorderInfo.getInstance(style,
                 pList.get(widthProp).getCondLength(),
                 pList.get(colorProp).getColor(ua)), side);
         }
+        
     }
 
     /**
@@ -291,7 +399,7 @@ public class CommonBorderPaddingBackground {
      * @param info the border information
      * @param side the side to apply the info to
      */
-    public void setBorderInfo(BorderInfo info, int side) {
+    private void setBorderInfo(BorderInfo info, int side) {
         this.borderInfo[side] = info;
     }
 
@@ -307,14 +415,6 @@ public class CommonBorderPaddingBackground {
         }
     }
 
-    /**
-     * Set padding.
-     * @param source the padding info to copy from
-     */
-    public void setPadding(CommonBorderPaddingBackground source) {
-        this.padding = source.padding;
-    }
-
     /**
      * @return the background image info object, null if there is
      *     no background image.
@@ -324,64 +424,103 @@ public class CommonBorderPaddingBackground {
     }
 
     /**
-     * @param bDiscard indicates whether the .conditionality component should be
+     * @param discard indicates whether the .conditionality component should be
      * considered (start of a reference-area)
+     * @return the width of the start-border, taking into account the specified conditionality
      */
-    public int getBorderStartWidth(boolean bDiscard) {
-        return getBorderWidth(START, bDiscard);
+    public int getBorderStartWidth(boolean discard) {
+        return getBorderWidth(START, discard);
     }
 
     /**
-     * @param bDiscard indicates whether the .conditionality component should be
+     * @param discard indicates whether the .conditionality component should be
      * considered (end of a reference-area)
+     * @return the width of the end-border, taking into account the specified conditionality
      */
-    public int getBorderEndWidth(boolean bDiscard) {
-        return getBorderWidth(END, bDiscard);
+    public int getBorderEndWidth(boolean discard) {
+        return getBorderWidth(END, discard);
     }
 
     /**
-     * @param bDiscard indicates whether the .conditionality component should be
+     * @param discard indicates whether the .conditionality component should be
      * considered (start of a reference-area)
+     * @return the width of the before-border, taking into account the specified conditionality
      */
-    public int getBorderBeforeWidth(boolean bDiscard) {
-        return getBorderWidth(BEFORE, bDiscard);
+    public int getBorderBeforeWidth(boolean discard) {
+        return getBorderWidth(BEFORE, discard);
     }
 
     /**
-     * @param bDiscard indicates whether the .conditionality component should be
+     * @param discard indicates whether the .conditionality component should be
      * considered (end of a reference-area)
+     * @return the width of the after-border, taking into account the specified conditionality
      */
-    public int getBorderAfterWidth(boolean bDiscard) {
-        return getBorderWidth(AFTER, bDiscard);
+    public int getBorderAfterWidth(boolean discard) {
+        return getBorderWidth(AFTER, discard);
     }
 
-    public int getPaddingStart(boolean bDiscard, PercentBaseContext context) {
-        return getPadding(START, bDiscard, context);
+    /**
+     * @param discard indicates whether the .conditionality component should be
+     * considered (start of a reference-area)
+     * @param context the context to evaluate percentage values
+     * @return the width of the start-padding, taking into account the specified conditionality
+     */
+    public int getPaddingStart(boolean discard, PercentBaseContext context) {
+        return getPadding(START, discard, context);
     }
 
-    public int getPaddingEnd(boolean bDiscard, PercentBaseContext context) {
-        return getPadding(END, bDiscard, context);
+    /**
+     * @param discard indicates whether the .conditionality component should be
+     * considered (start of a reference-area)
+     * @param context the context to evaluate percentage values
+     * @return the width of the end-padding, taking into account the specified conditionality
+     */
+    public int getPaddingEnd(boolean discard, PercentBaseContext context) {
+        return getPadding(END, discard, context);
     }
 
-    public int getPaddingBefore(boolean bDiscard, PercentBaseContext context) {
-        return getPadding(BEFORE, bDiscard, context);
+    /**
+     * @param discard indicates whether the .conditionality component should be
+     * considered (start of a reference-area)
+     * @param context the context to evaluate percentage values
+     * @return the width of the before-padding, taking into account the specified conditionality
+     */
+    public int getPaddingBefore(boolean discard, PercentBaseContext context) {
+        return getPadding(BEFORE, discard, context);
     }
 
-    public int getPaddingAfter(boolean bDiscard, PercentBaseContext context) {
-        return getPadding(AFTER, bDiscard, context);
+    /**
+     * @param discard indicates whether the .conditionality component should be
+     * considered (start of a reference-area)
+     * @param context the context to evaluate percentage values
+     * @return the width of the after-padding, taking into account the specified conditionality
+     */
+    public int getPaddingAfter(boolean discard, PercentBaseContext context) {
+        return getPadding(AFTER, discard, context);
     }
 
-    public int getBorderWidth(int side, boolean bDiscard) {
+    /**
+     * @param discard indicates whether the .conditionality component should be
+     * considered (end of a reference-area)
+     * @return the width of the start-border, taking into account the specified conditionality
+     */
+    public int getBorderWidth(int side, boolean discard) {
         if ((borderInfo[side] == null)
                 || (borderInfo[side].mStyle == Constants.EN_NONE)
                 || (borderInfo[side].mStyle == Constants.EN_HIDDEN)
-                || (bDiscard && borderInfo[side].mWidth.isDiscard())) {
+                || (discard && borderInfo[side].mWidth.isDiscard())) {
             return 0;
         } else {
             return borderInfo[side].mWidth.getLengthValue();
         }
     }
 
+    /**
+     * The border-color for the given side
+     * 
+     * @param side one of {@link #BEFORE}, {@link #AFTER}, {@link #START}, {@link #END}
+     * @return  the border-color for the given side
+     */
     public Color getBorderColor(int side) {
         if (borderInfo[side] != null) {
             return borderInfo[side].getColor();
@@ -390,6 +529,12 @@ public class CommonBorderPaddingBackground {
         }
     }
 
+    /**
+     * The border-style for the given side
+     * 
+     * @param side one of {@link #BEFORE}, {@link #AFTER}, {@link #START}, {@link #END}
+     * @return  the border-style for the given side
+     */
     public int getBorderStyle(int side) {
         if (borderInfo[side] != null) {
             return borderInfo[side].mStyle;
@@ -398,8 +543,18 @@ public class CommonBorderPaddingBackground {
         }
     }
 
-    public int getPadding(int side, boolean bDiscard, PercentBaseContext context) {
-        if ((padding[side] == null) || (bDiscard && padding[side].isDiscard())) {
+    /**
+     * Return the padding for the given side, taking into account
+     * the conditionality and evaluating any percentages in the given
+     * context.
+     * 
+     * @param side  one of {@link #BEFORE}, {@link #AFTER}, {@link #START}, {@link #END}
+     * @param discard   true if the conditionality component should be considered
+     * @param context   the context for percentage-resolution
+     * @return  the computed padding for the given side
+     */
+    public int getPadding(int side, boolean discard, PercentBaseContext context) {
+        if ((padding[side] == null) || (discard && padding[side].isDiscard())) {
             return 0;
         } else {
             return padding[side].getLengthValue(context);
@@ -418,27 +573,27 @@ public class CommonBorderPaddingBackground {
     /**
      * Return all the border and padding width in the inline progression
      * dimension.
-     * @param bDiscard the discard flag.
+     * @param discard the discard flag.
      * @param context for percentage evaluation.
      * @return all the padding and border width.
      */
-    public int getIPPaddingAndBorder(boolean bDiscard, PercentBaseContext context) {
-        return getPaddingStart(bDiscard, context)
-            + getPaddingEnd(bDiscard, context)
-            + getBorderStartWidth(bDiscard)
-            + getBorderEndWidth(bDiscard);
+    public int getIPPaddingAndBorder(boolean discard, PercentBaseContext context) {
+        return getPaddingStart(discard, context)
+            + getPaddingEnd(discard, context)
+            + getBorderStartWidth(discard)
+            + getBorderEndWidth(discard);
     }
 
     /**
      * Return all the border and padding height in the block progression
      * dimension.
-     * @param bDiscard the discard flag.
+     * @param discard the discard flag.
      * @param context for percentage evaluation
      * @return all the padding and border height.
      */
-    public int getBPPaddingAndBorder(boolean bDiscard, PercentBaseContext context) {
-        return getPaddingBefore(bDiscard, context) + getPaddingAfter(bDiscard, context)
-               + getBorderBeforeWidth(bDiscard) + getBorderAfterWidth(bDiscard);
+    public int getBPPaddingAndBorder(boolean discard, PercentBaseContext context) {
+        return getPaddingBefore(discard, context) + getPaddingAfter(discard, context)
+               + getBorderBeforeWidth(discard) + getBorderAfterWidth(discard);
     }
 
     /** {@inheritDoc} */
@@ -479,4 +634,54 @@ public class CommonBorderPaddingBackground {
         return (borderInfo[BEFORE] != null || borderInfo[AFTER] != null
                 || borderInfo[START] != null || borderInfo[END] != null);
     }
+    
+    /** {@inheritDoc} */
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        if (obj instanceof CommonBorderPaddingBackground) {
+            CommonBorderPaddingBackground cbpb = (CommonBorderPaddingBackground)obj;
+            return (this.backgroundAttachment == cbpb.backgroundAttachment
+                && this.backgroundColor == cbpb.backgroundColor
+                && this.backgroundImage.equals(cbpb.backgroundImage)
+                && this.backgroundPositionHorizontal == cbpb.backgroundPositionHorizontal
+                && this.backgroundPositionVertical == cbpb.backgroundPositionVertical
+                && this.backgroundRepeat == cbpb.backgroundRepeat
+                && this.borderInfo[BEFORE] == cbpb.borderInfo[BEFORE]
+                && this.borderInfo[AFTER] == cbpb.borderInfo[AFTER]
+                && this.borderInfo[START] == cbpb.borderInfo[START]
+                && this.borderInfo[END] == cbpb.borderInfo[END]
+                && this.padding[BEFORE] == cbpb.padding[BEFORE]
+                && this.padding[AFTER] == cbpb.padding[AFTER]
+                && this.padding[START] == cbpb.padding[START]
+                && this.padding[END] == cbpb.padding[END]);
+        }
+        
+        return false;
+    }
+    
+    /** {@inheritDoc} */
+    public int hashCode() {
+        if (this.hash == -1) {
+            int hash = 17;
+            hash = 37 * hash + backgroundAttachment;
+            hash = 37 * hash + (backgroundColor == null ? 0 : backgroundColor.hashCode());
+            hash = 37 * hash + (backgroundImage == null ? 0 : backgroundImage.hashCode());
+            hash = 37 * hash + (backgroundPositionHorizontal == null ? 0 : backgroundPositionHorizontal.hashCode());
+            hash = 37 * hash + (backgroundPositionVertical == null ? 0 : backgroundPositionVertical.hashCode());
+            hash = 37 * hash + backgroundRepeat;
+            hash = 37 * hash + (borderInfo[BEFORE] == null ? 0 : borderInfo[BEFORE].hashCode());
+            hash = 37 * hash + (borderInfo[AFTER] == null ? 0 : borderInfo[AFTER].hashCode());
+            hash = 37 * hash + (borderInfo[START] == null ? 0 : borderInfo[START].hashCode());
+            hash = 37 * hash + (borderInfo[END] == null ? 0 : borderInfo[END].hashCode());
+            hash = 37 * hash + (padding[BEFORE] == null ? 0 : padding[BEFORE].hashCode());
+            hash = 37 * hash + (padding[AFTER] == null ? 0 : padding[AFTER].hashCode());
+            hash = 37 * hash + (padding[START] == null ? 0 : padding[START].hashCode());
+            hash = 37 * hash + (padding[END] == null ? 0 : padding[END].hashCode());
+            this.hash = hash;
+        }
+        
+        return this.hash;
+    }
 }

src/java/org/apache/fop/fo/properties/CondLengthProperty.java

@@ -31,8 +31,16 @@ import org.apache.fop.fo.expr.PropertyException;
  * Superclass for properties that have conditional lengths
  */
 public class CondLengthProperty extends Property implements CompoundDatatype {
+    
+    /** cache holding canonical instances (for absolute conditional lengths) */
+    private static final PropertyCache cache = new PropertyCache(CondLengthProperty.class);
+    
+    /** components */
     private Property length;
     private EnumProperty conditionality;
+    
+    private boolean isCached = false;
+    private int hash = -1;
 
     /**
      * Inner class for creating instances of CondLengthProperty
@@ -71,6 +79,11 @@ public class CondLengthProperty extends Property implements CompoundDatatype {
      */
     public void setComponent(int cmpId, Property cmpnValue,
                              boolean bIsDefault) {
+        if (isCached) {
+            throw new IllegalStateException(
+                    "CondLengthProperty.setComponent() called on a cached value!");
+        }
+        
         if (cmpId == CP_LENGTH) {
             length = cmpnValue;
         } else if (cmpId == CP_CONDITIONALITY) {
@@ -144,7 +157,15 @@ public class CondLengthProperty extends Property implements CompoundDatatype {
      * @return this.condLength
      */
     public CondLengthProperty getCondLength() {
-        return this;
+        if (this.length.getLength().isAbsolute()) {
+            CondLengthProperty clp = (CondLengthProperty) cache.fetch(this);
+            if (clp == this) {
+                isCached = true;
+            }
+            return clp;
+        } else {
+            return this;
+        }
     }
 
     /**
@@ -162,4 +183,28 @@ public class CondLengthProperty extends Property implements CompoundDatatype {
         return this;
     }
 
+    /** {@inheritDoc} */
+    public boolean equals(Object obj) {
+        if (this == obj) {
+            return true;
+        }
+        
+        if (obj instanceof CondLengthProperty) {
+            CondLengthProperty clp = (CondLengthProperty)obj;
+            return (this.length == clp.length
+                    && this.conditionality == clp.conditionality);
+        }
+        return false;
+    }
+    
+    /** {@inheritDoc} */
+    public int hashCode() {
+        if (this.hash == -1) {
+            int hash = 17;
+            hash = 37 * hash + (length == null ? 0 : length.hashCode());
+            hash = 37 * hash + (conditionality == null ? 0 : conditionality.hashCode());
+            this.hash = hash;
+        }
+        return this.hash;
+    }
 }

src/java/org/apache/fop/fo/properties/PropertyCache.java

@@ -344,6 +344,32 @@ public final class PropertyCache {
         return (CommonFont) fetch((Object) cf);
     }
 
+    /**
+     *  Checks if the given {@link CommonBorderPaddingBackground} is present in the cache - 
+     *  if so, returns a reference to the cached instance. 
+     *  Otherwise the given object is added to the cache and returned.
+     *  
+     *  @param cbpb the CommonBorderPaddingBackground instance to check for
+     *  @return the cached instance
+     */
+    public final CommonBorderPaddingBackground fetch(CommonBorderPaddingBackground cbpb) {
+        
+        return (CommonBorderPaddingBackground) fetch((Object) cbpb);
+    }
+
+    /**
+     *  Checks if the given {@link CommonBorderPaddingBackground.BorderInfo} is present in the cache - 
+     *  if so, returns a reference to the cached instance. 
+     *  Otherwise the given object is added to the cache and returned.
+     *  
+     *  @param bi the BorderInfo instance to check for
+     *  @return the cached instance
+     */
+    public final CommonBorderPaddingBackground.BorderInfo fetch(CommonBorderPaddingBackground.BorderInfo bi) {
+        
+        return (CommonBorderPaddingBackground.BorderInfo) fetch((Object) bi);
+    }
+
     /** {@inheritDoc} */
     public String toString() {
         return super.toString() + "[runtimeType=" + this.runtimeType + "]";

src/java/org/apache/fop/fo/properties/StringProperty.java

@@ -21,6 +21,11 @@ package org.apache.fop.fo.properties;
 
 import org.apache.fop.fo.FObj;
 import org.apache.fop.fo.PropertyList;
+import org.apache.fop.fo.FOValidationEventProducer;
+import org.apache.fop.fo.ValidationException;
+import org.apache.fop.fo.expr.PropertyException;
+
+import java.util.Set;
 
 /**
  * Exists primarily as a container for its Maker inner class, which is
@@ -77,6 +82,48 @@ public final class StringProperty extends Property {
 
     }
 
+    /**
+     * Inner class dedicated to the "id" property, which should provide a random
+     * unique identifier as an initial value.
+     * The values for "id" are never cached, as they're typically valid for one
+     * document.
+     */
+    public static class IdMaker extends PropertyMaker {
+
+        /**
+         * @param propId    the id of the property for which the maker should be created
+         */
+        public IdMaker(int propId) {
+            super(propId);
+        }
+
+        /** {@inheritDoc} */
+        public Property make(PropertyList propertyList) throws PropertyException {
+            String newId = "FO_";
+            newId += propertyList.getFObj().getFOEventHandler().getNextId();
+            return new StringProperty(newId);
+        }
+        
+        /** {@inheritDoc} */
+        public Property make(PropertyList propertyList, 
+                             String value,
+                             FObj fo) throws PropertyException {
+            
+            Property idProp;
+            
+            //no parsing necessary; just return a new StringProperty
+            //TODO: Should we move validation here? (see FObj#checkId())
+            if ("".equals(value)) {
+                //if an empty string was specified, return the default
+                idProp = this.make(propertyList);
+            } else {
+                idProp = new StringProperty(value);
+            }
+            
+            return idProp;
+        }
+    }
+
     /** cache containing all canonical StringProperty instances */
     private static final PropertyCache cache = new PropertyCache(StringProperty.class);
     

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

@@ -99,6 +99,8 @@ class ActiveCell {
         private int totalLength;
         /** Length of the penalty ending this step, if any. */
         private int penaltyLength;
+        /** Value of the penalty ending this step, 0 if the step does not end on a penalty. */
+        private int penaltyValue;
         /**
          * One of {@link Constants#EN_AUTO}, {@link Constants#EN_COLUMN},
          * {@link Constants#EN_PAGE}, {@link Constants#EN_EVEN_PAGE},
@@ -127,6 +129,7 @@ class ActiveCell {
             this.contentLength = other.contentLength;
             this.totalLength   = other.totalLength;
             this.penaltyLength = other.penaltyLength;
+            this.penaltyValue  = other.penaltyValue;
             this.condBeforeContentLength = other.condBeforeContentLength;
             this.breakClass    = other.breakClass;
         }
@@ -287,6 +290,7 @@ class ActiveCell {
 
     private void gotoNextLegalBreak() {
         afterNextStep.penaltyLength = 0;
+        afterNextStep.penaltyValue = 0;
         afterNextStep.condBeforeContentLength = 0;
         afterNextStep.breakClass = Constants.EN_AUTO;
         boolean breakFound = false;
@@ -299,8 +303,9 @@ class ActiveCell {
                 if (el.getP() < KnuthElement.INFINITE) {
                     // First legal break point
                     breakFound = true;
-                    afterNextStep.penaltyLength = el.getW();
                     KnuthPenalty p = (KnuthPenalty) el;
+                    afterNextStep.penaltyLength = p.getW();
+                    afterNextStep.penaltyValue = p.getP();
                     if (p.isForcedBreak()) {
                         afterNextStep.breakClass = p.getBreakClass();
                     }
@@ -542,6 +547,13 @@ class ActiveCell {
         return keepWithNextStrength;
     }
 
+    int getPenaltyValue() {
+        if (includedInLastStep()) {
+            return nextStep.penaltyValue;
+        } else {
+            return previousStep.penaltyValue;
+        }
+    }
 
     /** {@inheritDoc} */
     public String toString() {

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

@@ -200,20 +200,11 @@ public class TableStepper {
             }
 
             //Put all involved grid units into a list
-            int stepPenalty = 0;
             List cellParts = new java.util.ArrayList(columnCount);
             for (Iterator iter = activeCells.iterator(); iter.hasNext();) {
                 ActiveCell activeCell = (ActiveCell) iter.next();
                 CellPart part = activeCell.createCellPart();
                 cellParts.add(part);
-                
-                //Record highest penalty value of part
-                if (part.end >= 0) {
-                    KnuthElement endEl = (KnuthElement)part.pgu.getElements().get(part.end);
-                    if (endEl instanceof KnuthPenalty) {
-                        stepPenalty = Math.max(stepPenalty, endEl.getP());
-                    }
-                }
             }
 
             //Create elements for step
@@ -242,9 +233,11 @@ public class TableStepper {
             }
 
             int strength = BlockLevelLayoutManager.KEEP_AUTO;
+            int stepPenalty = 0;
             for (Iterator iter = activeCells.iterator(); iter.hasNext();) {
                 ActiveCell activeCell = (ActiveCell) iter.next();
                 strength = Math.max(strength, activeCell.getKeepWithNextStrength());
+                stepPenalty = Math.max(stepPenalty, activeCell.getPenaltyValue());
             }
             if (!rowFinished) {
                 strength = Math.max(strength, rowGroup[activeRowIndex].getKeepTogetherStrength());

test/fotree/disabled-testcases.xml

@@ -28,14 +28,4 @@
     <description>The code currently evaluates this function according to the column in which the 
     marker appears in the source document, rather than the column it is retrieved in.</description>
   </testcase>
-  <testcase>
-    <name>column-number_negative-or-zero</name>
-    <file>column-number_negative-or-zero.fo</file>
-    <description></description>
-  </testcase>
-  <testcase>
-    <name>column-number_non-integer</name>
-    <file>column-number_non-integer.fo</file>
-    <description></description>
-  </testcase>
 </disabled-testcases>

test/fotree/testcases/id_auto.fo

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format" xmlns:test="http://xmlgraphics.apache.org/fop/test">
+  <test:assert property="id" expected="FO_1" />
+  <fo:layout-master-set>
+    <test:assert property="id" expected="FO_2" />
+    <fo:simple-page-master master-name="simpleA4" page-height="29.7cm" page-width="21cm" margin-top="2cm" margin-bottom="2cm" margin-left="2cm" margin-right="2cm">
+      <test:assert property="id" expected="FO_3" />
+      <fo:region-body>
+        <test:assert property="id" expected="FO_4" />
+      </fo:region-body>
+    </fo:simple-page-master>
+  </fo:layout-master-set>
+  <fo:page-sequence master-reference="simpleA4">
+    <test:assert property="id" expected="FO_5" />
+    <fo:flow flow-name="xsl-region-body">
+      <test:assert property="id" expected="FO_6" />
+      <fo:block font-size="14pt" id="block">Hello
+        <test:assert property="id" expected="block"/>
+        <fo:inline> World!
+          <test:assert property="id" expected="FO_7" />
+        </fo:inline>
+      </fo:block>
+      <fo:block font-family="ZapfDingbats">&#2702;
+        <test:assert property="id" expected="FO_8" />
+      </fo:block>
+    </fo:flow>
+  </fo:page-sequence>
+</fo:root>