Bugzilla #45715: Break before (break-before) not respected on blocks nested in inlines.

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

src/java/org/apache/fop/layoutmgr/AbstractLayoutManager.java

@@ -41,8 +41,7 @@ import org.apache.fop.fo.flow.RetrieveMarker;
 /**
  * The base class for most LayoutManagers.
  */
-public abstract class AbstractLayoutManager extends AbstractBaseLayoutManager
-        implements Constants {
+public abstract class AbstractLayoutManager extends AbstractBaseLayoutManager implements Constants {
 
     /** logging instance */
     private static Log log = LogFactory.getLog(AbstractLayoutManager.class);

src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java

@@ -44,8 +44,8 @@ import org.apache.fop.traits.SpaceVal;
 /**
  * LayoutManager for a block-container FO.
  */
-public class BlockContainerLayoutManager extends BlockStackingLayoutManager
-                implements ConditionalElementListener {
+public class BlockContainerLayoutManager extends BlockStackingLayoutManager implements
+        ConditionalElementListener, BreakOpportunity {
 
     /**
      * logging instance
@@ -1043,6 +1043,11 @@ public class BlockContainerLayoutManager extends BlockStackingLayoutManager
         }
         return true;
     }
+
+    public int getBreakBefore() {
+        return BreakOpportunityHelper.getBreakBefore(this);
+    }
+
 }
 
 

src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java

@@ -43,8 +43,8 @@ import org.apache.fop.traits.SpaceVal;
 /**
  * LayoutManager for a block FO.
  */
-public class BlockLayoutManager extends BlockStackingLayoutManager
-            implements ConditionalElementListener {
+public class BlockLayoutManager extends BlockStackingLayoutManager implements ConditionalElementListener,
+        BreakOpportunity {
 
     /** logging instance */
     private static Log log = LogFactory.getLog(BlockLayoutManager.class);
@@ -504,4 +504,8 @@ public class BlockLayoutManager extends BlockStackingLayoutManager
         return true;
     }
 
+    public int getBreakBefore() {
+        return BreakOpportunityHelper.getBreakBefore(this);
+    }
+
 }

src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java

@@ -38,7 +38,6 @@ import org.apache.fop.fo.properties.KeepProperty;
 import org.apache.fop.fo.properties.SpaceProperty;
 import org.apache.fop.layoutmgr.inline.InlineLayoutManager;
 import org.apache.fop.traits.MinOptMax;
-import org.apache.fop.util.BreakUtil;
 import org.apache.fop.util.ListUtil;
 
 /**
@@ -1036,7 +1035,7 @@ public abstract class BlockStackingLayoutManager extends AbstractLayoutManager
      * @return true if an element has been added due to a break-before.
      */
     protected boolean addKnuthElementsForBreakBefore(List returnList, LayoutContext context) {
-        int breakBefore = getBreakBefore();
+        int breakBefore = BreakOpportunityHelper.getBreakBefore(this);
         if (breakBefore == EN_PAGE
                 || breakBefore == EN_COLUMN
                 || breakBefore == EN_EVEN_PAGE
@@ -1050,27 +1049,6 @@ public abstract class BlockStackingLayoutManager extends AbstractLayoutManager
         }
     }
 
-    /**
-     * Returns the break-before value of the current formatting object.
-     * @return the break-before value (Constants.EN_*)
-     */
-    private int getBreakBefore() {
-        int breakBefore = EN_AUTO;
-        if (fobj instanceof BreakPropertySet) {
-            breakBefore = ((BreakPropertySet)fobj).getBreakBefore();
-        }
-        if (true /* 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());
-            }
-        }
-        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

src/java/org/apache/fop/layoutmgr/BreakOpportunity.java

@@ -0,0 +1,36 @@
+/*
+ * 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.layoutmgr;
+
+/**
+ * Defines methods to evaluate break opportunities at a particular location in the tree of
+ * layout managers.
+ */
+public interface BreakOpportunity {
+
+    /**
+     * Returns the highest priority break-before value on this layout manager or its
+     * relevant descendants.
+     *
+     * @return the break-before value (Constants.EN_*)
+     */
+    int getBreakBefore();
+
+}

src/java/org/apache/fop/layoutmgr/BreakOpportunityHelper.java

@@ -0,0 +1,54 @@
+/*
+ * 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.layoutmgr;
+
+import org.apache.fop.fo.Constants;
+import org.apache.fop.fo.properties.BreakPropertySet;
+import org.apache.fop.util.BreakUtil;
+
+/**
+ * Helper implementations of the {@link BreakOpportunity} methods.
+ */
+public final class BreakOpportunityHelper {
+
+    private BreakOpportunityHelper() { }
+
+    /**
+     * Returns the break opportunity before the given layout manager. There is a break
+     * opportunity if the LM's FO has the break-before property set, or if there is a
+     * break opportunity before its first child LM.
+     *
+     * @return the break-before value (Constants.EN_*)
+     */
+    public static int getBreakBefore(AbstractLayoutManager layoutManager) {
+        int breakBefore = Constants.EN_AUTO;
+        if (layoutManager.getFObj() instanceof BreakPropertySet) {
+            breakBefore = ((BreakPropertySet) layoutManager.getFObj()).getBreakBefore();
+        }
+        LayoutManager childLM = layoutManager.getChildLM();
+        // It is assumed this is only called when the first LM is active.
+        if (childLM instanceof BreakOpportunity) {
+            BreakOpportunity bo = (BreakOpportunity) childLM;
+            breakBefore = BreakUtil.compareBreakClasses(breakBefore, bo.getBreakBefore());
+        }
+        return breakBefore;
+    }
+
+}

src/java/org/apache/fop/layoutmgr/inline/InlineLayoutManager.java

@@ -388,6 +388,13 @@ public class InlineLayoutManager extends InlineStackingLayoutManager {
             }
             lastSequence = ListUtil.getLast(returnList);
             lastChildLM = curLM;
+            // the context used to create this childLC above was applied a LayoutContext.SUPPRESS_BREAK_BEFORE
+            // in the getNextChildElements() method of the parent BlockLayoutManger; as a consequence all
+            // line breaks in blocks nested inside the inline associated with this ILM are being supressed;
+            // here we revert that supression; we do not need to do that for the first element since that
+            // is handled by the getBreakBefore() method of the wrapping BlockStackingLayoutManager.
+            // Note: this fix seems to work but is far from being the ideal way to do this
+            childLC.setFlags(LayoutContext.SUPPRESS_BREAK_BEFORE, false);
         }
 
         if (lastSequence != null) {

src/java/org/apache/fop/layoutmgr/inline/InlineStackingLayoutManager.java

@@ -28,6 +28,8 @@ import org.apache.fop.area.inline.Space;
 import org.apache.fop.fo.FObj;
 import org.apache.fop.fo.properties.SpaceProperty;
 import org.apache.fop.layoutmgr.AbstractLayoutManager;
+import org.apache.fop.layoutmgr.BreakOpportunity;
+import org.apache.fop.layoutmgr.BreakOpportunityHelper;
 import org.apache.fop.layoutmgr.KnuthElement;
 import org.apache.fop.layoutmgr.LayoutContext;
 import org.apache.fop.layoutmgr.NonLeafPosition;
@@ -39,8 +41,8 @@ import org.apache.fop.traits.MinOptMax;
  * which stack children in the inline direction, such as Inline or
  * Line. It should not be instantiated directly.
  */
-public abstract class InlineStackingLayoutManager extends AbstractLayoutManager
-                                         implements InlineLevelLayoutManager {
+public abstract class InlineStackingLayoutManager extends AbstractLayoutManager implements
+        InlineLevelLayoutManager, BreakOpportunity {
 
     /**
      * Size of border and padding in BPD (ie, before and after).
@@ -385,4 +387,9 @@ public abstract class InlineStackingLayoutManager extends AbstractLayoutManager
 
         return returnList;
     }
+
+    public int getBreakBefore() {
+        return BreakOpportunityHelper.getBreakBefore(this);
+    }
+
 }

status.xml

@@ -63,7 +63,10 @@
       documents. Example: the fix of marks layering will be such a case when it's done.
     -->
     <release version="FOP Trunk" date="TBD">
-      <action context="Code" dev="GA" type="fix" fixes-bug="48723" due-to="R. Meyer">
+      <action context="Layout" dev="GA" type="fix" fixes-bug="45715" due-to="Luis Bernardo">
+        Break before (break-before) not respected on blocks nested in inlines.
+      </action>
+      <action context="Renderers" dev="GA" type="fix" fixes-bug="48723" due-to="Robert Meyer">
         Fix for XGC when rendering PostScript using SVG being drawn upside down when using a custom affine transform.
       </action>
       <action context="Code" dev="GA" type="fix" fixes-bug="43940" due-to="Julien Aymé, Ognjen Blagojevic">
@@ -85,20 +88,20 @@
         Improve property function argument parsing, specifically, better separate required, optional, and variable arguments and the handling of optional argument defaults.
         Regularize property function class names.
       </action>
-      <action context="Code" dev="GA" type="fix" fixes-bug="51043" due-to="Pascal Sancho">
+      <action context="Layout" dev="GA" type="fix" fixes-bug="51043" due-to="Pascal Sancho">
         Don't restart layout unless abs(ipd difference) > 1 in order to prevent rounding issues from triggering false restart.
       </action>
       <action context="Fonts" dev="GA" type="update">
         Removing experimental feature that violates XSL-FO and Unicode semantics by misinterpreting Basic Latin code points. Users must use private use codepoints to access font specific
         character mappings that have no assigned Unicode code point. See bug 50492.
       </action>
-      <action context="Code" dev="GA" type="fix" fixes-bug="53103" due-to="Matthias Reischenbacher">
+      <action context="Layout" dev="GA" type="fix" fixes-bug="53103" due-to="Matthias Reischenbacher">
         Ensure that table cell spanning works in right-to-left writing mode.
       </action>
-      <action context="Code" dev="GA" type="fix" fixes-bug="53086">
+      <action context="Layout" dev="GA" type="fix" fixes-bug="53086">
         Ensure that table footer and header are included in bididirectional resolution.
       </action>
-      <action context="Code" dev="GA" type="fix" fixes-bug="53097">
+      <action context="Layout" dev="GA" type="fix" fixes-bug="53097">
         Ensure writing-mode specified on fo:table is used to determine writing mode of table and its descendants.
       </action>
       <action context="Code" dev="GA" type="fix" fixes-bug="53094" due-to="Luis Bernardo">
@@ -113,13 +116,13 @@
       <action context="Code" dev="GA" type="fix" fixes-bug="52572" due-to="Pascal Sancho">
         Prevent NPE on use of unsupported collapse-with-precedence; fall back to collapse. Fix checkstyle errors from prior commit.
       </action>
-      <action context="Code" dev="GA" type="fix" fixes-bug="52514" due-to="Luis Bernardo">
+      <action context="Layout" dev="GA" type="fix" fixes-bug="52514" due-to="Luis Bernardo">
         Ensure square image is appropriately scaled.
       </action>
       <action context="Code" dev="GA" type="fix" fixes-bug="50062">
         Invoke JVM in headless mode from FOP command scripts and JS shell to prevent stealing focus from GUI applications.
       </action>
-      <action context="Code" dev="GA" type="fix" fixes-bug="52114">
+      <action context="Renderers" dev="GA" type="fix" fixes-bug="52114">
         Take leading derived space before/after into account when computing rows for TXT renderer.
       </action>
       <action context="Code" dev="PH" type="add" fixes-bug="49893">

test/layoutengine/standard-testcases/block-break-inline-break-before.xml

@@ -0,0 +1,38 @@
+<?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 tests that two breaks across an inline are treated as one.</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-width="220pt" page-height="220pt"
+          margin="10pt">
+          <fo:region-body />
+        </fo:simple-page-master>
+      </fo:layout-master-set>
+      <fo:page-sequence master-reference="page">
+        <fo:flow flow-name="xsl-region-body">
+          <fo:block>one</fo:block>
+          <fo:block break-before="page">
+            <fo:inline>
+              <fo:block break-before="page">two</fo:block>
+            </fo:inline>
+          </fo:block>
+        </fo:flow>
+      </fo:page-sequence>
+    </fo:root>
+  </fo>
+  <checks>
+    <eval expected="1" xpath="//lineArea[starts-with(., 'one')]/ancestor::pageViewport/@nr" />
+    <eval expected="2" xpath="//lineArea[starts-with(., 'two')]/ancestor::pageViewport/@nr" />
+  </checks>
+</testcase>

test/layoutengine/standard-testcases/block-inline-break-before.xml

@@ -0,0 +1,40 @@
+<?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 basic breaks.</p>
+  </info>
+  <fo>
+    <fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+      <fo:layout-master-set>
+        <fo:simple-page-master master-name="normal" page-width="5in" page-height="5in" 
+          margin="0.5in">
+          <fo:region-body/>
+        </fo:simple-page-master>
+      </fo:layout-master-set>
+      <fo:page-sequence master-reference="normal">
+        <fo:flow flow-name="xsl-region-body">
+          <fo:block>one</fo:block>
+          <fo:block>
+            <fo:inline>
+              <fo:block break-before="page">two</fo:block>
+              <fo:block break-before="page">three</fo:block>
+            </fo:inline>
+          </fo:block>
+        </fo:flow>
+      </fo:page-sequence>
+    </fo:root>
+  </fo>
+  <checks>
+    <eval expected="1" xpath="//lineArea[starts-with(., 'one')]/ancestor::pageViewport/@nr" />
+    <eval expected="2" xpath="//lineArea[starts-with(., 'two')]/ancestor::pageViewport/@nr" />
+    <eval expected="3" xpath="//lineArea[starts-with(., 'three')]/ancestor::pageViewport/@nr" />
+  </checks>
+</testcase>

test/layoutengine/standard-testcases/block-inline-inline-break-before.xml

@@ -0,0 +1,40 @@
+<?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 tests a break inside nested inlines.</p>
+  </info>
+  <fo>
+    <fo:root xmlns:fo="http://www.w3.org/1999/XSL/Format">
+      <fo:layout-master-set>
+        <fo:simple-page-master master-name="normal" page-width="5in" page-height="5in" 
+          margin="0.5in">
+          <fo:region-body/>
+        </fo:simple-page-master>
+      </fo:layout-master-set>
+      <fo:page-sequence master-reference="normal">
+        <fo:flow flow-name="xsl-region-body">
+          <fo:block>one</fo:block>
+          <fo:block>
+            <fo:inline>
+              <fo:inline>
+                <fo:block break-before="page">two</fo:block>
+              </fo:inline>
+            </fo:inline>
+          </fo:block>
+        </fo:flow>
+      </fo:page-sequence>
+    </fo:root>
+  </fo>
+  <checks>
+    <eval expected="1" xpath="//lineArea[starts-with(., 'one')]/ancestor::pageViewport/@nr" />
+    <eval expected="2" xpath="//lineArea[starts-with(., 'two')]/ancestor::pageViewport/@nr" />
+  </checks>
+</testcase>