From c1c33af031b1cada6f6d1dad7aba6b8074f54567 Mon Sep 17 00:00:00 2001 From: Vincent Hennebert Date: Mon, 16 Jul 2012 19:09:00 +0000 Subject: [PATCH] Bugfix: An IllegalArgumentException was thrown when break-before was used inside a list git-svn-id: https://svn.apache.org/repos/asf/xmlgraphics/fop/trunk@1362203 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/fop/layoutmgr/AbstractBreaker.java | 12 ++- .../BlockContainerLayoutManager.java | 4 - .../fop/layoutmgr/BlockLayoutManager.java | 4 - .../layoutmgr/BlockStackingLayoutManager.java | 13 ++- .../org/apache/fop/layoutmgr/PageBreaker.java | 4 +- .../list/ListItemContentLayoutManager.java | 16 +++- .../layoutmgr/list/ListItemLayoutManager.java | 31 ++++-- status.xml | 3 + .../list-item-body_break-before.xml | 96 +++++++++++++++++++ ...ist-item-body_break-before_multicolumn.xml | 56 +++++++++++ .../page-sequence_last-page_space-before.xml | 59 ++++++++++++ .../table-cell_break-before_multicolumn.xml | 68 +++++++++++++ 12 files changed, 347 insertions(+), 19 deletions(-) create mode 100644 test/layoutengine/standard-testcases/list-item-body_break-before.xml create mode 100644 test/layoutengine/standard-testcases/list-item-body_break-before_multicolumn.xml create mode 100644 test/layoutengine/standard-testcases/page-sequence_last-page_space-before.xml create mode 100644 test/layoutengine/standard-testcases/table-cell_break-before_multicolumn.xml diff --git a/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java b/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java index 0f7dda3e7..843748226 100644 --- a/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java +++ b/src/java/org/apache/fop/layoutmgr/AbstractBreaker.java @@ -506,7 +506,17 @@ public abstract class AbstractBreaker { ListElement lastBreakElement = effectiveList.getElement(endElementIndex); if (lastBreakElement.isPenalty()) { KnuthPenalty pen = (KnuthPenalty)lastBreakElement; - lastBreakClass = pen.getBreakClass(); + if (pen.getPenalty() == KnuthPenalty.INFINITE) { + /** + * That means that there was a keep.within-page="always", but that + * it's OK to break at a column. TODO The break class is being + * abused to implement keep.within-column and keep.within-page. + * This is very misleading and must be revised. + */ + lastBreakClass = Constants.EN_COLUMN; + } else { + lastBreakClass = pen.getBreakClass(); + } } else { lastBreakClass = Constants.EN_COLUMN; } diff --git a/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java b/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java index adce89ac0..53e59e6ef 100644 --- a/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java +++ b/src/java/org/apache/fop/layoutmgr/BlockContainerLayoutManager.java @@ -1044,10 +1044,6 @@ public class BlockContainerLayoutManager extends BlockStackingLayoutManager impl return true; } - public int getBreakBefore() { - return BreakOpportunityHelper.getBreakBefore(this); - } - } diff --git a/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java b/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java index 03b2d380c..711bf492f 100644 --- a/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java +++ b/src/java/org/apache/fop/layoutmgr/BlockLayoutManager.java @@ -504,8 +504,4 @@ public class BlockLayoutManager extends BlockStackingLayoutManager implements Co return true; } - public int getBreakBefore() { - return BreakOpportunityHelper.getBreakBefore(this); - } - } diff --git a/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java b/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java index 78ab6711a..d0586aa95 100644 --- a/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java +++ b/src/java/org/apache/fop/layoutmgr/BlockStackingLayoutManager.java @@ -1035,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 = BreakOpportunityHelper.getBreakBefore(this); + int breakBefore = getBreakBefore(); if (breakBefore == EN_PAGE || breakBefore == EN_COLUMN || breakBefore == EN_EVEN_PAGE @@ -1049,6 +1049,17 @@ public abstract class BlockStackingLayoutManager extends AbstractLayoutManager } } + /** + * Returns the highest priority break-before value on this layout manager or its + * relevant descendants. + * + * @return the break-before value (Constants.EN_*) + * @see BreakOpportunity#getBreakBefore() + */ + public int getBreakBefore() { + return BreakOpportunityHelper.getBreakBefore(this); + } + /** * Creates Knuth elements for break-after and adds them to the return list. * @param returnList return list to add the additional elements to diff --git a/src/java/org/apache/fop/layoutmgr/PageBreaker.java b/src/java/org/apache/fop/layoutmgr/PageBreaker.java index 777180c55..791adc763 100644 --- a/src/java/org/apache/fop/layoutmgr/PageBreaker.java +++ b/src/java/org/apache/fop/layoutmgr/PageBreaker.java @@ -521,7 +521,6 @@ public class PageBreaker extends AbstractBreaker { return; case Constants.EN_COLUMN: case Constants.EN_AUTO: - case Constants.EN_PAGE: case -1: PageViewport pv = curPage.getPageViewport(); @@ -545,6 +544,7 @@ public class PageBreaker extends AbstractBreaker { /*curPage = */pslm.makeNewPage(false); } return; + case Constants.EN_PAGE: default: log.debug("handling break-before after page " + pslm.getCurrentPageNum() + " breakVal=" + getBreakClassName(breakVal)); @@ -560,7 +560,7 @@ public class PageBreaker extends AbstractBreaker { } /** - * Check if a blank page is needed to accomodate + * Check if a blank page is needed to accommodate * desired even or odd page number. * @param breakVal - value of break-before or break-after trait. */ diff --git a/src/java/org/apache/fop/layoutmgr/list/ListItemContentLayoutManager.java b/src/java/org/apache/fop/layoutmgr/list/ListItemContentLayoutManager.java index a84069082..4db101cd3 100644 --- a/src/java/org/apache/fop/layoutmgr/list/ListItemContentLayoutManager.java +++ b/src/java/org/apache/fop/layoutmgr/list/ListItemContentLayoutManager.java @@ -20,6 +20,7 @@ package org.apache.fop.layoutmgr.list; import java.util.LinkedList; +import java.util.List; import org.apache.fop.area.Area; import org.apache.fop.area.Block; @@ -28,9 +29,11 @@ import org.apache.fop.fo.flow.ListItemBody; import org.apache.fop.fo.flow.ListItemLabel; import org.apache.fop.fo.properties.KeepProperty; import org.apache.fop.layoutmgr.BlockStackingLayoutManager; +import org.apache.fop.layoutmgr.BreakOpportunity; import org.apache.fop.layoutmgr.Keep; import org.apache.fop.layoutmgr.LayoutContext; import org.apache.fop.layoutmgr.LayoutManager; +import org.apache.fop.layoutmgr.ListElement; import org.apache.fop.layoutmgr.NonLeafPosition; import org.apache.fop.layoutmgr.Position; import org.apache.fop.layoutmgr.PositionIterator; @@ -40,7 +43,7 @@ import org.apache.fop.layoutmgr.TraitSetter; /** * LayoutManager for a list-item-label or list-item-body FO. */ -public class ListItemContentLayoutManager extends BlockStackingLayoutManager { +public class ListItemContentLayoutManager extends BlockStackingLayoutManager implements BreakOpportunity { private Block curBlockArea; @@ -220,5 +223,16 @@ public class ListItemContentLayoutManager extends BlockStackingLayoutManager { public Keep getKeepWithPrevious() { return Keep.KEEP_AUTO; } + + @SuppressWarnings("unchecked") + @Override + public List getNextKnuthElements(LayoutContext context, int alignment) { + List elements = new LinkedList(); + do { + elements.addAll(super.getNextKnuthElements(context, alignment)); + } while (!isFinished()); + return elements; + } + } diff --git a/src/java/org/apache/fop/layoutmgr/list/ListItemLayoutManager.java b/src/java/org/apache/fop/layoutmgr/list/ListItemLayoutManager.java index bc7daa24f..dd2e8d6e1 100644 --- a/src/java/org/apache/fop/layoutmgr/list/ListItemLayoutManager.java +++ b/src/java/org/apache/fop/layoutmgr/list/ListItemLayoutManager.java @@ -35,6 +35,8 @@ import org.apache.fop.fo.flow.ListItemLabel; import org.apache.fop.fo.properties.KeepProperty; import org.apache.fop.layoutmgr.BlockStackingLayoutManager; import org.apache.fop.layoutmgr.BreakElement; +import org.apache.fop.layoutmgr.BreakOpportunity; +import org.apache.fop.layoutmgr.BreakOpportunityHelper; import org.apache.fop.layoutmgr.ConditionalElementListener; import org.apache.fop.layoutmgr.ElementListObserver; import org.apache.fop.layoutmgr.ElementListUtils; @@ -56,13 +58,14 @@ import org.apache.fop.layoutmgr.SpaceResolver; import org.apache.fop.layoutmgr.TraitSetter; import org.apache.fop.traits.MinOptMax; import org.apache.fop.traits.SpaceVal; +import org.apache.fop.util.BreakUtil; /** * LayoutManager for a list-item FO. * The list item contains a list item label and a list item body. */ -public class ListItemLayoutManager extends BlockStackingLayoutManager - implements ConditionalElementListener { +public class ListItemLayoutManager extends BlockStackingLayoutManager implements ConditionalElementListener, + BreakOpportunity { /** logging instance */ private static Log log = LogFactory.getLog(ListItemLayoutManager.class); @@ -204,6 +207,7 @@ public class ListItemLayoutManager extends BlockStackingLayoutManager // label childLC = makeChildLayoutContext(context); + childLC.setFlags(LayoutContext.SUPPRESS_BREAK_BEFORE); label.initialize(); labelList = label.getNextKnuthElements(childLC, alignment); @@ -217,6 +221,7 @@ public class ListItemLayoutManager extends BlockStackingLayoutManager // body childLC = makeChildLayoutContext(context); + childLC.setFlags(LayoutContext.SUPPRESS_BREAK_BEFORE); body.initialize(); bodyList = body.getNextKnuthElements(childLC, alignment); @@ -296,16 +301,23 @@ public class ListItemLayoutManager extends BlockStackingLayoutManager //Additional penalty height from penalties in the source lists int additionalPenaltyHeight = 0; int stepPenalty = 0; + int breakClass = EN_AUTO; KnuthElement endEl = (KnuthElement)elementLists[0].get(end[0]); if (endEl instanceof KnuthPenalty) { additionalPenaltyHeight = endEl.getWidth(); - stepPenalty = Math.max(stepPenalty, endEl.getPenalty()); + stepPenalty = endEl.getPenalty() == -KnuthElement.INFINITE ? -KnuthElement.INFINITE : Math + .max(stepPenalty, endEl.getPenalty()); + breakClass = BreakUtil.compareBreakClasses(breakClass, + ((KnuthPenalty) endEl).getBreakClass()); } endEl = (KnuthElement)elementLists[1].get(end[1]); if (endEl instanceof KnuthPenalty) { additionalPenaltyHeight = Math.max( additionalPenaltyHeight, endEl.getWidth()); - stepPenalty = Math.max(stepPenalty, endEl.getPenalty()); + stepPenalty = endEl.getPenalty() == -KnuthElement.INFINITE ? -KnuthElement.INFINITE : Math + .max(stepPenalty, endEl.getPenalty()); + breakClass = BreakUtil.compareBreakClasses(breakClass, + ((KnuthPenalty) endEl).getBreakClass()); } int boxHeight = step - addedBoxHeight - penaltyHeight; @@ -343,9 +355,9 @@ public class ListItemLayoutManager extends BlockStackingLayoutManager int p = stepPenalty; if (p > -KnuthElement.INFINITE) { p = Math.max(p, keep.getPenalty()); + breakClass = keep.getContext(); } - returnList.add(new BreakElement(stepPosition, penaltyHeight, p, keep.getContext(), - context)); + returnList.add(new BreakElement(stepPosition, penaltyHeight, p, breakClass, context)); } } @@ -693,6 +705,13 @@ public class ListItemLayoutManager extends BlockStackingLayoutManager body.reset(); } + @Override + public int getBreakBefore() { + int breakBefore = BreakOpportunityHelper.getBreakBefore(this); + breakBefore = BreakUtil.compareBreakClasses(breakBefore, label.getBreakBefore()); + breakBefore = BreakUtil.compareBreakClasses(breakBefore, body.getBreakBefore()); + return breakBefore; + } } diff --git a/status.xml b/status.xml index 3a433044f..35acb0e49 100644 --- a/status.xml +++ b/status.xml @@ -63,6 +63,9 @@ documents. Example: the fix of marks layering will be such a case when it's done. --> + + An IllegalArgumentException was thrown when break-before was used inside a list. + When restarting layout for the last page, discard glues and penalties at the beginning of the restarted Knuth sequence. diff --git a/test/layoutengine/standard-testcases/list-item-body_break-before.xml b/test/layoutengine/standard-testcases/list-item-body_break-before.xml new file mode 100644 index 000000000..64e45ad8f --- /dev/null +++ b/test/layoutengine/standard-testcases/list-item-body_break-before.xml @@ -0,0 +1,96 @@ + + + + + +

This test checks basic breaks in list bodies.

+ + + + + + + + + + + + + + (a) + + + + This content starts on page 1 and overflows to page 2. This content starts on page 1 and + overflows to page 2. This content starts on page 1 and overflows to page 2. This content + starts on page 1 and overflows to page 2. This content starts on page 1 and overflows to + page 2. This content starts on page 1 and overflows to page 2. This content starts on page 1 + and overflows to page 2. + + This content is on page 3. + + This content is on page 4. + + + + + + (b) + + + + + This content is on page 5. + + + + + + (c) + + + + This content is on page 6. + + + + + + (d) + + + + This content is on page 7. + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/test/layoutengine/standard-testcases/list-item-body_break-before_multicolumn.xml b/test/layoutengine/standard-testcases/list-item-body_break-before_multicolumn.xml new file mode 100644 index 000000000..a47daf107 --- /dev/null +++ b/test/layoutengine/standard-testcases/list-item-body_break-before_multicolumn.xml @@ -0,0 +1,56 @@ + + + + + +

This test checks basic breaks in list bodies.

+ + + + + + + + + + + + + + (a) + + + + On first page, first column + + On second page, first column, NOT first page, first column NEITHER first page, second column. + + + + + + + + + + + + + + \ No newline at end of file diff --git a/test/layoutengine/standard-testcases/page-sequence_last-page_space-before.xml b/test/layoutengine/standard-testcases/page-sequence_last-page_space-before.xml new file mode 100644 index 000000000..81f716105 --- /dev/null +++ b/test/layoutengine/standard-testcases/page-sequence_last-page_space-before.xml @@ -0,0 +1,59 @@ + + + + + +

When layout is re-started for the last page, the space-before on the first element to appear + on that page must be discarded.

+ + + + + + + + + + + + + + + + + + Filler 1 + Filler 2 + Filler 3 + + + Before line 1 + + Before the page break + After the page break + After line 1 + After line 2 + + + + + + + + diff --git a/test/layoutengine/standard-testcases/table-cell_break-before_multicolumn.xml b/test/layoutengine/standard-testcases/table-cell_break-before_multicolumn.xml new file mode 100644 index 000000000..cb8398af0 --- /dev/null +++ b/test/layoutengine/standard-testcases/table-cell_break-before_multicolumn.xml @@ -0,0 +1,68 @@ + + + + + +

This test checks basic breaks in table cells.

+ + + + + + + + + + + + + + + Page 1, Column 1, Cell 1 + + + Page 1, Column 1, Cell 2 + + + Page 1, Column 1, Cell 3 + + + + + Page 2, Column 1, Cell 1 + + + Page 2, Column 1, Cell 2 + + + Page 2, Column 1, Cell 3 + + + + + + + + + + + + + + \ No newline at end of file -- 2.39.5