You are viewing a plain text version of this content. The canonical link for it is here.
Posted to fop-commits@xmlgraphics.apache.org by je...@apache.org on 2008/06/18 11:02:50 UTC

svn commit: r669118 - in /xmlgraphics/fop/branches/fop-0_95: ./ src/java/org/apache/fop/fo/flow/ src/java/org/apache/fop/fo/flow/table/ src/java/org/apache/fop/fo/properties/ src/java/org/apache/fop/layoutmgr/ src/java/org/apache/fop/layoutmgr/list/ sr...

Author: jeremias
Date: Wed Jun 18 02:02:45 2008
New Revision: 669118

URL: http://svn.apache.org/viewvc?rev=669118&view=rev
Log:
Bugzilla #44412:
Regression fix for empty pages caused by multiple collapsible breaks.
No more empty block areas if a break-before occurs on the first child of an FO to match the behaviour of tables and other FO implementations (clarification by XSL WG pending).
Added an accessor interface for break-before/-after to avoid long if..else lists in BlockStackingLayoutManager.

Added:
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/properties/BreakPropertySet.java   (with props)
    xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412_2.xml   (with props)
Modified:
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/Block.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/BlockContainer.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/ListBlock.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/ListItem.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/Table.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/TableAndCaption.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/TableRow.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/LayoutContext.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/list/ListItemLayoutManager.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/table/TableLayoutManager.java
    xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/util/BreakUtil.java
    xmlgraphics/fop/branches/fop-0_95/status.xml
    xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412.xml

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/Block.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/Block.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/Block.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/Block.java Wed Jun 18 02:02:45 2008
@@ -32,6 +32,7 @@
 import org.apache.fop.fo.NullCharIterator;
 import org.apache.fop.fo.PropertyList;
 import org.apache.fop.fo.ValidationException;
+import org.apache.fop.fo.properties.BreakPropertySet;
 import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
 import org.apache.fop.fo.properties.CommonFont;
 import org.apache.fop.fo.properties.CommonHyphenation;
@@ -43,7 +44,7 @@
  /**
   * Class modelling the fo:block object.
   */
-public class Block extends FObjMixed {
+public class Block extends FObjMixed implements BreakPropertySet {
 
     // used for FO validation
     private boolean blockOrInlineItemFound = false;

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/BlockContainer.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/BlockContainer.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/BlockContainer.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/BlockContainer.java Wed Jun 18 02:02:45 2008
@@ -28,6 +28,7 @@
 import org.apache.fop.fo.FObj;
 import org.apache.fop.fo.PropertyList;
 import org.apache.fop.fo.ValidationException;
+import org.apache.fop.fo.properties.BreakPropertySet;
 import org.apache.fop.fo.properties.CommonAbsolutePosition;
 import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
 import org.apache.fop.fo.properties.CommonMarginBlock;
@@ -37,7 +38,7 @@
 /**
  * Class modelling the fo:block-container object.
  */
-public class BlockContainer extends FObj {
+public class BlockContainer extends FObj implements BreakPropertySet {
     // The value of properties relevant for fo:block-container.
     private CommonAbsolutePosition commonAbsolutePosition;
     private CommonBorderPaddingBackground commonBorderPaddingBackground;

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/ListBlock.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/ListBlock.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/ListBlock.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/ListBlock.java Wed Jun 18 02:02:45 2008
@@ -27,6 +27,7 @@
 import org.apache.fop.fo.FObj;
 import org.apache.fop.fo.PropertyList;
 import org.apache.fop.fo.ValidationException;
+import org.apache.fop.fo.properties.BreakPropertySet;
 import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
 import org.apache.fop.fo.properties.CommonMarginBlock;
 import org.apache.fop.fo.properties.KeepProperty;
@@ -34,7 +35,7 @@
 /**
  * Class modelling the fo:list-block object.
  */
-public class ListBlock extends FObj {
+public class ListBlock extends FObj implements BreakPropertySet {
     // The value of properties relevant for fo:list-block.
     private CommonBorderPaddingBackground commonBorderPaddingBackground;
     private CommonMarginBlock commonMarginBlock;

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/ListItem.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/ListItem.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/ListItem.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/ListItem.java Wed Jun 18 02:02:45 2008
@@ -26,6 +26,7 @@
 import org.apache.fop.fo.FObj;
 import org.apache.fop.fo.PropertyList;
 import org.apache.fop.fo.ValidationException;
+import org.apache.fop.fo.properties.BreakPropertySet;
 import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
 import org.apache.fop.fo.properties.CommonMarginBlock;
 import org.apache.fop.fo.properties.KeepProperty;
@@ -33,7 +34,7 @@
 /**
  * Class modelling the fo:list-item object.
  */
-public class ListItem extends FObj {
+public class ListItem extends FObj implements BreakPropertySet {
     // The value of properties relevant for fo:list-item.
     private CommonBorderPaddingBackground commonBorderPaddingBackground;
     private CommonMarginBlock commonMarginBlock;

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/Table.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/Table.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/Table.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/Table.java Wed Jun 18 02:02:45 2008
@@ -22,6 +22,8 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import org.xml.sax.Locator;
+
 import org.apache.fop.apps.FOPException;
 import org.apache.fop.datatypes.Length;
 import org.apache.fop.datatypes.ValidationPercentBaseContext;
@@ -29,18 +31,18 @@
 import org.apache.fop.fo.PropertyList;
 import org.apache.fop.fo.StaticPropertyList;
 import org.apache.fop.fo.ValidationException;
+import org.apache.fop.fo.properties.BreakPropertySet;
 import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
 import org.apache.fop.fo.properties.CommonMarginBlock;
 import org.apache.fop.fo.properties.KeepProperty;
 import org.apache.fop.fo.properties.LengthPairProperty;
 import org.apache.fop.fo.properties.LengthRangeProperty;
 import org.apache.fop.fo.properties.TableColLength;
-import org.xml.sax.Locator;
 
 /**
  * Class modelling the fo:table object.
  */
-public class Table extends TableFObj implements ColumnNumberManagerHolder {
+public class Table extends TableFObj implements ColumnNumberManagerHolder, BreakPropertySet {
 
     /** properties */
     private CommonBorderPaddingBackground commonBorderPaddingBackground;

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/TableAndCaption.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/TableAndCaption.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/TableAndCaption.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/TableAndCaption.java Wed Jun 18 02:02:45 2008
@@ -31,7 +31,7 @@
  * Class modelling the fo:table-and-caption property.
  * @todo needs implementation
  */
-public class TableAndCaption extends FObj {
+public class TableAndCaption extends FObj /*implements BreakPropertySet*/ {
     // The value of properties relevant for fo:table-and-caption.
     // Unused but valid items, commented out for performance:
     //     private CommonAccessibility commonAccessibility;

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/TableRow.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/TableRow.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/TableRow.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/flow/table/TableRow.java Wed Jun 18 02:02:45 2008
@@ -19,21 +19,23 @@
 
 package org.apache.fop.fo.flow.table;
 
+import org.xml.sax.Attributes;
+import org.xml.sax.Locator;
+
 import org.apache.fop.apps.FOPException;
 import org.apache.fop.datatypes.Length;
 import org.apache.fop.fo.FONode;
 import org.apache.fop.fo.PropertyList;
 import org.apache.fop.fo.ValidationException;
+import org.apache.fop.fo.properties.BreakPropertySet;
 import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
 import org.apache.fop.fo.properties.KeepProperty;
 import org.apache.fop.fo.properties.LengthRangeProperty;
-import org.xml.sax.Attributes;
-import org.xml.sax.Locator;
 
 /**
  * Class modelling the fo:table-row object.
  */
-public class TableRow extends TableCellContainer {
+public class TableRow extends TableCellContainer implements BreakPropertySet {
     // The value of properties relevant for fo:table-row.
     private LengthRangeProperty blockProgressionDimension;
     private CommonBorderPaddingBackground commonBorderPaddingBackground;

Added: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/properties/BreakPropertySet.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/properties/BreakPropertySet.java?rev=669118&view=auto
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/properties/BreakPropertySet.java (added)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/properties/BreakPropertySet.java Wed Jun 18 02:02:45 2008
@@ -0,0 +1,33 @@
+/*
+ * 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.fo.properties;
+
+/**
+ * Defines property access methods for the break-before and break-after properties.
+ */
+public interface BreakPropertySet {
+
+    /** @return the "break-after" property. */
+    int getBreakAfter();
+
+    /** @return the "break-before" property. */
+    int getBreakBefore();
+    
+}

Propchange: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/properties/BreakPropertySet.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/fo/properties/BreakPropertySet.java
------------------------------------------------------------------------------
    svn:keywords = Id

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java Wed Jun 18 02:02:45 2008
@@ -21,6 +21,7 @@
 
 import java.awt.Point;
 import java.awt.geom.Rectangle2D;
+import java.lang.ref.WeakReference;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.ListIterator;
@@ -253,12 +254,12 @@
         LinkedList returnList = new LinkedList();
         
         if (!breakBeforeServed) {
-            try {
+            breakBeforeServed = true;
+            if (!context.suppressBreakBefore()) {
                 if (addKnuthElementsForBreakBefore(returnList, context)) {
+                    this.childLMForBreakSkip = new WeakReference(getChildLM());
                     return returnList;
                 }
-            } finally {
-                breakBeforeServed = true;
             }
         }
 
@@ -282,6 +283,10 @@
                 childLC.setStackLimitBP(MinOptMax.subtract(context.getStackLimitBP(), stackLimit));
                 childLC.setRefIPD(relDims.ipd);
                 childLC.setWritingMode(getBlockContainerFO().getWritingMode());
+                if (this.childLMForBreakSkip != null && curLM == this.childLMForBreakSkip.get()) {
+                    childLC.setFlags(LayoutContext.SUPPRESS_BREAK_BEFORE);
+                    //Handled already by the parent (break collapsing, see above)
+                }
 
                 // get elements from curLM
                 returnedList = curLM.getNextKnuthElements(childLC, alignment);

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java Wed Jun 18 02:02:45 2008
@@ -19,6 +19,8 @@
 
 package org.apache.fop.layoutmgr;
 
+import java.lang.ref.Reference;
+import java.lang.ref.WeakReference;
 import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
@@ -31,11 +33,13 @@
 import org.apache.fop.area.Block;
 import org.apache.fop.area.BlockParent;
 import org.apache.fop.fo.FObj;
+import org.apache.fop.fo.properties.BreakPropertySet;
 import org.apache.fop.fo.properties.CommonBorderPaddingBackground;
 import org.apache.fop.fo.properties.SpaceProperty;
 import org.apache.fop.layoutmgr.inline.InlineLayoutManager;
 import org.apache.fop.layoutmgr.inline.LineLayoutManager;
 import org.apache.fop.traits.MinOptMax;
+import org.apache.fop.util.BreakUtil;
 
 /**
  * Base LayoutManager class for all areas which stack their child
@@ -66,6 +70,11 @@
     protected LinkedList storedList = null;
     /** Indicates whether break before has been served or not */
     protected boolean breakBeforeServed = false;
+    /**
+     * Holds a weak reference on the first layout manager for optionally skipping
+     * a break-before that has already been handled by the parent.
+     */
+    protected Reference childLMForBreakSkip;
     /** Indicates whether the first visible mark has been returned by this LM, yet */
     protected boolean firstVisibleMarkServed = false;
     /** Reference IPD available */
@@ -246,12 +255,12 @@
         LinkedList returnList = new LinkedList();
 
         if (!breakBeforeServed) {
-            try {
+            breakBeforeServed = true;
+            if (!context.suppressBreakBefore()) {
                 if (addKnuthElementsForBreakBefore(returnList, context)) {
+                    this.childLMForBreakSkip = new WeakReference(getChildLM());
                     return returnList;
                 }
-            } finally {
-                breakBeforeServed = true;
             }
         }
 
@@ -284,6 +293,10 @@
                 childLC.setStackLimitBP(context.getStackLimitBP());
                 childLC.setRefIPD(referenceIPD);
             }
+            if (this.childLMForBreakSkip != null && curLM == this.childLMForBreakSkip.get()) {
+                childLC.setFlags(LayoutContext.SUPPRESS_BREAK_BEFORE);
+                //Handled already by the parent (break collapsing, see above)
+            }
 
             // get elements from curLM
             returnedList = curLM.getNextKnuthElements(childLC, alignment);
@@ -946,18 +959,7 @@
      */
     protected boolean addKnuthElementsForBreakBefore(LinkedList returnList, 
             LayoutContext context) {
-        int breakBefore = -1;
-        if (fobj instanceof org.apache.fop.fo.flow.Block) {
-            breakBefore = ((org.apache.fop.fo.flow.Block) fobj).getBreakBefore();
-        } else if (fobj instanceof org.apache.fop.fo.flow.BlockContainer) {
-            breakBefore = ((org.apache.fop.fo.flow.BlockContainer) fobj).getBreakBefore();
-        } else if (fobj instanceof org.apache.fop.fo.flow.ListBlock) {
-            breakBefore = ((org.apache.fop.fo.flow.ListBlock) fobj).getBreakBefore();
-        } else if (fobj instanceof org.apache.fop.fo.flow.ListItem) {
-            breakBefore = ((org.apache.fop.fo.flow.ListItem) fobj).getBreakBefore();
-        } else if (fobj instanceof org.apache.fop.fo.flow.table.Table) {
-            breakBefore = ((org.apache.fop.fo.flow.table.Table) fobj).getBreakBefore();
-        }
+        int breakBefore = getBreakBefore(true);
         if (breakBefore == EN_PAGE
                 || breakBefore == EN_COLUMN 
                 || breakBefore == EN_EVEN_PAGE 
@@ -972,6 +974,37 @@
     }
 
     /**
+     * Returns the break-before value of the current formatting object.
+     * @param mergedWithChildren true if any break-before on first children should be checked, too
+     * @return the break-before value (Constants.EN_*)
+     */
+    protected int getBreakBefore(boolean mergedWithChildren) {
+        int breakBefore = getBreakBefore();
+        if (mergedWithChildren /* uncomment to only partially merge: && breakBefore != EN_AUTO*/) {
+            LayoutManager lm = getChildLM();
+            //It is assumed this is only called when the first LM is active.
+            if (lm instanceof BlockStackingLayoutManager) {
+                BlockStackingLayoutManager bslm = (BlockStackingLayoutManager)lm;
+                breakBefore = BreakUtil.compareBreakClasses(
+                        breakBefore, bslm.getBreakBefore(true));
+            }
+        }
+        return breakBefore;
+    }
+    
+    /**
+     * Returns the break-before value of the current formatting object.
+     * @return the break-before value (Constants.EN_*)
+     */
+    protected int getBreakBefore() {
+        int breakBefore = EN_AUTO;
+        if (fobj instanceof BreakPropertySet) {
+            breakBefore = ((BreakPropertySet)fobj).getBreakBefore();
+        }
+        return breakBefore;
+    }
+
+    /**
      * Creates Knuth elements for break-after and adds them to the return list.
      * @param returnList return list to add the additional elements to
      * @param context the layout context
@@ -980,16 +1013,8 @@
     protected boolean addKnuthElementsForBreakAfter(LinkedList returnList, 
             LayoutContext context) {
         int breakAfter = -1;
-        if (fobj instanceof org.apache.fop.fo.flow.Block) {
-            breakAfter = ((org.apache.fop.fo.flow.Block) fobj).getBreakAfter();
-        } else if (fobj instanceof org.apache.fop.fo.flow.BlockContainer) {
-            breakAfter = ((org.apache.fop.fo.flow.BlockContainer) fobj).getBreakAfter();
-        } else if (fobj instanceof org.apache.fop.fo.flow.ListBlock) {
-            breakAfter = ((org.apache.fop.fo.flow.ListBlock) fobj).getBreakAfter();
-        } else if (fobj instanceof org.apache.fop.fo.flow.ListItem) {
-            breakAfter = ((org.apache.fop.fo.flow.ListItem) fobj).getBreakAfter();
-        } else if (fobj instanceof org.apache.fop.fo.flow.table.Table) {
-            breakAfter = ((org.apache.fop.fo.flow.table.Table) fobj).getBreakAfter();
+        if (fobj instanceof BreakPropertySet) {
+            breakAfter = ((BreakPropertySet)fobj).getBreakAfter();
         }
         if (breakAfter == EN_PAGE
                 || breakAfter == EN_COLUMN

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/LayoutContext.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/LayoutContext.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/LayoutContext.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/LayoutContext.java Wed Jun 18 02:02:45 2008
@@ -47,12 +47,10 @@
     public static final int CHECK_REF_AREA = 0x08;
 
     /**
-     * If this flag is set, it indicates that any leading fo:character
-     * objects with suppress-at-line-break="suppress" should not generate
-     * areas. This is the case at the beginning of each new LineArea
-     * except the first.
+     * If this flag is set, it indicates that any break-before values other than "auto" should
+     * not cause a mandatory break as this break was already handled by a parent layout manager.
      */
-    public static final int SUPPRESS_LEADING_SPACE = 0x10;
+    public static final int SUPPRESS_BREAK_BEFORE = 0x10;
     public static final int FIRST_AREA = 0x20;
     public static final int TRY_HYPHENATE = 0x40;
     public static final int LAST_AREA = 0x80;
@@ -224,8 +222,8 @@
         return ((this.flags & LAST_AREA) != 0);
     }
 
-    public boolean suppressLeadingSpace() {
-        return ((this.flags & SUPPRESS_LEADING_SPACE) != 0);
+    public boolean suppressBreakBefore() {
+        return ((this.flags & SUPPRESS_BREAK_BEFORE) != 0);
     }
 
     public boolean isKeepWithNextPending() {
@@ -590,7 +588,7 @@
         + "\nSpace Adjust: \t" + getSpaceAdjust()
         + "\nIPD Adjust: \t" + getIPDAdjust()
         + "\nResolve Leading Space: \t" + resolveLeadingSpace()
-        + "\nSuppress Leading Space: \t" + suppressLeadingSpace()
+        + "\nSuppress Break Before: \t" + suppressBreakBefore()
         + "\nIs First Area: \t" + isFirstArea()
         + "\nStarts New Area: \t" + startsNewArea()
         + "\nIs Last Area: \t" + isLastArea()

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/list/ListItemLayoutManager.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/list/ListItemLayoutManager.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/list/ListItemLayoutManager.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/list/ListItemLayoutManager.java Wed Jun 18 02:02:45 2008
@@ -195,12 +195,11 @@
         LinkedList returnList = new LinkedList();
         
         if (!breakBeforeServed) {
-            try {
+            breakBeforeServed = true;
+            if (!context.suppressBreakBefore()) {
                 if (addKnuthElementsForBreakBefore(returnList, context)) {
                     return returnList;
                 }
-            } finally {
-                breakBeforeServed = true;
             }
         }
 

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/table/TableLayoutManager.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/table/TableLayoutManager.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/table/TableLayoutManager.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/layoutmgr/table/TableLayoutManager.java Wed Jun 18 02:02:45 2008
@@ -26,6 +26,7 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.apache.fop.area.Area;
 import org.apache.fop.area.Block;
 import org.apache.fop.datatypes.LengthBase;
@@ -265,12 +266,14 @@
         }
         addKnuthElementsForSpaceAfter(returnList, alignment);
 
-        //addKnuthElementsForBreakBefore(returnList, context);
-        int breakBefore = BreakUtil.compareBreakClasses(getTable().getBreakBefore(),
-                childLC.getBreakBefore());
-        if (breakBefore != Constants.EN_AUTO) {
-            returnList.addFirst(new BreakElement(getAuxiliaryPosition(), 
-                    0, -KnuthElement.INFINITE, breakBefore, context));
+        if (!context.suppressBreakBefore()) {
+            //addKnuthElementsForBreakBefore(returnList, context);
+            int breakBefore = BreakUtil.compareBreakClasses(getTable().getBreakBefore(),
+                    childLC.getBreakBefore());
+            if (breakBefore != Constants.EN_AUTO) {
+                returnList.addFirst(new BreakElement(getAuxiliaryPosition(), 
+                        0, -KnuthElement.INFINITE, breakBefore, context));
+            }
         }
 
         //addKnuthElementsForBreakAfter(returnList, context);

Modified: xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/util/BreakUtil.java
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/util/BreakUtil.java?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/util/BreakUtil.java (original)
+++ xmlgraphics/fop/branches/fop-0_95/src/java/org/apache/fop/util/BreakUtil.java Wed Jun 18 02:02:45 2008
@@ -36,7 +36,8 @@
         case Constants.EN_PAGE:      return 2;
         case Constants.EN_EVEN_PAGE: return 3;
         case Constants.EN_ODD_PAGE:  return 3;
-        default: throw new IllegalArgumentException();
+        default: throw new IllegalArgumentException(
+                "Illegal value for breakClass: " + breakClass);
         }
     }
 

Modified: xmlgraphics/fop/branches/fop-0_95/status.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/status.xml?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/status.xml (original)
+++ xmlgraphics/fop/branches/fop-0_95/status.xml Wed Jun 18 02:02:45 2008
@@ -60,6 +60,9 @@
       -->
     <!--/release-->
     <release version="0.95" date="TBD">
+      <action context="Layout" dev="JM" type="fix" fixes-bug="44412">
+        Regression bugfix: Multiple collapsable breaks don't cause empty pages anymore.
+      </action>
       <action context="Renderers" dev="JM" type="fix">
         Fixed resolution handling inside AWT preview dialog.
       </action>

Modified: xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412.xml?rev=669118&r1=669117&r2=669118&view=diff
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412.xml (original)
+++ xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412.xml Wed Jun 18 02:02:45 2008
@@ -20,8 +20,7 @@
   <info>
     <p>
       This test checks Bugzilla #44412 where a break-before on the first child of an otherwise
-      empty block is set. It is expected that the parent block creates two areas, the first with
-      only border-before on the first page and zero bpd.
+      empty block is set.
     </p>
   </info>
   <fo>
@@ -44,20 +43,12 @@
     </fo:root>
   </fo>
   <checks>
-    <eval expected="2" xpath="count(//block[@prod-id = 'b1'])"/>
-    <eval expected="4000 4000 4000 0" xpath="(//block[@prod-id = 'b1'])[1]/@bap"/>
-    <eval expected="4000 4000 0 4000" xpath="(//block[@prod-id = 'b1'])[2]/@bap"/>
-    <eval expected="0" xpath="(//block[@prod-id = 'b1'])[1]/@bpd"/>
-    <eval expected="43200" xpath="(//block[@prod-id = 'b1'])[2]/@bpd"/>
+    <eval expected="1" xpath="count(//block[@prod-id = 'b1'])"/>
+    <eval expected="4000 4000 4000 4000" xpath="(//block[@prod-id = 'b1'])[1]/@bap"/>
+    <eval expected="43200" xpath="(//block[@prod-id = 'b1'])[1]/@bpd"/>
     
     <element-list category="breaker" index="0">
       <box w="14400"/>
-      <penalty w="0" p="0"/>
-      <box w="0" aux="true"/>
-      <penalty w="0" p="INF"/>
-      <glue w="4000"/> <!-- border-before -->
-      <box w="0"/> <!-- first block area of the "b1" block with zero bpd -->
-      
       <skip>3</skip>
     </element-list>
   </checks>

Added: xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412_2.xml
URL: http://svn.apache.org/viewvc/xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412_2.xml?rev=669118&view=auto
==============================================================================
--- xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412_2.xml (added)
+++ xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412_2.xml Wed Jun 18 02:02:45 2008
@@ -0,0 +1,151 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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$ -->
+<testcase>
+  <info>
+    <p>
+      This test checks for the correct behaviour of multiple breaks at the same break possibility.
+    </p>
+  </info>
+  <fo>
+    <fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+      <fo:layout-master-set>
+        <fo:simple-page-master master-name="page" page-height="5in" page-width="5in" margin="20pt">
+          <fo:region-body/>
+          <fo:region-after extent="1.2em"/>
+        </fo:simple-page-master>
+      </fo:layout-master-set>
+      <fo:page-sequence master-reference="page" id="ps1" initial-page-number="1" force-page-count="no-force">
+        <fo:static-content flow-name="xsl-region-after">
+          <fo:block>page: <fo:page-number/></fo:block>
+        </fo:static-content>
+        <fo:flow flow-name="xsl-region-body">
+          <fo:block id="b1">Before the break</fo:block>
+          <fo:block break-before="page" break-after="column">
+            <fo:block id="b2" break-before="page">
+              This text should be on page 2.
+            </fo:block>
+            <fo:block>
+              <fo:block id="b3" break-after="page">
+                Inner block with break-after.
+              </fo:block>
+            </fo:block>  
+          </fo:block>
+          <fo:block id="b4">This text should be on page 3.</fo:block>
+        </fo:flow>
+      </fo:page-sequence>
+      <fo:page-sequence master-reference="page" id="ps2" initial-page-number="1" force-page-count="no-force">
+        <fo:static-content flow-name="xsl-region-after">
+          <fo:block>page: <fo:page-number/></fo:block>
+        </fo:static-content>
+        <fo:flow flow-name="xsl-region-body">
+          <fo:block id="b11">Before the break</fo:block>
+          <fo:block-container break-before="page" break-after="column">
+            <fo:block-container break-before="page">
+              <fo:block id="b12">This text should be on page 2.</fo:block>
+            </fo:block-container>
+            <fo:block>
+              <fo:block id="b13" break-after="page">
+                Inner block with break-after.
+              </fo:block>
+            </fo:block>  
+          </fo:block-container>
+          <fo:block id="b14">This text should be on page 3.</fo:block>
+        </fo:flow>
+      </fo:page-sequence>
+      <fo:page-sequence master-reference="page" id="ps3" initial-page-number="1" force-page-count="no-force">
+        <fo:static-content flow-name="xsl-region-after">
+          <fo:block>page: <fo:page-number/></fo:block>
+        </fo:static-content>
+        <fo:flow flow-name="xsl-region-body">
+          <fo:block id="b21">Before the break</fo:block>
+          <fo:block break-before="page" break-after="column">
+            <fo:table table-layout="fixed" width="100%" break-before="page">
+              <fo:table-column column-width="proportional-column-width(1)"/>
+              <fo:table-body>
+                <fo:table-row>
+                  <fo:table-cell>
+                    <fo:block id="b22">This text should be on page 2.</fo:block>
+                  </fo:table-cell>
+                </fo:table-row>
+              </fo:table-body>
+            </fo:table>
+            <fo:block>
+              <fo:table table-layout="fixed" width="100%" break-after="page">
+                <fo:table-column column-width="proportional-column-width(1)"/>
+                <fo:table-body>
+                  <fo:table-row>
+                    <fo:table-cell>
+                      <fo:block id="b23">Inner block in table with break-after.</fo:block>
+                    </fo:table-cell>
+                  </fo:table-row>
+                </fo:table-body>
+              </fo:table>
+            </fo:block>  
+          </fo:block>
+          <fo:block id="b24">This text should be on page 3.</fo:block>
+        </fo:flow>
+      </fo:page-sequence>
+      <fo:page-sequence master-reference="page" id="ps4" initial-page-number="1" force-page-count="no-force">
+        <fo:static-content flow-name="xsl-region-after">
+          <fo:block>page: <fo:page-number/></fo:block>
+        </fo:static-content>
+        <fo:flow flow-name="xsl-region-body">
+          <fo:block id="b31">Before the break</fo:block>
+          <fo:block break-before="page" break-after="column">
+            <fo:list-block break-before="page">
+              <fo:list-item>
+                <fo:list-item-label end-indent="label-end()">
+                  <fo:block>+</fo:block>
+                </fo:list-item-label>
+                <fo:list-item-body start-indent="body-start()">
+                  <fo:block id="b32">This text should be on page 2.</fo:block>
+                </fo:list-item-body>
+              </fo:list-item>
+              <fo:list-item break-after="page">
+                <fo:list-item-label end-indent="label-end()">
+                  <fo:block>+</fo:block>
+                </fo:list-item-label>
+                <fo:list-item-body start-indent="body-start()">
+                  <fo:block id="b33">Inner block in list-item with break-after.</fo:block>
+                </fo:list-item-body>
+              </fo:list-item>
+            </fo:list-block>
+          </fo:block>
+          <fo:block id="b34">This text should be on page 3.</fo:block>
+        </fo:flow>
+      </fo:page-sequence>
+    </fo:root>
+  </fo>
+  <checks>
+    <eval expected="1" xpath="//block[@prod-id = 'b1']/ancestor::pageViewport/@nr"/>
+    <eval expected="2" xpath="//block[@prod-id = 'b2']/ancestor::pageViewport/@nr"/>
+    <eval expected="2" xpath="//block[@prod-id = 'b3']/ancestor::pageViewport/@nr"/>
+    <eval expected="3" xpath="//block[@prod-id = 'b4']/ancestor::pageViewport/@nr"/>
+
+    <eval expected="1" xpath="//block[@prod-id = 'b11']/ancestor::pageViewport/@nr"/>
+    <eval expected="2" xpath="//block[@prod-id = 'b12']/ancestor::pageViewport/@nr"/>
+    <eval expected="2" xpath="//block[@prod-id = 'b13']/ancestor::pageViewport/@nr"/>
+    <eval expected="3" xpath="//block[@prod-id = 'b14']/ancestor::pageViewport/@nr"/>
+
+    <eval expected="1" xpath="//block[@prod-id = 'b21']/ancestor::pageViewport/@nr"/>
+    <eval expected="2" xpath="//block[@prod-id = 'b22']/ancestor::pageViewport/@nr"/>
+    <eval expected="2" xpath="//block[@prod-id = 'b23']/ancestor::pageViewport/@nr"/>
+    <eval expected="3" xpath="//block[@prod-id = 'b24']/ancestor::pageViewport/@nr"/>
+  </checks>
+</testcase>

Propchange: xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412_2.xml
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: xmlgraphics/fop/branches/fop-0_95/test/layoutengine/standard-testcases/block_break-before_bug44412_2.xml
------------------------------------------------------------------------------
    svn:keywords = Id



---------------------------------------------------------------------
To unsubscribe, e-mail: fop-commits-unsubscribe@xmlgraphics.apache.org
For additional commands, e-mail: fop-commits-help@xmlgraphics.apache.org


Re: svn commit: r669118 - in /xmlgraphics/fop/branches/fop-0_95: ./ src/java/org/apache/fop/fo/flow/ src/java/org/apache/fop/fo/flow/table/ src/java/org/apache/fop/fo/properties/ src/java/org/apache/fop/layoutmgr/ src/java/org/apache/fop/layoutmgr/list/ sr...

Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
Thanks for your feedback!

On 19.06.2008 12:36:30 Vincent Hennebert wrote:
> Hi,
> 
> > Author: jeremias
> > Date: Wed Jun 18 02:02:45 2008
> > New Revision: 669118
> > 
> > URL: http://svn.apache.org/viewvc?rev=669118&view=rev
> > Log:
> > Bugzilla #44412:
> > Regression fix for empty pages caused by multiple collapsible breaks.
> > No more empty block areas if a break-before occurs on the first child of an FO to match the behaviour of tables and other FO implementations (clarification by XSL WG pending).
> > Added an accessor interface for break-before/-after to avoid long if..else lists in BlockStackingLayoutManager.
> 
> I’m afraid I’m not happy with this change. There are a couple of issues
> that should be looked at IMO.
> 
> Simple things first:
> - BlockStackingLM.getBreakBefore(boolean) is always given true as its
>   parameter. Why specifying a parameter then? Plus this method is called
>   only within BlockStackingLM, so it should be made private (at least
>   for now).

Same reason as usual. Refactoring, planning for the "future", then at
the end not noticing that some people like it more cleaned up. ;-)

> - BlockStackingLM.getBreakBefore(): same here, this method should be
>   made private. And if fobj doesn’t implement BreakPropertySet, why
>   should EN_AUTO be returned? If this method is called on an object that
>   doesn’t support it, I guess it should rather throw an exception than
>   silently return a default value. Otherwise that makes it difficult to
>   track down errors.

BlockStackingLM is base class for FOs not supporting breaks (ex.
footnote-body or static-content). addKnuthElementsForBreak*() is called
anyway. EN_AUTO means no break condition and no action. So I think this
is not problematic.

> - why is childLMForBreakSkip a weak reference?? What will happen if the
>   object has been released at the moment where it’s retrieved? Will the
>   code still behave correctly? Furthermore, this childLMForBreakSkip
>   object will by definition always be the first child LM of the current
>   LM. So this field might not be needed at all in the end.

That was a mistake. I didn't remember that there is a childLMs field.
That's improved now.

> - I was hoping that the way I implemented breaks in table could be
>   generalized to the other LMs. Maybe that approach has its flaws that
>   I haven’t noticed but so far this works for tables.
>   Basically the idea is that instead of creating break elements, a LM
>   sets a flag on the LayoutContext it was given through the
>   getNextKnuthElements call. Then it’s the ancestor block stacking LM
>   that decides whether a break element must be produced (when the child
>   LM is in the middle of the sequence) or if the break property must be
>   passed on to the parent LM (when the child LM is the first child
>   of the block stacking LM).
>   See TableContentLM.getKnuthElementsForRowIterator,
>   RowGroupLM.getNextKnuthElements,
>   LayoutContext.get/setBreakBefore/After
>
> I think it would be very unfortunate to have two different mechanisms
> for handling breaks.

Please note that the break handling in tables has always been different
than all other FOs. And table came last. When I first implemented table
layout I took a shortcut by not supporting reentry to
getNextKnuthElements() after a forced break. After all, the method is
called getNextKnuthElements(), not getKnuthElements().

The problem with your approach is that you have to create the full
element list for the table (to get the right values back through the
LayoutContext) and only then can you do add the break element. In
the other LMs, break-before is handled first (and the method returns
early if a break-before happens). I assumed this gets revisited with the
new page breaking approach which is why I didn't invest more time in
this.

> The thing is, I’ll be offline for the next 10 days
> so I’ll have no time to do/review anything, and I realise that we
> wouldn’t want to delay the release of 0.95 too much. We could imagine to
> leave this change as is in the 0.95 branch, but it should not be merged
> back into the Trunk.
> 
> Vincent
> 
> 
> --
> Vincent Hennebert                            Anyware Technologies
> http://people.apache.org/~vhennebert         http://www.anyware-tech.com
> Apache FOP Committer                         FOP Development/Consulting




Jeremias Maerki


Re: svn commit: r669118 - in /xmlgraphics/fop/branches/fop-0_95: ./ src/java/org/apache/fop/fo/flow/ src/java/org/apache/fop/fo/flow/table/ src/java/org/apache/fop/fo/properties/ src/java/org/apache/fop/layoutmgr/ src/java/org/apache/fop/layoutmgr/list/ sr...

Posted by Jeremias Maerki <de...@jeremias-maerki.ch>.
I'm going to be away most of this week so I don't have much time for FOP,
either. And next week isn't much better. I guess it's going to be July
then.

On 19.06.2008 12:36:30 Vincent Hennebert wrote:
<snip/>
> I think it would be very unfortunate to have two different mechanisms
> for handling breaks. The thing is, I’ll be offline for the next 10 days
> so I’ll have no time to do/review anything, and I realise that we
> wouldn’t want to delay the release of 0.95 too much. We could imagine to
> leave this change as is in the 0.95 branch, but it should not be merged
> back into the Trunk.



Jeremias Maerki


Re: svn commit: r669118 - in /xmlgraphics/fop/branches/fop-0_95: ./ src/java/org/apache/fop/fo/flow/ src/java/org/apache/fop/fo/flow/table/ src/java/org/apache/fop/fo/properties/ src/java/org/apache/fop/layoutmgr/ src/java/org/apache/fop/layoutmgr/list/ sr...

Posted by Vincent Hennebert <vi...@anyware-tech.com>.
Hi,

> Author: jeremias
> Date: Wed Jun 18 02:02:45 2008
> New Revision: 669118
> 
> URL: http://svn.apache.org/viewvc?rev=669118&view=rev
> Log:
> Bugzilla #44412:
> Regression fix for empty pages caused by multiple collapsible breaks.
> No more empty block areas if a break-before occurs on the first child of an FO to match the behaviour of tables and other FO implementations (clarification by XSL WG pending).
> Added an accessor interface for break-before/-after to avoid long if..else lists in BlockStackingLayoutManager.

I’m afraid I’m not happy with this change. There are a couple of issues
that should be looked at IMO.

Simple things first:
- BlockStackingLM.getBreakBefore(boolean) is always given true as its
  parameter. Why specifying a parameter then? Plus this method is called
  only within BlockStackingLM, so it should be made private (at least
  for now).
- BlockStackingLM.getBreakBefore(): same here, this method should be
  made private. And if fobj doesn’t implement BreakPropertySet, why
  should EN_AUTO be returned? If this method is called on an object that
  doesn’t support it, I guess it should rather throw an exception than
  silently return a default value. Otherwise that makes it difficult to
  track down errors.
- why is childLMForBreakSkip a weak reference?? What will happen if the
  object has been released at the moment where it’s retrieved? Will the
  code still behave correctly? Furthermore, this childLMForBreakSkip
  object will by definition always be the first child LM of the current
  LM. So this field might not be needed at all in the end.
- I was hoping that the way I implemented breaks in table could be
  generalized to the other LMs. Maybe that approach has its flaws that
  I haven’t noticed but so far this works for tables.
  Basically the idea is that instead of creating break elements, a LM
  sets a flag on the LayoutContext it was given through the
  getNextKnuthElements call. Then it’s the ancestor block stacking LM
  that decides whether a break element must be produced (when the child
  LM is in the middle of the sequence) or if the break property must be
  passed on to the parent LM (when the child LM is the first child
  of the block stacking LM).
  See TableContentLM.getKnuthElementsForRowIterator,
  RowGroupLM.getNextKnuthElements,
  LayoutContext.get/setBreakBefore/After

I think it would be very unfortunate to have two different mechanisms
for handling breaks. The thing is, I’ll be offline for the next 10 days
so I’ll have no time to do/review anything, and I realise that we
wouldn’t want to delay the release of 0.95 too much. We could imagine to
leave this change as is in the 0.95 branch, but it should not be merged
back into the Trunk.

Vincent


--
Vincent Hennebert                            Anyware Technologies
http://people.apache.org/~vhennebert         http://www.anyware-tech.com
Apache FOP Committer                         FOP Development/Consulting